From 64612d4e44289285a5cbdd26544421cdfd0a51b2 Mon Sep 17 00:00:00 2001 From: Conrad Meyer Date: Wed, 1 Jul 2020 02:16:36 +0000 Subject: [PATCH] geom(4): Kill GEOM_PART_EBR_COMPAT option Take advantage of Warner's nice new real GEOM aliasing system and use it for aliased partition names that actually work. Our canonical EBR partition name is the weird, not-default-on-x86-prior-to- this-revision "da1p4+00001234." However, if compatibility mode (tunable kern.geom.part.ebr.compat_aliases) is enabled (1, default), we continue to provide the alias names like "da1p5" in addition to the weird canonical names. Naming partition providers was just one aspect of the COMPAT knob; in addition it limited mutability, in part because it did not preserve existing EBR header content aside from that of LBA 0. This change saves the EBR header for LBA 0, as well as for every EBR partition encountered. That way, when we write out the EBR partition table on modification, we can restore any bootloader or other metadata in both LBA0 (the first data-containing EBR may start after 0) as well as every logical EBR we read from the disk, and only update the geometry metadata and linked list pointers that describe the actual partitioning. (This change does not add support for the 'bootcode' verb to EBR.) PR: 232463 Reported by: Manish Jain Discussed with: ae (no objection) Relnotes: maybe Differential Revision: https://reviews.freebsd.org/D24939 --- sys/amd64/conf/DEFAULTS | 1 - sys/conf/NOTES | 1 - sys/conf/options | 1 - sys/geom/part/g_part_ebr.c | 138 ++++++++++++++++++++++--------------- sys/i386/conf/DEFAULTS | 1 - 5 files changed, 83 insertions(+), 59 deletions(-) diff --git a/sys/amd64/conf/DEFAULTS b/sys/amd64/conf/DEFAULTS index e6af042986a0..f8334bd9af20 100644 --- a/sys/amd64/conf/DEFAULTS +++ b/sys/amd64/conf/DEFAULTS @@ -18,7 +18,6 @@ device uart_ns8250 # Default partitioning schemes options GEOM_PART_BSD options GEOM_PART_EBR -options GEOM_PART_EBR_COMPAT options GEOM_PART_MBR options GEOM_PART_GPT diff --git a/sys/conf/NOTES b/sys/conf/NOTES index e4a424432378..c55717b7a163 100644 --- a/sys/conf/NOTES +++ b/sys/conf/NOTES @@ -171,7 +171,6 @@ options GEOM_PART_APM # Apple partitioning options GEOM_PART_BSD # BSD disklabel options GEOM_PART_BSD64 # BSD disklabel64 options GEOM_PART_EBR # Extended Boot Records -options GEOM_PART_EBR_COMPAT # Backward compatible partition names options GEOM_PART_GPT # GPT partitioning options GEOM_PART_LDM # Logical Disk Manager options GEOM_PART_MBR # MBR partitioning diff --git a/sys/conf/options b/sys/conf/options index 2d52d43dcb65..25a5dfbaf588 100644 --- a/sys/conf/options +++ b/sys/conf/options @@ -125,7 +125,6 @@ GEOM_PART_APM opt_geom.h GEOM_PART_BSD opt_geom.h GEOM_PART_BSD64 opt_geom.h GEOM_PART_EBR opt_geom.h -GEOM_PART_EBR_COMPAT opt_geom.h GEOM_PART_GPT opt_geom.h GEOM_PART_LDM opt_geom.h GEOM_PART_MBR opt_geom.h diff --git a/sys/geom/part/g_part_ebr.c b/sys/geom/part/g_part_ebr.c index 40f852c6ddbd..0d372ec5d730 100644 --- a/sys/geom/part/g_part_ebr.c +++ b/sys/geom/part/g_part_ebr.c @@ -52,40 +52,48 @@ __FBSDID("$FreeBSD$"); FEATURE(geom_part_ebr, "GEOM partitioning class for extended boot records support"); -#if defined(GEOM_PART_EBR_COMPAT) FEATURE(geom_part_ebr_compat, "GEOM EBR partitioning class: backward-compatible partition names"); -#endif +SYSCTL_DECL(_kern_geom_part); +static SYSCTL_NODE(_kern_geom_part, OID_AUTO, ebr, CTLFLAG_RW | CTLFLAG_MPSAFE, + 0, "GEOM_PART_EBR Extended Boot Record"); + +static bool compat_aliases = true; +SYSCTL_BOOL(_kern_geom_part_ebr, OID_AUTO, compat_aliases, + CTLFLAG_RDTUN, &compat_aliases, 0, + "Set non-zero to enable EBR compatibility alias names (e.g., ada0p5)"); + +#define EBRNAMFMT "+%08u" #define EBRSIZE 512 struct g_part_ebr_table { struct g_part_table base; -#ifndef GEOM_PART_EBR_COMPAT - u_char ebr[EBRSIZE]; -#endif + u_char lba0_ebr[EBRSIZE]; }; struct g_part_ebr_entry { struct g_part_entry base; struct dos_partition ent; + u_char ebr[EBRSIZE]; + u_int ebr_compat_idx; }; static int g_part_ebr_add(struct g_part_table *, struct g_part_entry *, struct g_part_parms *); +static void g_part_ebr_add_alias(struct g_part_table *, struct g_provider *, + struct g_part_entry *, const char *); static int g_part_ebr_create(struct g_part_table *, struct g_part_parms *); static int g_part_ebr_destroy(struct g_part_table *, struct g_part_parms *); static void g_part_ebr_dumpconf(struct g_part_table *, struct g_part_entry *, struct sbuf *, const char *); static int g_part_ebr_dumpto(struct g_part_table *, struct g_part_entry *); -#if defined(GEOM_PART_EBR_COMPAT) -static void g_part_ebr_fullname(struct g_part_table *, struct g_part_entry *, - struct sbuf *, const char *); -#endif static int g_part_ebr_modify(struct g_part_table *, struct g_part_entry *, struct g_part_parms *); static const char *g_part_ebr_name(struct g_part_table *, struct g_part_entry *, char *, size_t); +static struct g_provider *g_part_ebr_new_provider(struct g_part_table *, + struct g_geom *, struct g_part_entry *, const char *); static int g_part_ebr_precheck(struct g_part_table *, enum g_part_ctl, struct g_part_parms *); static int g_part_ebr_probe(struct g_part_table *, struct g_consumer *); @@ -100,15 +108,14 @@ static int g_part_ebr_resize(struct g_part_table *, struct g_part_entry *, static kobj_method_t g_part_ebr_methods[] = { KOBJMETHOD(g_part_add, g_part_ebr_add), + KOBJMETHOD(g_part_add_alias, g_part_ebr_add_alias), KOBJMETHOD(g_part_create, g_part_ebr_create), KOBJMETHOD(g_part_destroy, g_part_ebr_destroy), KOBJMETHOD(g_part_dumpconf, g_part_ebr_dumpconf), KOBJMETHOD(g_part_dumpto, g_part_ebr_dumpto), -#if defined(GEOM_PART_EBR_COMPAT) - KOBJMETHOD(g_part_fullname, g_part_ebr_fullname), -#endif KOBJMETHOD(g_part_modify, g_part_ebr_modify), KOBJMETHOD(g_part_name, g_part_ebr_name), + KOBJMETHOD(g_part_new_provider, g_part_ebr_new_provider), KOBJMETHOD(g_part_precheck, g_part_ebr_precheck), KOBJMETHOD(g_part_probe, g_part_ebr_probe), KOBJMETHOD(g_part_read, g_part_ebr_read), @@ -171,7 +178,7 @@ ebr_entry_link(struct g_part_table *table, uint32_t start, uint32_t end, buf[0] = 0 /* dp_flag */; ebr_set_chs(table, start, &buf[3] /* dp_scyl */, &buf[1] /* dp_shd */, &buf[2] /* dp_ssect */); - buf[4] = 5 /* dp_typ */; + buf[4] = DOSPTYP_EXT /* dp_typ */; ebr_set_chs(table, end, &buf[7] /* dp_ecyl */, &buf[5] /* dp_ehd */, &buf[6] /* dp_esect */); le32enc(buf + 8, start); @@ -249,7 +256,9 @@ g_part_ebr_add(struct g_part_table *basetable, struct g_part_entry *baseentry, { struct g_provider *pp; struct g_part_ebr_entry *entry; + struct g_part_entry *iter; uint32_t start, size; + u_int idx; if (gpp->gpp_parms & G_PART_PARM_LABEL) return (EINVAL); @@ -276,9 +285,48 @@ g_part_ebr_add(struct g_part_table *basetable, struct g_part_entry *baseentry, &entry->ent.dp_shd, &entry->ent.dp_ssect); ebr_set_chs(basetable, baseentry->gpe_end, &entry->ent.dp_ecyl, &entry->ent.dp_ehd, &entry->ent.dp_esect); + + if (compat_aliases) { + idx = 5; + LIST_FOREACH(iter, &basetable->gpt_entry, gpe_entry) + idx++; + entry->ebr_compat_idx = idx; + } return (ebr_parse_type(gpp->gpp_type, &entry->ent.dp_typ)); } +static void +g_part_ebr_add_alias(struct g_part_table *table, struct g_provider *pp, + struct g_part_entry *baseentry, const char *pfx) +{ + struct g_part_ebr_entry *entry; + + g_provider_add_alias(pp, "%s%s" EBRNAMFMT, pfx, g_part_separator, + baseentry->gpe_index); + if (compat_aliases) { + entry = (struct g_part_ebr_entry *)baseentry; + g_provider_add_alias(pp, "%.*s%u", (int)strlen(pfx) - 1, pfx, + entry->ebr_compat_idx); + } +} + +static struct g_provider * +g_part_ebr_new_provider(struct g_part_table *table, struct g_geom *gp, + struct g_part_entry *baseentry, const char *pfx) +{ + struct g_part_ebr_entry *entry; + struct g_provider *pp; + + pp = g_new_providerf(gp, "%s%s" EBRNAMFMT, pfx, g_part_separator, + baseentry->gpe_index); + if (compat_aliases) { + entry = (struct g_part_ebr_entry *)baseentry; + g_provider_add_alias(pp, "%.*s%u", (int)strlen(pfx) - 1, pfx, + entry->ebr_compat_idx); + } + return (pp); +} + static int g_part_ebr_create(struct g_part_table *basetable, struct g_part_parms *gpp) { @@ -358,24 +406,6 @@ g_part_ebr_dumpto(struct g_part_table *table, struct g_part_entry *baseentry) entry->ent.dp_typ == DOSPTYP_LINSWP) ? 1 : 0); } -#if defined(GEOM_PART_EBR_COMPAT) -static void -g_part_ebr_fullname(struct g_part_table *table, struct g_part_entry *entry, - struct sbuf *sb, const char *pfx) -{ - struct g_part_entry *iter; - u_int idx; - - idx = 5; - LIST_FOREACH(iter, &table->gpt_entry, gpe_entry) { - if (iter == entry) - break; - idx++; - } - sbuf_printf(sb, "%.*s%u", (int)strlen(pfx) - 1, pfx, idx); -} -#endif - static int g_part_ebr_modify(struct g_part_table *basetable, struct g_part_entry *baseentry, struct g_part_parms *gpp) @@ -409,8 +439,7 @@ static const char * g_part_ebr_name(struct g_part_table *table, struct g_part_entry *entry, char *buf, size_t bufsz) { - - snprintf(buf, bufsz, "+%08u", entry->gpe_index); + snprintf(buf, bufsz, EBRNAMFMT, entry->gpe_index); return (buf); } @@ -418,11 +447,6 @@ static int g_part_ebr_precheck(struct g_part_table *table, enum g_part_ctl req, struct g_part_parms *gpp) { -#if defined(GEOM_PART_EBR_COMPAT) - if (req == G_PART_CTL_DESTROY) - return (0); - return (ECANCELED); -#else /* * The index is a function of the start of the partition. * This is not something the user can override, nor is it @@ -432,7 +456,6 @@ g_part_ebr_precheck(struct g_part_table *table, enum g_part_ctl req, if (req == G_PART_CTL_ADD) gpp->gpp_index = (gpp->gpp_start / table->gpt_sectors) + 1; return (0); -#endif } static int @@ -501,9 +524,10 @@ g_part_ebr_read(struct g_part_table *basetable, struct g_consumer *cp) struct g_part_ebr_entry *entry; u_char *buf; off_t ofs, msize; - u_int lba; + u_int lba, idx; int error, index; + idx = 5; pp = cp->provider; table = (struct g_part_ebr_table *)basetable; msize = MIN(pp->mediasize / pp->sectorsize, UINT32_MAX); @@ -525,18 +549,21 @@ g_part_ebr_read(struct g_part_table *basetable, struct g_consumer *cp) printf("GEOM: %s: invalid entries in the EBR ignored.\n", pp->name); } -#ifndef GEOM_PART_EBR_COMPAT - /* Save the first EBR, it can contain a boot code */ + /* + * Preserve EBR, it can contain boot code or other metadata we + * are ignorant of. + */ if (lba == 0) - bcopy(buf, table->ebr, sizeof(table->ebr)); -#endif - g_free(buf); + memcpy(table->lba0_ebr, buf, sizeof(table->lba0_ebr)); - if (ent[0].dp_typ == 0) + if (ent[0].dp_typ == 0) { + g_free(buf); break; + } if (ent[0].dp_typ == 5 && ent[1].dp_typ == 0) { lba = ent[0].dp_start; + g_free(buf); continue; } @@ -547,6 +574,10 @@ g_part_ebr_read(struct g_part_table *basetable, struct g_consumer *cp) pp->sectorsize; entry = (struct g_part_ebr_entry *)baseentry; entry->ent = ent[0]; + memcpy(entry->ebr, buf, sizeof(entry->ebr)); + if (compat_aliases) + entry->ebr_compat_idx = idx++; + g_free(buf); if (ent[1].dp_typ == 0) break; @@ -618,9 +649,7 @@ g_part_ebr_type(struct g_part_table *basetable, struct g_part_entry *baseentry, static int g_part_ebr_write(struct g_part_table *basetable, struct g_consumer *cp) { -#ifndef GEOM_PART_EBR_COMPAT struct g_part_ebr_table *table; -#endif struct g_provider *pp; struct g_part_entry *baseentry, *next; struct g_part_ebr_entry *entry; @@ -630,10 +659,10 @@ g_part_ebr_write(struct g_part_table *basetable, struct g_consumer *cp) pp = cp->provider; buf = g_malloc(pp->sectorsize, M_WAITOK | M_ZERO); -#ifndef GEOM_PART_EBR_COMPAT table = (struct g_part_ebr_table *)basetable; - bcopy(table->ebr, buf, DOSPARTOFF); -#endif + + _Static_assert(DOSPARTOFF <= sizeof(table->lba0_ebr), ""); + memcpy(buf, table->lba0_ebr, DOSPARTOFF); le16enc(buf + DOSMAGICOFFSET, DOSMAGIC); baseentry = LIST_FIRST(&basetable->gpt_entry); @@ -661,6 +690,9 @@ g_part_ebr_write(struct g_part_table *basetable, struct g_consumer *cp) do { entry = (struct g_part_ebr_entry *)baseentry; + _Static_assert(DOSPARTOFF <= sizeof(entry->ebr), ""); + memcpy(buf, entry->ebr, DOSPARTOFF); + p = buf + DOSPARTOFF; p[0] = entry->ent.dp_flag; p[1] = entry->ent.dp_shd; @@ -686,10 +718,6 @@ g_part_ebr_write(struct g_part_table *basetable, struct g_consumer *cp) error = g_write_data(cp, baseentry->gpe_start * pp->sectorsize, buf, pp->sectorsize); -#ifndef GEOM_PART_EBR_COMPAT - if (baseentry->gpe_start == 0) - bzero(buf, DOSPARTOFF); -#endif baseentry = next; } while (!error && baseentry != NULL); diff --git a/sys/i386/conf/DEFAULTS b/sys/i386/conf/DEFAULTS index bcde90c677e3..f0aee10c9c3c 100644 --- a/sys/i386/conf/DEFAULTS +++ b/sys/i386/conf/DEFAULTS @@ -19,7 +19,6 @@ device uart_ns8250 # Default partitioning schemes options GEOM_PART_BSD options GEOM_PART_EBR -options GEOM_PART_EBR_COMPAT options GEOM_PART_MBR options GEOM_PART_GPT