> On 18 Feb 2019, at 18:25, Warner Losh <i...@bsdimp.com> wrote: > > > > On Mon, Feb 18, 2019 at 7:31 AM Ian Lepore <i...@freebsd.org > <mailto:i...@freebsd.org>> wrote: > On Mon, 2019-02-18 at 15:09 +0200, Toomas Soome wrote: > > > On 18 Feb 2019, at 01:32, Ian Lepore <i...@freebsd.org> wrote: > > > > > > Author: ian > > > Date: Sun Feb 17 23:32:09 2019 > > > New Revision: 344238 > > > URL: https://svnweb.freebsd.org/changeset/base/344238 > > > <https://svnweb.freebsd.org/changeset/base/344238> > > > > > > Log: > > > Restore loader(8)'s ability for lsdev to show partitions within a bsd > > > slice. > > > > > > 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. > > > > > > 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. > > > > > > 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). > > > > Of course I can see the appeal for something like “boot disk0:” 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. > > > > I have no idea. As is so often the case, the commit message for the > revert said what the commit did ("revert to historic behavior") without > any hint of why the change was made. One has to assume that it broke > some working cases and people complained. > > Part of the problem for the disk_open() "api" is that there is not even > a comment block with some hints in it. I was thinking one potential > solution might instead of using "if (partition < 0)" we could define a > couple magical partition number values, PNUM_GETBEST = -1, > PNUM_RAWSLICE = -2, that sort of thing. But it would require carefully > combing through the existing code looking at all calls to disk_open() > and all usage of disk_devdesc.d_partition. > > I think that we should fix the disk_open() api. And then fix everything that > uses it to comply with the new rules. The current hodge-podge that we have > causes a number of issues for different environments that are only mostly > worked around. I've done some work in this area to make things more regular, > but it's still a dog's breakfast of almost-stackable devices that we overload > too many things on, resulting in the mis-mash we have today. When the whole > world was just MBR + BSD label, we could get away with it. But now that we > have GPT as well as GELI support and ZFS pool devices on top of disk devices, > the arrangements aren't working as well as could be desired. > > Warner
I am looking on it too, but it has more traps than I was suspecting:) rgds, toomas _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
