From 59f925c4ff3f576beffdfba3fd16093a44bd98a6 Mon Sep 17 00:00:00 2001 From: ian Date: Sun, 24 Mar 2019 18:51:52 +0000 Subject: [PATCH] Distinguish between "no partition" and "choose best partition" with a constant. 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 evolutions 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_partition, and adjusts all existing uses of those fields to use the new constants. The phab review for this timed out without being accepted, but I'm still citing it below because there is useful commentary there. Differential Revision: https://reviews.freebsd.org/D19262 --- stand/common/disk.c | 23 +++++++----------- stand/common/disk.h | 29 +++++++++++++++-------- stand/efi/libefi/efipart.c | 4 ++-- stand/efi/loader/main.c | 6 ++--- stand/i386/libi386/biosdisk.c | 8 +++---- stand/libsa/zfs/zfs.c | 2 +- stand/mips/beri/loader/beri_disk_cfi.c | 4 ++-- stand/mips/beri/loader/beri_disk_sdcard.c | 4 ++-- stand/uboot/common/main.c | 29 +++++++++++++---------- stand/uboot/lib/disk.c | 4 ++-- stand/usb/storage/umass_loader.c | 4 ++-- stand/userboot/userboot/main.c | 8 +++---- stand/userboot/userboot/userboot_disk.c | 4 ++-- 13 files changed, 69 insertions(+), 60 deletions(-) diff --git a/stand/common/disk.c b/stand/common/disk.c index fb0ec980581f..96ea87a4dd69 100644 --- a/stand/common/disk.c +++ b/stand/common/disk.c @@ -129,15 +129,8 @@ ptable_print(void *arg, const char *pname, const struct ptable_entry *part) dev.dd.d_dev = pa->dev->dd.d_dev; dev.dd.d_unit = pa->dev->dd.d_unit; dev.d_slice = part->index; - dev.d_partition = -1; + dev.d_partition = D_PARTNONE; if (disk_open(&dev, partsize, sectsize) == 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 = -1; - dev.d_offset = part->start; table = ptable_open(&dev, partsize, sectsize, ptblread); if (table != NULL) { sprintf(line, " %s%s", pa->prefix, pname); @@ -244,8 +237,8 @@ disk_open(struct disk_devdesc *dev, uint64_t mediasize, u_int sectorsize) */ memcpy(&partdev, dev, sizeof(partdev)); partdev.d_offset = 0; - partdev.d_slice = -1; - partdev.d_partition = -1; + partdev.d_slice = D_SLICENONE; + partdev.d_partition = D_PARTNONE; dev->d_offset = 0; table = NULL; @@ -373,9 +366,9 @@ disk_fmtdev(struct disk_devdesc *dev) char *cp; cp = buf + sprintf(buf, "%s%d", dev->dd.d_dev->dv_name, dev->dd.d_unit); - if (dev->d_slice >= 0) { + if (dev->d_slice > D_SLICENONE) { #ifdef LOADER_GPT_SUPPORT - if (dev->d_partition == 255) { + if (dev->d_partition == D_PARTISGPT) { sprintf(cp, "p%d:", dev->d_slice); return (buf); } else @@ -384,7 +377,7 @@ disk_fmtdev(struct disk_devdesc *dev) cp += sprintf(cp, "s%d", dev->d_slice); #endif } - if (dev->d_partition >= 0) + if (dev->d_partition > D_PARTNONE) cp += sprintf(cp, "%c", dev->d_partition + 'a'); strcat(cp, ":"); return (buf); @@ -398,7 +391,9 @@ disk_parsedev(struct disk_devdesc *dev, const char *devspec, const char **path) char *cp; np = devspec; - unit = slice = partition = -1; + unit = -1; + slice = D_SLICEWILD; + partition = D_PARTWILD; if (*np != '\0' && *np != ':') { unit = strtol(np, &cp, 10); if (cp == np) diff --git a/stand/common/disk.h b/stand/common/disk.h index 1e4543d09c9a..83109981e0a8 100644 --- a/stand/common/disk.h +++ b/stand/common/disk.h @@ -32,32 +32,35 @@ * * Whole disk access: * - * d_slice = -1 - * d_partition = -1 + * d_slice = D_SLICENONE + * d_partition = * * Whole MBR slice: * * d_slice = MBR slice number (typically 1..4) - * d_partition = -1 + * d_partition = D_PARTNONE * * BSD disklabel partition within an MBR slice: * * d_slice = MBR slice number (typically 1..4) - * d_partition = disklabel partition (typically 0..19) + * d_partition = disklabel partition (typically 0..19 or D_PARTWILD) * * BSD disklabel partition on the true dedicated disk: * - * d_slice = -1 - * d_partition = disklabel partition (typically 0..19) + * d_slice = D_SLICENONE + * d_partition = disklabel partition (typically 0..19 or D_PARTWILD) * * GPT partition: * * d_slice = GPT partition number (typically 1..N) - * d_partition = 255 + * d_partition = D_PARTISGPT * - * For both MBR and GPT, to automatically find the 'best' slice or partition, - * set d_slice to zero. This uses the partition type to decide which partition - * to use according to the following list of preferences: + * For MBR, setting d_partition to D_PARTWILD will automatically use the first + * partition within the slice. + * + * For both MBR and GPT, to automatically find the 'best' slice and partition, + * set d_slice to D_SLICEWILD. This uses the partition type to decide which + * partition to use according to the following list of preferences: * * FreeBSD (active) * FreeBSD (inactive) @@ -81,6 +84,12 @@ #ifndef _DISK_H #define _DISK_H +#define D_SLICENONE -1 +#define D_SLICEWILD 0 +#define D_PARTNONE -1 +#define D_PARTWILD -2 +#define D_PARTISGPT 255 + struct disk_devdesc { struct devdesc dd; /* Must be first. */ int d_slice; diff --git a/stand/efi/libefi/efipart.c b/stand/efi/libefi/efipart.c index 6f7f621b70c3..e893652bc3a6 100644 --- a/stand/efi/libefi/efipart.c +++ b/stand/efi/libefi/efipart.c @@ -813,8 +813,8 @@ efipart_print_common(struct devsw *dev, pdinfo_list_t *pdlist, int verbose) pd->pd_blkio = blkio; pd_dev.dd.d_dev = dev; pd_dev.dd.d_unit = pd->pd_unit; - pd_dev.d_slice = -1; - pd_dev.d_partition = -1; + pd_dev.d_slice = D_SLICENONE; + pd_dev.d_partition = D_PARTNONE; ret = disk_open(&pd_dev, blkio->Media->BlockSize * (blkio->Media->LastBlock + 1), blkio->Media->BlockSize); diff --git a/stand/efi/loader/main.c b/stand/efi/loader/main.c index 62a155599ea2..0a58bc7b7a49 100644 --- a/stand/efi/loader/main.c +++ b/stand/efi/loader/main.c @@ -217,12 +217,12 @@ set_currdev_pdinfo(pdinfo_t *dp) currdev.dd.d_dev = dp->pd_devsw; if (dp->pd_parent == NULL) { currdev.dd.d_unit = dp->pd_unit; - currdev.d_slice = -1; - currdev.d_partition = -1; + currdev.d_slice = D_SLICENONE; + currdev.d_partition = D_PARTNONE; } else { currdev.dd.d_unit = dp->pd_parent->pd_unit; currdev.d_slice = dp->pd_unit; - currdev.d_partition = 255; /* Assumes GPT */ + currdev.d_partition = D_PARTISGPT; /* XXX Assumes GPT */ } set_currdev_devdesc((struct devdesc *)&currdev); } else { diff --git a/stand/i386/libi386/biosdisk.c b/stand/i386/libi386/biosdisk.c index 036182d72c2b..65ea9516dbc5 100644 --- a/stand/i386/libi386/biosdisk.c +++ b/stand/i386/libi386/biosdisk.c @@ -691,8 +691,8 @@ bd_print_common(struct devsw *dev, bdinfo_list_t *bdi, int verbose) devd.dd.d_dev = dev; devd.dd.d_unit = i; - devd.d_slice = -1; - devd.d_partition = -1; + devd.d_slice = D_SLICENONE; + devd.d_partition = D_PARTNONE; if (disk_open(&devd, bd->bd_sectorsize * bd->bd_sectors, bd->bd_sectorsize) == 0) { @@ -745,8 +745,8 @@ bd_disk_get_sectors(struct disk_devdesc *dev) disk.dd.d_dev = dev->dd.d_dev; disk.dd.d_unit = dev->dd.d_unit; - disk.d_slice = -1; - disk.d_partition = -1; + disk.d_slice = D_SLICENONE; + disk.d_partition = D_PARTNONE; disk.d_offset = 0; size = bd->bd_sectors * bd->bd_sectorsize; diff --git a/stand/libsa/zfs/zfs.c b/stand/libsa/zfs/zfs.c index 8e9e50b6e85d..86fd3129318e 100644 --- a/stand/libsa/zfs/zfs.c +++ b/stand/libsa/zfs/zfs.c @@ -588,7 +588,7 @@ zfs_probe_dev(const char *devname, uint64_t *pool_guid) int slice = dev->d_slice; free(dev); - if (partition != -1 && slice != -1) { + if (partition != D_PARTNONE && slice != D_SLICENONE) { ret = zfs_probe(pa.fd, pool_guid); if (ret == 0) return (0); diff --git a/stand/mips/beri/loader/beri_disk_cfi.c b/stand/mips/beri/loader/beri_disk_cfi.c index 76b7bb7a9a76..d6e62662b0ab 100644 --- a/stand/mips/beri/loader/beri_disk_cfi.c +++ b/stand/mips/beri/loader/beri_disk_cfi.c @@ -129,8 +129,8 @@ beri_cfi_disk_print(int verbose) return (ret); dev.dd.d_dev = &beri_cfi_disk; dev.dd.d_unit = 0; - dev.d_slice = -1; - dev.d_partition = -1; + dev.d_slice = D_SLICENONE; + dev.d_partition = D_PARTNONE; if (disk_open(&dev, cfi_get_mediasize(), cfi_get_sectorsize()) == 0) { snprintf(line, sizeof(line), " cfi%d", 0); ret = disk_print(&dev, line, verbose); diff --git a/stand/mips/beri/loader/beri_disk_sdcard.c b/stand/mips/beri/loader/beri_disk_sdcard.c index f577eaf55289..820b0cea1600 100644 --- a/stand/mips/beri/loader/beri_disk_sdcard.c +++ b/stand/mips/beri/loader/beri_disk_sdcard.c @@ -135,8 +135,8 @@ beri_sdcard_disk_print(int verbose) return (ret); dev.dd.d_dev = &beri_sdcard_disk; dev.dd.d_unit = 0; - dev.d_slice = -1; - dev.d_partition = -1; + dev.d_slice = D_SLICENONE; + dev.d_partition = D_PARTNONE; if (disk_open(&dev, altera_sdcard_get_mediasize(), altera_sdcard_get_sectorsize()) == 0) { snprintf(line, sizeof(line), " sdcard%d", 0); diff --git a/stand/uboot/common/main.c b/stand/uboot/common/main.c index df53097c2a79..b8bd1cf05089 100644 --- a/stand/uboot/common/main.c +++ b/stand/uboot/common/main.c @@ -213,8 +213,8 @@ get_load_device(int *type, int *unit, int *slice, int *partition) *type = DEV_TYP_NONE; *unit = -1; - *slice = 0; - *partition = -1; + *slice = D_SLICEWILD; + *partition = D_PARTWILD; devstr = ub_env_get("loaderdev"); if (devstr == NULL) { @@ -295,7 +295,7 @@ get_load_device(int *type, int *unit, int *slice, int *partition) if (p == endp) { *type = DEV_TYP_NONE; *unit = -1; - *slice = 0; + *slice = D_SLICEWILD; return; } @@ -309,7 +309,7 @@ get_load_device(int *type, int *unit, int *slice, int *partition) if (*p != '.') { *type = DEV_TYP_NONE; *unit = -1; - *slice = 0; + *slice = D_SLICEWILD; return; } @@ -329,8 +329,8 @@ get_load_device(int *type, int *unit, int *slice, int *partition) /* Junk beyond partition number. */ *type = DEV_TYP_NONE; *unit = -1; - *slice = 0; - *partition = -1; + *slice = D_SLICEWILD; + *partition = D_PARTWILD; } static void @@ -339,15 +339,20 @@ print_disk_probe_info() char slice[32]; char partition[32]; - if (currdev.d_disk.d_slice > 0) - sprintf(slice, "%d", currdev.d_disk.d_slice); + if (currdev.d_disk.d_slice == D_SLICENONE) + strlcpy(slice, "", sizeof(slice)); + else if (currdev.d_disk.d_slice == D_SLICEWILD) + strlcpy(slice, "", sizeof(slice)); else - strcpy(slice, ""); + snprintf(slice, sizeof(slice), "%d", currdev.d_disk.d_slice); - if (currdev.d_disk.d_partition >= 0) - sprintf(partition, "%d", currdev.d_disk.d_partition); + if (currdev.d_disk.d_partition == D_PARTNONE) + strlcpy(partition, "", sizeof(partition)); + else if (currdev.d_disk.d_partition == D_PARTWILD) + strlcpy(partition, "", sizeof(partition)); else - strcpy(partition, ""); + snprintf(partition, sizeof(partition), "%d", + currdev.d_disk.d_partition); printf(" Checking unit=%d slice=%s partition=%s...", currdev.dd.d_unit, slice, partition); diff --git a/stand/uboot/lib/disk.c b/stand/uboot/lib/disk.c index bf07d04f7e36..70dcfdcc8863 100644 --- a/stand/uboot/lib/disk.c +++ b/stand/uboot/lib/disk.c @@ -254,8 +254,8 @@ stor_print(int verbose) for (i = 0; i < stor_info_no; i++) { dev.dd.d_dev = &uboot_storage; dev.dd.d_unit = i; - dev.d_slice = -1; - dev.d_partition = -1; + dev.d_slice = D_SLICENONE; + dev.d_partition = D_PARTNONE; snprintf(line, sizeof(line), "\tdisk%d (%s)\n", i, ub_stor_type(SI(&dev).type)); if ((ret = pager_output(line)) != 0) diff --git a/stand/usb/storage/umass_loader.c b/stand/usb/storage/umass_loader.c index fbb890be279a..1c12f05c2769 100644 --- a/stand/usb/storage/umass_loader.c +++ b/stand/usb/storage/umass_loader.c @@ -196,8 +196,8 @@ umass_disk_print(int verbose) return (ret); dev.d_dev = &umass_disk; dev.d_unit = 0; - dev.d_slice = -1; - dev.d_partition = -1; + dev.d_slice = D_SLICENONE; + dev.d_partition = D_PARTNONE; if (umass_disk_open_sub(&dev) == 0) { ret = disk_print(&dev, " umass0", verbose); diff --git a/stand/userboot/userboot/main.c b/stand/userboot/userboot/main.c index 25308b681b68..148398bd6c91 100644 --- a/stand/userboot/userboot/main.c +++ b/stand/userboot/userboot/main.c @@ -240,15 +240,15 @@ extract_currdev(void) if (userboot_disk_maxunit > 0) { dev.dd.d_dev = &userboot_disk; dev.dd.d_unit = 0; - dev.d_slice = 0; - dev.d_partition = 0; + dev.d_slice = D_SLICEWILD; + dev.d_partition = D_PARTWILD; /* * If we cannot auto-detect the partition type then * access the disk as a raw device. */ if (dev.dd.d_dev->dv_open(NULL, &dev)) { - dev.d_slice = -1; - dev.d_partition = -1; + dev.d_slice = D_SLICENONE; + dev.d_partition = D_PARTNONE; } dd = &dev.dd; } else { diff --git a/stand/userboot/userboot/userboot_disk.c b/stand/userboot/userboot/userboot_disk.c index 149ba25dc059..a4214997007e 100644 --- a/stand/userboot/userboot/userboot_disk.c +++ b/stand/userboot/userboot/userboot_disk.c @@ -137,8 +137,8 @@ userdisk_print(int verbose) break; dev.dd.d_dev = &userboot_disk; dev.dd.d_unit = i; - dev.d_slice = -1; - dev.d_partition = -1; + dev.d_slice = D_SLICENONE; + dev.d_partition = D_PARTNONE; if (disk_open(&dev, ud_info[i].mediasize, ud_info[i].sectorsize) == 0) { snprintf(line, sizeof(line), " disk%d", i);