Distinguish between "no partition" and "choose best partition" with a constant.

[• ian]

Summary

The values of the d_slice and d_partition fields of a disk_devdesc have a few values with special meanings in the disk_open() routine. Through various evoluations of the loader code over time, a d_partition value of -1 has meant both "use the first ufs partition found in the bsd label" and "don't open a bsd partition at all, open the raw slice."

This defines a new special value of -2 to mean open the raw slice, and it gives symbolic names to all the special values used in d_slice and d_parition, and adjusts all existing uses of those fields to use the new constants.

[• ian]

Update comments to explain D_PARTWILD and use D_PARTISGPT instead of 255.

[tsoome]

stand/common/disk.c

137 ↗

(On Diff #54117)

We did disk_open() and should be using handle from it. pa->dev is working here only because we *usually* do not have second level of nesting and we only do use sectorsize from this structure.

239 ↗

(On Diff #54117)

Actually this whole partdev variable is not needed. We only need to save the original slice and partition values (which we do anyhow).

On successful disk_open() we end up dev having the correct slice and partition set, and d_offset pointing to either disk or partition/slice start.

266 ↗

(On Diff #54117)

There and at line 277 we still continue to use the original hack and we do not check for the D_PARTWILD.

Perhaps something like this will give better overview what we want to see and will avoid getting lost in the if statement spaghetti:

switch (ptable_gettype(od->table)) {
case PTABLE_FREEBSD:

 if (partition == D_PARTWILD)
     ..
else
     ..
break;

case PTABLE_ISO9660:

...
break;

case PTABLE_GPT;

...
break;

case PTABLE_MBR:

...

}

369 ↗

(On Diff #54117)

Such constructs are now bad; it depends on specific value setup and is assuming particular ordering. If someone will change the values, all the related code will suddenly be wrong.

395 ↗

(On Diff #54117)

D_SLICENONE

stand/libsa/zfs/zfs.c

591 ↗

(On Diff #54117)

D_SLICENONE?

stand/uboot/common/main.c

342 ↗

(On Diff #54117)

Same as above about carefully crafted value ordering...

[imp]

I love this. Or at least the idea of this.
If we also kill some of the type-punning that remains for disk_devdesc and other types, I think we'll have tamed the beast
I've not checked through every single instance as I'm busy today, but wanted to blast out that I loved this.
The only other alternative might be to have a flags field inside of disc_devdesc which controls this sort of behavior if that would be easier. Given how pervasive this idiom is, though, I'm guessing maybe not. I mention this in case I'm wrong in my guess.

[• ian]

stand/common/disk.c

266 ↗

(On Diff #54117)

I specifically didn't change anything in disk_open() because I don't want to change the logic and change all the names at the same time. I'd rather commit the number->name changes and then commit a rewrite of disk_open() along the lines you suggest here in a week or so as a separate thing. That way if something goes wrong we don't have to guess whether it was a bad number->name change somewhere or bad logic in the disk_open() rewrite.

369 ↗

(On Diff #54117)

Same as disk_open(), I'd rather leave the logic changes for the next round.

395 ↗

(On Diff #54117)

Good catch, I missed this one somehow. But, it looks like what we actually want is D_SLICEWILD, which is a behavior change, but a sensible one. I would expect "ls disk0:" to work and find the best slice and partition, which it will only do with D_SLICEWILD. This is the one place where I do think we should change the logic by using WILD instead of NONE.

stand/uboot/common/main.c

342 ↗

(On Diff #54117)

This one is less complicated than disk_open(), so I will go ahead and switch to using the names here even though it's a bit of a logic change.

[• ian]

Fix a couple missed number->name changes.

In disk_parsedev() the change of slice = -1 to slice = D_SLICEWILD is actually a logic change as well, allowing diskN: to find the right slice and partition using the usual rules (preferring active freebsd partitions, etc).