mav feedback

Signed-off-by: Paul Dagnelie <paul.dagnelie@klarasystems.com>

Author: pcd1193182

cmd/zdb/zdb.c

@@ -5637,37 +5637,21 @@ dump_label(const char *dev)
 			if (dump_opt['l'] > 2)
 				nvlist_print(stdout, toc);
 
-			uint32_t conf_size, toc_size, bootenv_size;
-			if (nvlist_lookup_uint32(toc, VDEV_TOC_TOC_SIZE,
-			    &toc_size) != 0) {
-				if (!dump_opt['q'])
-					(void) printf("failed to read size of "
-					    "TOC of label %d\n", l);
-				error = B_TRUE;
-				continue;
-			}
-			int err;
-			if ((err = nvlist_lookup_uint32(toc,
-			    VDEV_TOC_BOOT_REGION, &bootenv_size)) != 0) {
-				if (!dump_opt['q'])
-					(void) printf("failed to read size of "
-					    "bootenv of label %d %s\n", l,
-					    strerror(err));
-				error = B_TRUE;
-				continue;
-			}
-			if (nvlist_lookup_uint32(toc, VDEV_TOC_VDEV_CONFIG,
-			    &conf_size) != 0) {
+			uint32_t conf_size, conf_off;
+			if (!vdev_toc_get_secinfo(toc, VDEV_TOC_VDEV_CONFIG,
+			    &conf_size, &conf_off)) {
 				if (!dump_opt['q'])
 					(void) printf("failed to read size of "
 					    "vdev config of label %d\n", l);
 				error = B_TRUE;
+				fnvlist_free(toc);
 				continue;
 			}
+			fnvlist_free(toc);
 			buf = alloca(conf_size);
 			buflen = conf_size;
 			uint64_t phys_off = label->label_offset +
-			    VDEV_LARGE_PAD_SIZE + toc_size + bootenv_size;
+			    VDEV_LARGE_PAD_SIZE + conf_off;
 			if (pread64(fd, buf, conf_size, phys_off) !=
 			    conf_size) {
 				if (!dump_opt['q'])

cmd/zhack.c

@@ -1168,25 +1168,29 @@ zhack_repair_one_label_large(const zhack_repair_op_t op, const int fd,
 	}
 
 	uint32_t bootenv_size, vc_size, sc_size;
-	if ((err = nvlist_lookup_uint32(toc, VDEV_TOC_BOOT_REGION,
-	    &bootenv_size)) || (err = nvlist_lookup_uint32(toc,
-	    VDEV_TOC_VDEV_CONFIG, &vc_size)) || (err = nvlist_lookup_uint32(toc,
-	    VDEV_TOC_POOL_CONFIG, &sc_size))) {
+	uint32_t bootenv_offset, vc_offset, sc_offset;
+	if (!vdev_toc_get_secinfo(toc, VDEV_TOC_BOOT_REGION,
+	    &bootenv_size, &bootenv_offset) || !vdev_toc_get_secinfo(toc,
+	    VDEV_TOC_VDEV_CONFIG, &vc_size, &vc_offset) ||
+	    !vdev_toc_get_secinfo(toc, VDEV_TOC_POOL_CONFIG, &sc_size,
+	    &sc_offset)) {
+		fnvlist_free(toc);
 		(void) fprintf(stderr,
 		    "error: TOC missing core fields %d\n", l);
 		goto out;
 	}
+	fnvlist_free(toc);
 	bootenv = malloc(bootenv_size);
 	zio_eck_t *bootenv_eck = (zio_eck_t *)(bootenv + bootenv_size) - 1;
 	vdev_config = malloc(vc_size);
 	zio_eck_t *vc_eck = (zio_eck_t *)(vdev_config + vc_size) - 1;
 	spa_config = malloc(sc_size);
 	zio_eck_t *sc_eck = (zio_eck_t *)(spa_config + sc_size) - 1;
 
-	uint64_t offset = label_offset + VDEV_TOC_SIZE;
+	uint64_t base_offset = label_offset;
 	if (bootenv_size != 0) {
 		if ((err = zhack_repair_read(fd, bootenv,
-		    bootenv_size, offset, l)))
+		    bootenv_size, base_offset + bootenv_offset, l)))
 			goto out;
 		if (byteswap) {
 			byteswap_uint64_array(&bootenv_eck->zec_cksum,
@@ -1196,15 +1200,15 @@ zhack_repair_one_label_large(const zhack_repair_op_t op, const int fd,
 		}
 		if ((op & ZHACK_REPAIR_OP_CKSUM) == 0 &&
 		    zhack_repair_test_cksum(byteswap, bootenv, bootenv_size,
-		    bootenv_eck, offset, l) != 0) {
+		    bootenv_eck, base_offset + bootenv_offset, l) != 0) {
 			(void) fprintf(stderr, "It would appear checksums are "
 			    "corrupted. Try zhack repair label -c <device>\n");
 			goto out;
 		}
 	}
 
-	offset += bootenv_size;
-	if ((err = zhack_repair_read(fd, vdev_config, vc_size, offset, l)))
+	if ((err = zhack_repair_read(fd, vdev_config, vc_size,
+	    base_offset + vc_offset, l)))
 		goto out;
 
 	if (byteswap) {
@@ -1214,13 +1218,13 @@ zhack_repair_one_label_large(const zhack_repair_op_t op, const int fd,
 	}
 	if ((op & ZHACK_REPAIR_OP_CKSUM) == 0 &&
 	    zhack_repair_test_cksum(byteswap, vdev_config, vc_size,
-	    vc_eck, offset, l) != 0) {
+	    vc_eck, base_offset + vc_offset, l) != 0) {
 		(void) fprintf(stderr, "It would appear checksums are "
 		    "corrupted. Try zhack repair label -c <device>\n");
 		goto out;
 	}
-	offset += vc_size;
-	if ((err = zhack_repair_read(fd, spa_config, sc_size, offset, l)))
+	if ((err = zhack_repair_read(fd, spa_config, sc_size,
+	    base_offset + sc_offset, l)))
 		goto out;
 
 	if (byteswap) {
@@ -1230,7 +1234,7 @@ zhack_repair_one_label_large(const zhack_repair_op_t op, const int fd,
 	}
 	if ((op & ZHACK_REPAIR_OP_CKSUM) == 0 &&
 	    zhack_repair_test_cksum(byteswap, spa_config, sc_size,
-	    sc_eck, offset, l) != 0) {
+	    sc_eck, base_offset + sc_offset, l) != 0) {
 		(void) fprintf(stderr, "It would appear checksums are "
 		    "corrupted. Try zhack repair label -c <device>\n");
 		goto out;
@@ -1250,11 +1254,8 @@ zhack_repair_one_label_large(const zhack_repair_op_t op, const int fd,
 	if (err)
 		goto out;
 
-	const char *cfg_keys[] = { ZPOOL_CONFIG_VERSION,
-	    ZPOOL_CONFIG_POOL_STATE, ZPOOL_CONFIG_GUID };
-	nvlist_t *vdev_tree_cfg = NULL;
 	uint64_t ashift;
-	err = zhack_repair_get_ashift(cfg, l, cfg, &ashift);
+	err = zhack_repair_get_ashift(cfg, l, &ashift);
 	if (err)
 		return;
 
@@ -1278,21 +1279,17 @@ zhack_repair_one_label_large(const zhack_repair_op_t op, const int fd,
 		    label_offset, labels_repaired);
 	}
 
-	offset = label_offset;
 	if (zhack_repair_write_label(l, fd, byteswap, toc_data, toc_eck,
-	    offset, VDEV_TOC_SIZE))
+	    base_offset, VDEV_TOC_SIZE))
 		labels_repaired[l] |= REPAIR_LABEL_STATUS_CKSUM;
