From 3a39d90b03f7581d60716e3e9cf4dbcf7c48b08a Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 13 Jan 2023 23:13:35 +0000 Subject: [PATCH] bdev_gpt: add new SPDK partition type for off-by-one fix The gpt bdev module has an off-by-one error. When it calculates the size of the partition, it simply does "end - start", when really it should be "end - start + 1". We cannot just fix it by changing the math here, any consumers of the partition may have put down metadata on the partition based on the old size. So instead add a new SPDK partition type. SPDK will keep the existing off-by-one behavior when it finds the old partition type, but will use the correct math when finding the new partition type. Fixes issue #2801. Signed-off-by: Jim Harris Change-Id: I48eb48c781f1968b59e52b4477ca45e9c81eac11 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16298 Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Paul Luse Reviewed-by: Mike Gerdts Reviewed-by: Tomasz Zawadzki --- deprecation.md | 15 +++++++++++++++ doc/bdev.md | 4 ++-- module/bdev/gpt/gpt.h | 18 +++++++++++++++++- module/bdev/gpt/vbdev_gpt.c | 23 +++++++++++++++++++---- scripts/common.sh | 15 ++++++++++++++- scripts/spdk-gpt.py | 3 ++- test/bdev/blockdev.sh | 3 ++- 7 files changed, 71 insertions(+), 10 deletions(-) diff --git a/deprecation.md b/deprecation.md index 0f4810c2a..ac6eecb99 100644 --- a/deprecation.md +++ b/deprecation.md @@ -43,3 +43,18 @@ With the removal of this deprecation, calls to vbdev modules' `examine_disk()` a `examine_config()` callbacks will happen only on the app thread. This means that vbdev module maintainers will not need to make any changes to examine callbacks that call `spdk_bdev_register()` on the same thread as the examine callback uses. + +### gpt + +### `old_gpt_guid` + +Deprecated the SPDK partition type GUID `7c5222bd-8f5d-4087-9c00-bf9843c7b58c`. Partitions of this +type have bdevs created that are one block less than the actual size of the partition. Existing +partitions using the deprecated GUID can continue to use that GUID; support for the deprecated GUID +will remain in SPDK indefinitely, and will continue to exhibit the off-by-one bug so that on-disk +metadata layouts based on the incorrect size are not affected. + +See GitHub issue [2801](https://github.com/spdk/spdk/issues/2801) for additional details on the bug. + +New SPDK partition types should use GUID `6527994e-2c5a-4eec-9613-8f5944074e8b` which will create +a bdev of the correct size. diff --git a/doc/bdev.md b/doc/bdev.md index 16ab4e4ed..6934f82ed 100644 --- a/doc/bdev.md +++ b/doc/bdev.md @@ -274,7 +274,7 @@ possibly multiple virtual bdevs. ### SPDK GPT partition table {#bdev_ug_gpt} -The SPDK partition type GUID is `7c5222bd-8f5d-4087-9c00-bf9843c7b58c`. Existing SPDK bdevs +The SPDK partition type GUID is `6527994e-2c5a-4eec-9613-8f5944074e8b`. Existing SPDK bdevs can be exposed as Linux block devices via NBD and then can be partitioned with standard partitioning tools. After partitioning, the bdevs will need to be deleted and attached again for the GPT bdev module to see any changes. NBD kernel module must be @@ -312,7 +312,7 @@ parted -s /dev/nbd0 mkpart MyPartition '0%' '50%' # Change the partition type to the SPDK GUID. # sgdisk is part of the gdisk package. -sgdisk -t 1:7c5222bd-8f5d-4087-9c00-bf9843c7b58c /dev/nbd0 +sgdisk -t 1:6527994e-2c5a-4eec-9613-8f5944074e8b /dev/nbd0 # Stop the NBD device (stop exporting /dev/nbd0). rpc.py nbd_stop_disk /dev/nbd0 diff --git a/module/bdev/gpt/gpt.h b/module/bdev/gpt/gpt.h index 06f5f8223..86b90e416 100644 --- a/module/bdev/gpt/gpt.h +++ b/module/bdev/gpt/gpt.h @@ -13,8 +13,24 @@ #include "spdk/stdinc.h" #include "spdk/gpt_spec.h" +#include "spdk/log.h" + +#define SPDK_GPT_PART_TYPE_GUID SPDK_GPT_GUID(0x6527994e, 0x2c5a, 0x4eec, 0x9613, 0x8f5944074e8b) + +/* PART_TYPE_GUID_OLD partitions will be constructed as bdevs with one fewer block than expected. + * See GitHub issue #2801. + */ +#ifdef REGISTER_GUID_DEPRECATION +/* Register the deprecation in the header file, to make it clear to readers that this GUID + * shouldn't be used for new SPDK GPT partitions. We will never actually log this deprecation + * though, since we are not recommending that users try to migrate existing partitions with the + * old GUID to the new GUID. Wrap it in this REGISTER_GUID_DEPRECATION flag to avoid defining + * this deprecation in multiple compilation units. + */ +SPDK_LOG_DEPRECATION_REGISTER(old_gpt_guid, "old gpt guid", "Never", 0) +#endif +#define SPDK_GPT_PART_TYPE_GUID_OLD SPDK_GPT_GUID(0x7c5222bd, 0x8f5d, 0x4087, 0x9c00, 0xbf9843c7b58c) -#define SPDK_GPT_PART_TYPE_GUID SPDK_GPT_GUID(0x7c5222bd, 0x8f5d, 0x4087, 0x9c00, 0xbf9843c7b58c) #define SPDK_GPT_BUFFER_SIZE 32768 /* 32KB */ #define SPDK_GPT_GUID_EQUAL(x,y) (memcmp(x, y, sizeof(struct spdk_gpt_guid)) == 0) diff --git a/module/bdev/gpt/vbdev_gpt.c b/module/bdev/gpt/vbdev_gpt.c index 15cec1b1e..785efedaa 100644 --- a/module/bdev/gpt/vbdev_gpt.c +++ b/module/bdev/gpt/vbdev_gpt.c @@ -8,6 +8,7 @@ * each partition. */ +#define REGISTER_GUID_DEPRECATION #include "gpt.h" #include "spdk/endian.h" @@ -313,12 +314,26 @@ vbdev_gpt_create_bdevs(struct gpt_base *gpt_base) p = &gpt->partitions[i]; uint64_t lba_start = from_le64(&p->starting_lba); uint64_t lba_end = from_le64(&p->ending_lba); + uint64_t partition_size = lba_end - lba_start + 1; - if (!SPDK_GPT_GUID_EQUAL(&gpt->partitions[i].part_type_guid, - &SPDK_GPT_PART_TYPE_GUID) || - lba_start == 0) { + if (lba_start == 0) { continue; } + + if (SPDK_GPT_GUID_EQUAL(&gpt->partitions[i].part_type_guid, + &SPDK_GPT_PART_TYPE_GUID_OLD)) { + /* GitHub issue #2801 - we continue to report these partitions with + * off-by-one sizing error to ensure we don't break layouts based + * on that smaller size. */ + partition_size -= 1; + } else if (!SPDK_GPT_GUID_EQUAL(&gpt->partitions[i].part_type_guid, + &SPDK_GPT_PART_TYPE_GUID)) { + /* Partition type isn't TYPE_GUID or TYPE_GUID_OLD, so this isn't + * an SPDK parition. Continue to the next partition. + */ + continue; + } + if (lba_start < head_lba_start || lba_end > head_lba_end) { continue; } @@ -339,7 +354,7 @@ vbdev_gpt_create_bdevs(struct gpt_base *gpt_base) } rc = spdk_bdev_part_construct(&d->part, gpt_base->part_base, name, - lba_start, lba_end - lba_start, "GPT Disk"); + lba_start, partition_size, "GPT Disk"); free(name); if (rc) { SPDK_ERRLOG("could not construct bdev part\n"); diff --git a/scripts/common.sh b/scripts/common.sh index 23ab34ba6..fea156efb 100644 --- a/scripts/common.sh +++ b/scripts/common.sh @@ -361,12 +361,25 @@ block_in_use() { return 0 } +get_spdk_gpt_old() { + local spdk_guid + + [[ -e $rootdir/module/bdev/gpt/gpt.h ]] || return 1 + + GPT_H="$rootdir/module/bdev/gpt/gpt.h" + IFS="()" read -r _ spdk_guid _ < <(grep -w SPDK_GPT_PART_TYPE_GUID_OLD "$GPT_H") + spdk_guid=${spdk_guid//, /-} spdk_guid=${spdk_guid//0x/} + + echo "$spdk_guid" +} + get_spdk_gpt() { local spdk_guid [[ -e $rootdir/module/bdev/gpt/gpt.h ]] || return 1 - IFS="()" read -r _ spdk_guid _ < <(grep SPDK_GPT_PART_TYPE_GUID "$rootdir/module/bdev/gpt/gpt.h") + GPT_H="$rootdir/module/bdev/gpt/gpt.h" + IFS="()" read -r _ spdk_guid _ < <(grep -w SPDK_GPT_PART_TYPE_GUID "$GPT_H") spdk_guid=${spdk_guid//, /-} spdk_guid=${spdk_guid//0x/} echo "$spdk_guid" diff --git a/scripts/spdk-gpt.py b/scripts/spdk-gpt.py index 83ac771ba..c5410738a 100755 --- a/scripts/spdk-gpt.py +++ b/scripts/spdk-gpt.py @@ -40,6 +40,7 @@ def is_spdk_gpt(block, entry): disk_lbsize = lbsize(block) gpt_sig = 0x5452415020494645 # EFI PART spdk_guid = [0x7c5222bd, 0x8f5d, 0x4087, 0x9c00, 0xbf9843c7b58c] + spdk_guid2 = [0x6527994e, 0x2c5a, 0x4eec, 0x9613, 0x8f5944074e8b] if readb(block_path, disk_lbsize, 8) != gpt_sig: print("No valid GPT data, bailing") @@ -56,7 +57,7 @@ def is_spdk_gpt(block, entry): readb(block_path, part_entry_lba + 10, 8, ">Q") >> 16 ] - return guid == spdk_guid + return guid == spdk_guid or guid == spdk_guid2 if __name__ == "__main__": diff --git a/test/bdev/blockdev.sh b/test/bdev/blockdev.sh index cbb1631cd..dae0754f0 100755 --- a/test/bdev/blockdev.sh +++ b/test/bdev/blockdev.sh @@ -121,9 +121,10 @@ function setup_gpt_conf() { # Create gpt partition table parted -s "$gpt_nvme" mklabel gpt mkpart SPDK_TEST_first '0%' '50%' mkpart SPDK_TEST_second '50%' '100%' # change the GUID to SPDK GUID value + SPDK_GPT_OLD_GUID=$(get_spdk_gpt_old) SPDK_GPT_GUID=$(get_spdk_gpt) sgdisk -t "1:$SPDK_GPT_GUID" "$gpt_nvme" - sgdisk -t "2:$SPDK_GPT_GUID" "$gpt_nvme" + sgdisk -t "2:$SPDK_GPT_OLD_GUID" "$gpt_nvme" "$rootdir/scripts/setup.sh" "$rpc_py" bdev_get_bdevs setup_nvme_conf