Page MenuHomeFreeBSD

geom_label: Partially reinstate old sysinstall(8) workaround
ClosedPublic

Authored by jrtc27 on Jul 5 2021, 7:58 PM.

Details

Summary

This partially reverts commit af433832f7520840c22edd1fe1266c1a5cb781ad.
Since such bogus disklabels still exist in the wild, we now probe for a
disklabel to decide whether to ignore the UFS partition or not; if there
is a label then we use the old behaviour, and if there isn't one then we
use the new behaviour.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jrtc27 requested review of this revision.Jul 5 2021, 7:58 PM
jrtc27 created this revision.

Fix return value for non-MBR/EBR case; at one point I had the whole function's meaning inverted and failed to update this line.

sys/geom/label/g_label_ufs.c
77

Testing, the above strcmp for MBR and EBR don't work. BSD and BSD64 don't work either.

Without the above tests, there is no regression.

78

Should this be the 64-bit fs_size?

sys/geom/label/g_label_ufs.c
77

Hm, if you just printf the providers coming through what do you see?

78

Ah, yes, it should be for UFS2, I failed to spot that difference between the two in the original code

Yes, printf was my next todo. Tonight or tomorrow.

I performed two tests. First on my laptop. MBR with three BSD slices, each BSD slice has a BSD label, i.e. ada0s2a = ufs/Sroot, ada0s2d = ufs/Svar, ada0s2f = ufs/Susr. The provider name is "PART".

On my sandbox machine there are two boot disks. Slices are mirrord (not the entire disk because the disks can potentially be different sizes). For example, ada0s1 is mirrored with ada1s1, named mirror/gm0. mirror/gm0 contains a BSD label containing four partitions. The provider names are "MIRROR" and "PART".

The printf was placed after the const char *provider_name.

Is there a more direct way to test to see if the provider index is the same as the index for the bsd partitions? That would solve the false match on the whole slice issue maybe?

Is there any ordering on the tasting? I'd naively assume there's nothing stopping geom_label from tasting the slice before geom_part tastes it and discovers the partitions?

Reworked to instead directly probe for a disklabel rather than trying to guess based on the provider. The previous code was wrong (should have been checking for PART then looking for MBR or EBR in the provider's softc as is done by g_label_gpt) but even the corrected version is still insufficient for a raw disklabel in a mirror with no MBR directly containing it (or even a whole drive with a bare disklabel, though that's highly discouraged).

Fix inequality in overlap check

It boots; fsck is good. On both machines. On the other machines, which use a more traditional configuration, no problems there either.

No comments about the code. LGTM.

This revision is now accepted and ready to land.Jul 8 2021, 1:19 AM

Code wise it looks good. I am not familiar with the exact layout on disk, but given cy@ has tested it and found it to work, I am happy that those aspects of the code are correct.