-	offset += VDEV_TOC_SIZE;
 	if (zhack_repair_write_label(l, fd, byteswap, bootenv, bootenv_eck,
-	    offset, bootenv_size))
+	    base_offset + bootenv_offset, bootenv_size))
 		labels_repaired[l] |= REPAIR_LABEL_STATUS_CKSUM;
-	offset += bootenv_size;
 	if (zhack_repair_write_label(l, fd, byteswap, vdev_config, vc_eck,
-	    offset, vc_size))
+	    base_offset + vc_offset, vc_size))
 		labels_repaired[l] |= REPAIR_LABEL_STATUS_CKSUM;
-	offset += vc_size;
 	if (zhack_repair_write_label(l, fd, byteswap, spa_config, sc_eck,
-	    offset, sc_size))
+	    base_offset + sc_offset, sc_size))
 		labels_repaired[l] |= REPAIR_LABEL_STATUS_CKSUM;
 
 	fsync(fd);

include/sys/vdev_impl.h

@@ -581,9 +581,10 @@ typedef struct vdev_label {
  * config, etc). The sub-nvlist contains relevant information, mostly offset
  * and size.
  *
- * Each sub-section is protected with an embedded checksum. In the event that a
- * sub-section is larger than 16MiB, it will be split in 16MiB - sizeof
- * (zio_eck_t) chunks, which will each have their own checksum.
+ * Currently, each sub-section is protected with an embedded checksum. In the
+ * event that a sub-section is larger than 16MiB, it will be split in
+ * 16MiB - sizeof (zio_eck_t) chunks, which will each have their own checksum.
+ * Future sub-sections may have their own checksum mechanisms (or none at all).
  */
 #define	VDEV_LARGE_PAD_SIZE	(1 << 24) // 16MiB
 #define	VDEV_LARGE_DATA_SIZE	((1 << 27) - VDEV_LARGE_PAD_SIZE)
