Date: Mon, 18 Feb 2019 15:09:02 +0200 From: Toomas Soome <tsoome@me.com> To: Ian Lepore <ian@FreeBSD.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r344238 - head/stand/common Message-ID: <2EAE5156-2C63-47FC-977F-0D9676CABF7F@me.com> In-Reply-To: <201902172332.x1HNW9ut059440@repo.freebsd.org> References: <201902172332.x1HNW9ut059440@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 18 Feb 2019, at 01:32, Ian Lepore <ian@FreeBSD.org> wrote: >=20 > Author: ian > Date: Sun Feb 17 23:32:09 2019 > New Revision: 344238 > URL: https://svnweb.freebsd.org/changeset/base/344238 >=20 > Log: > Restore loader(8)'s ability for lsdev to show partitions within a bsd = slice. >=20 > I'm pretty sure this used to work at one time, perhaps long ago. It = has > been failing recently because if you call disk_open() with = dev->d_partition > set to -1 when d_slice refers to a bsd slice, it assumes you want it = to > open the first partition within that slice. When you then pass that = open > dev instance to ptable_open(), it tries to read the start of the 'a' > partition and decides there is no recognizable partition type there. >=20 > This restores the old functionality by resetting d_offset to the = start > of the raw slice after disk_open() returns. For good measure, = d_partition > is also set back to -1, although that doesn't currently affect = anything. >=20 > I would have preferred to make disk_open() avoid such rude = assumptions and > if you ask for partition -1 you get the raw slice. But the commit = history > shows that someone already did that once (r239058), and had to revert = it > (r239232), so I didn't even try to go down that road. What was the reason for the revert? I still do think the current = disk_open() approach is not good because it does break the promise to = get MBR partition (see common/disk.h).=20 Of course I can see the appeal for something like =E2=80=9Cboot = disk0:=E2=80=9D but case like that should be solved by iterating = partition table, and not by making API to do wrong thing - if I did ask = to for disk0s0: I really would expect to get disk0s0: and not disk0s0a: But anyhow, it would be good to understand the actual reasoning behind = that decision. rgds, toomas >=20 > Modified: > head/stand/common/disk.c >=20 > Modified: head/stand/common/disk.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- head/stand/common/disk.c Sun Feb 17 20:25:07 2019 = (r344237) > +++ head/stand/common/disk.c Sun Feb 17 23:32:09 2019 = (r344238) > @@ -133,6 +133,13 @@ ptable_print(void *arg, const char *pname, const = struc > dev.d_partition =3D -1; > if (disk_open(&dev, part->end - part->start + 1, > od->sectorsize) =3D=3D 0) { > + /* > + * disk_open() for partition -1 on a bsd slice = assumes > + * you want the first bsd partition. Reset = things so > + * that we're looking at the start of the raw = slice. > + */ > + dev.d_partition =3D -1; > + dev.d_offset =3D part->start; > table =3D ptable_open(&dev, part->end - = part->start + 1, > od->sectorsize, ptblread); > if (table !=3D NULL) { >=20
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2EAE5156-2C63-47FC-977F-0D9676CABF7F>