@@ -594,22 +595,57 @@ typedef struct vdev_label {
 #define	VDEV_RESERVE_OFFSET	(VDEV_LARGE_LABEL_SIZE * 2)
 #define	VDEV_RESERVE_SIZE	(1 << 29) // 512MiB
 
-#define	VDEV_TOC_SIZE		(1 << 13)
+#define	VDEV_TOC_SIZE		(1 << 15)
+
+/*
+ * Each section in the label has its entry in the "sections" nvlist. This can
+ * store any necessary data, but will usually contain at least these two
+ * fields, representing the size and offset of the section.
+ */
+#define	VDEV_SECTION_SIZE	"section_size"
+#define	VDEV_SECTION_OFFSET	"section_offset"
 
 /*
  * While the data part of the TOC is always VDEV_TOC_SIZE, the actual write
  * gets rounded up to the ashift. We don't know the ashift yet early in import,
  * when we need to read this info.
  */
 #define	VDEV_TOC_TOC_SIZE	"toc_size"
-/* The size of the section that stores the boot region */
+#define	VDEV_TOC_SECTIONS	"sections"
+
+/* The section that stores the boot region */
 #define	VDEV_TOC_BOOT_REGION	"boot_region"
-/* The size of the section that stores the vdev config */
+/* The section that stores the vdev config */
 #define	VDEV_TOC_VDEV_CONFIG	"vdev_config"
-/* The size of the section that stores the pool config */
+/* The section that stores the pool config */
 #define	VDEV_TOC_POOL_CONFIG	"pool_config"
-/* The size of the section that stores auxilliary uberblocks */
-#define	VDEV_TOC_AUX_UBERBLOCK	"aux_uberblock"
+
+static inline boolean_t
+vdev_toc_get_secinfo(nvlist_t *toc, const char *section, uint32_t *size,
+    uint32_t *offset)
+{
+	nvlist_t *sections, *secinfo;
+	if (nvlist_lookup_nvlist(toc, VDEV_TOC_SECTIONS, &sections) != 0)
+		return (B_FALSE);
+	if (nvlist_lookup_nvlist(sections, section, &secinfo) != 0)
+		return (B_FALSE);
+	if (nvlist_lookup_uint32(secinfo, VDEV_SECTION_SIZE, size) != 0)
+		return (B_FALSE);
+	if (nvlist_lookup_uint32(secinfo, VDEV_SECTION_OFFSET, offset) != 0)
+		return (B_FALSE);
+	return (B_TRUE);
+}
+
+static inline void
+vdev_toc_add_secinfo(nvlist_t *sections, const char *section, uint32_t size,
+    uint32_t offset)
+{
+	nvlist_t *secinfo = fnvlist_alloc();
+	fnvlist_add_uint32(secinfo, VDEV_SECTION_SIZE, size);
+	fnvlist_add_uint32(secinfo, VDEV_SECTION_OFFSET, offset);
+	fnvlist_add_nvlist(sections, section, secinfo);
+	fnvlist_free(secinfo);
+}
 
 /*
  * vdev_dirty() flags

lib/libzfs/libzfs.abi

@@ -640,7 +640,7 @@
     <elf-symbol name='fletcher_4_superscalar_ops' size='128' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='libzfs_config_ops' size='16' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='sa_protocol_names' size='16' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
-    <elf-symbol name='spa_feature_table' size='2632' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
+    <elf-symbol name='spa_feature_table' size='2688' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfeature_checks_disable' size='4' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfs_deleg_perm_tab' size='544' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
     <elf-symbol name='zfs_history_event_names' size='328' type='object-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
@@ -9521,8 +9521,8 @@
     </function-decl>
   </abi-instr>
   <abi-instr address-size='64' path='module/zcommon/zfeature_common.c' language='LANG_C99'>
-    <array-type-def dimensions='1' type-id='83f29ca2' size-in-bits='21056' id='fd43354e'>
-      <subrange length='47' type-id='7359adad' id='8f8900fe'/>
+    <array-type-def dimensions='1' type-id='83f29ca2' size-in-bits='21504' id='bd288d11'>
+      <subrange length='48' type-id='7359adad' id='8f6d2a81'/>
     </array-type-def>
     <enum-decl name='zfeature_flags' id='6db816a4'>
       <underlying-type type-id='9cac1fee'/>
@@ -9600,7 +9600,7 @@
     <pointer-type-def type-id='611586a1' size-in-bits='64' id='2e243169'/>
     <qualified-type-def type-id='eaa32e2f' const='yes' id='83be723c'/>
     <pointer-type-def type-id='83be723c' size-in-bits='64' id='7acd98a2'/>
-    <var-decl name='spa_feature_table' type-id='fd43354e' mangled-name='spa_feature_table' visibility='default' elf-symbol-id='spa_feature_table'/>
+    <var-decl name='spa_feature_table' type-id='bd288d11' mangled-name='spa_feature_table' visibility='default' elf-symbol-id='spa_feature_table'/>
     <var-decl name='zfeature_checks_disable' type-id='c19b74c3' mangled-name='zfeature_checks_disable' visibility='default' elf-symbol-id='zfeature_checks_disable'/>
     <function-decl name='opendir' visibility='default' binding='global' size-in-bits='64'>
       <parameter type-id='80f4b756'/>

module/zfs/vdev_label.c

@@ -854,15 +854,12 @@ vdev_label_read_config(vdev_t *vd, uint64_t txg)
 
 			if (zio_wait(zio[l]) == 0 && nvlist_unpack(
 			    (char *)vp[l], VDEV_TOC_SIZE, &toc, 0) == 0) {
-				uint32_t bootconf_size;
-				if (nvlist_lookup_uint32(toc,
-				    VDEV_TOC_BOOT_REGION, &bootconf_size) != 0)
+				uint32_t off;
+				if (!vdev_toc_get_secinfo(toc,
+				    VDEV_TOC_VDEV_CONFIG,
+				    &vp_size[l], &off))
 					continue;
-				if (nvlist_lookup_uint32(toc,
-				    VDEV_TOC_VDEV_CONFIG, &vp_size[l]) != 0)
-					continue;
-				vp_off[l] = VDEV_LARGE_PAD_SIZE + toc_size +
-				    bootconf_size;
+				vp_off[l] = VDEV_LARGE_PAD_SIZE + off;
 				fnvlist_free(toc);
 			}
 		}
@@ -1323,9 +1320,14 @@ vdev_label_init(vdev_t *vd, uint64_t crtxg, vdev_labeltype_t reason)
 		    NV_ENCODE_XDR, KM_SLEEP));
 		fnvlist_free(spa_config);
 		fnvlist_add_uint32(toc, VDEV_TOC_TOC_SIZE, writesize);
-		fnvlist_add_uint32(toc, VDEV_TOC_BOOT_REGION, 0);
-		fnvlist_add_uint32(toc, VDEV_TOC_VDEV_CONFIG, vp_size);
-		fnvlist_add_uint32(toc, VDEV_TOC_POOL_CONFIG, sc_buflen);
+
+		nvlist_t *sections = fnvlist_alloc();
+		vdev_toc_add_secinfo(sections, VDEV_TOC_VDEV_CONFIG, vp_size,
+		    writesize);
+		vdev_toc_add_secinfo(sections, VDEV_TOC_POOL_CONFIG, sc_buflen,
+		    writesize + vp_size);
+		fnvlist_add_nvlist(toc, VDEV_TOC_SECTIONS, sections);
+		fnvlist_free(sections);
 
 		if (ub_abd2 == NULL)
 			ub_abd2 = abd_alloc_linear(SPA_MAXBLOCKSIZE, B_TRUE);
@@ -1454,18 +1456,22 @@ vdev_label_read_bootenv_impl(zio_t *zio, vdev_t *vd, int flags)
 				continue;
 			}
 			nvlist_t *toc;
-			uint32_t bootenv_size;
 			if (!nvlist_unpack(abd_to_buf(toc_abds[l]), toc_size,
-			    &toc, KM_SLEEP) || !nvlist_lookup_uint32(toc,
-			    VDEV_TOC_BOOT_REGION, &bootenv_size)) {
+			    &toc, KM_SLEEP)) {
 				abd_free(toc_abds[l]);
 				continue;
 			}
 			abd_free(toc_abds[l]);
+			uint32_t bootenv_size, bootenv_offset;
+			if (!vdev_toc_get_secinfo(toc, VDEV_TOC_BOOT_REGION,
+			    &bootenv_size, &bootenv_offset)) {
+				fnvlist_free(toc);
+				continue;
+			}
 
 			vdev_label_read(zio, vd, l, B_FALSE,
 			    abd_alloc_linear(bootenv_size, B_FALSE),
-			    VDEV_LARGE_PAD_SIZE + toc_size, bootenv_size,
+			    VDEV_LARGE_PAD_SIZE + bootenv_offset, bootenv_size,
 			    vdev_label_read_bootenv_done, zio, flags);
 		}
 	} else {
@@ -1659,18 +1665,23 @@ vdev_label_write_bootenv(vdev_t *vd, nvlist_t *env)
 				continue;
 			}
 			nvlist_t *toc;
-			uint32_t bootenv_size;
+			uint32_t bootenv_size, bootenv_offset;
 			if (!(error = nvlist_unpack(abd_to_buf(toc_abds[l]),
-			    toc_size, &toc, KM_SLEEP)) ||
-			    (error = !nvlist_lookup_uint32(toc,
-			    VDEV_TOC_BOOT_REGION, &bootenv_size))) {
+			    toc_size, &toc, KM_SLEEP))) {
 				all_writeable = B_FALSE;
 				continue;
 			}
+			if (!vdev_toc_get_secinfo(toc, VDEV_TOC_BOOT_REGION,
+			    &bootenv_size, &bootenv_offset)) {
+				fnvlist_free(toc);
+				all_writeable = B_FALSE;
+				continue;
+			}
+			fnvlist_free(toc);
 
 			if (new_abd_size == bootenv_size) {
 				vdev_label_write(zio, vd, l, B_TRUE, abd,
-				    VDEV_LARGE_PAD_SIZE + toc_size,
+				    VDEV_LARGE_PAD_SIZE + bootenv_offset,
 				    new_abd_size, NULL, NULL, flags);
 			} else {
 				all_writeable = B_FALSE;
@@ -2163,9 +2174,18 @@ vdev_label_sync_large(vdev_t *vd, zio_t *zio, uint64_t *good_writes,
 	size_t writesize = P2ROUNDUP(toc_buflen, 1 << vd->vdev_ashift);
 	nvlist_t *toc = fnvlist_alloc();
 	fnvlist_add_uint32(toc, VDEV_TOC_TOC_SIZE, writesize);
-	fnvlist_add_uint32(toc, VDEV_TOC_BOOT_REGION, bootenv_size);
-	fnvlist_add_uint32(toc, VDEV_TOC_VDEV_CONFIG, vdev_config_size);
-	fnvlist_add_uint32(toc, VDEV_TOC_POOL_CONFIG, pool_config_size);
+
+	nvlist_t *sections = fnvlist_alloc();
+	if (bootenv_size != 0) {
+		vdev_toc_add_secinfo(sections, VDEV_TOC_BOOT_REGION,
+		    bootenv_size, writesize);
+	}
+	vdev_toc_add_secinfo(sections, VDEV_TOC_VDEV_CONFIG, vdev_config_size,
+	    writesize + bootenv_size);
+	vdev_toc_add_secinfo(sections, VDEV_TOC_POOL_CONFIG, pool_config_size,
+	    writesize + bootenv_size + vdev_config_size);
+	fnvlist_add_nvlist(toc, VDEV_TOC_SECTIONS, sections);
+	fnvlist_free(sections);
 
 	ASSERT3U(fnvlist_size(toc) + sizeof (zio_eck_t), <=, VDEV_TOC_SIZE);
 

tests/zfs-tests/tests/functional/large_label/large_label_001_pos.ksh

@@ -42,7 +42,7 @@ log_assert "Verify that new label works for basic pool operation"
 log_onexit cleanup
 
 mntpnt="$TESTDIR1"
-log_must truncate --size=2T $mntpnt/dsk0
+log_must truncate -s 2T $mntpnt/dsk0
 
 DSK="$mntpnt/dsk"