From a1c7ae2d3f4ebf5037ecb4295731d715a50146a0 Mon Sep 17 00:00:00 2001 From: Krzysztof Karas Date: Mon, 24 Oct 2022 12:51:10 +0000 Subject: [PATCH] bdev: remove generation of UUIDs for bdevs that do not provide one Remove automatic generation of UUIDs for bdevs that do not provide this value themselves. This is to clarify whether this field can be depended upon. Modified match files to reflect change in UUID generation. Disabled nullglob shell option, as it deletes empty arrays during word splitting. Bdevs with no aliases would instead of "[]", have nullpointer printed, which makes resulting JSON invalid. Part of enhancement proposed in #2516. Change-Id: Ic1d5f8f8d001ae1a219e876aef2a19b1ff0b2f2c Signed-off-by: Krzysztof Karas Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15150 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker Reviewed-by: Tomasz Zawadzki Community-CI: Mellanox Build Bot --- lib/bdev/bdev.c | 28 +++++++++---------- test/bdev/blockdev.sh | 7 ++++- .../spdkcli_details_vhost.test.match | 5 +--- .../match_files/spdkcli_pmem.test.match | 4 +-- .../match_files/spdkcli_raid.test.match | 2 +- .../match_files/spdkcli_vhost.test.match | 18 ++++++------ test/unit/lib/bdev/bdev.c/bdev_ut.c | 2 ++ 7 files changed, 35 insertions(+), 31 deletions(-) diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 026de0795..06fc93262 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -6419,20 +6419,20 @@ bdev_register(struct spdk_bdev *bdev) return ret; } - /* If the user didn't specify a uuid, generate one. */ - if (spdk_mem_all_zero(&bdev->uuid, sizeof(bdev->uuid))) { - spdk_uuid_generate(&bdev->uuid); - } - - /* Add the UUID alias only if it's different than the name */ - spdk_uuid_fmt_lower(uuid, sizeof(uuid), &bdev->uuid); - if (strcmp(bdev->name, uuid) != 0) { - ret = spdk_bdev_alias_add(bdev, uuid); - if (ret != 0) { - SPDK_ERRLOG("Unable to add uuid:%s alias for bdev %s\n", uuid, bdev->name); - bdev_name_del(&bdev->internal.bdev_name); - free(bdev_name); - return ret; + /* UUID has to be specified by the user or defined by bdev itself. + * Otherwise this field must remain empty, to indicate that this + * value cannot be depended upon. */ + if (!spdk_mem_all_zero(&bdev->uuid, sizeof(bdev->uuid))) { + /* Add the UUID alias only if it's different than the name */ + spdk_uuid_fmt_lower(uuid, sizeof(uuid), &bdev->uuid); + if (strcmp(bdev->name, uuid) != 0) { + ret = spdk_bdev_alias_add(bdev, uuid); + if (ret != 0) { + SPDK_ERRLOG("Unable to add uuid:%s alias for bdev %s\n", uuid, bdev->name); + bdev_name_del(&bdev->internal.bdev_name); + free(bdev_name); + return ret; + } } } diff --git a/test/bdev/blockdev.sh b/test/bdev/blockdev.sh index 86ae556ca..0e98ae338 100755 --- a/test/bdev/blockdev.sh +++ b/test/bdev/blockdev.sh @@ -5,7 +5,11 @@ rootdir=$(readlink -f $testdir/../..) source $rootdir/test/common/autotest_common.sh source $testdir/nbd_common.sh -shopt -s nullglob extglob +# nullglob will remove unmatched words containing '*', '?', '[' characters during word splitting. +# This means that empty alias arrays will be removed instead of printing "[]", which breaks +# consecutive "jq" calls, as the "aliases" key will have no value and the whole JSON will be +# invalid. Hence do not enable this option for the duration of the tests in this script. +shopt -s extglob rpc_py=rpc_cmd conf_file="$testdir/bdev.json" @@ -626,6 +630,7 @@ CONF bdevs=$("$rpc_py" bdev_get_bdevs | jq -r '.[] | select(.claimed == false)') bdevs_name=$(echo $bdevs | jq -r '.name') bdev_list=($bdevs_name) + hello_world_bdev=${bdev_list[0]} trap - SIGINT SIGTERM EXIT killprocess "$spdk_tgt_pid" diff --git a/test/spdkcli/match_files/spdkcli_details_vhost.test.match b/test/spdkcli/match_files/spdkcli_details_vhost.test.match index b5aa7fe27..81982ad97 100644 --- a/test/spdkcli/match_files/spdkcli_details_vhost.test.match +++ b/test/spdkcli/match_files/spdkcli_details_vhost.test.match @@ -1,7 +1,5 @@ { - "aliases": [ - "$(UUID)" - ], + "aliases": [], "assigned_rate_limits": { "r_mbytes_per_sec": $(N), "rw_ios_per_sec": $(N), @@ -32,6 +30,5 @@ "write": $(S), "write_zeroes": $(S) }, - "uuid": "$(S)", "zoned": false } diff --git a/test/spdkcli/match_files/spdkcli_pmem.test.match b/test/spdkcli/match_files/spdkcli_pmem.test.match index 2a686f932..85b938d09 100644 --- a/test/spdkcli/match_files/spdkcli_pmem.test.match +++ b/test/spdkcli/match_files/spdkcli_pmem.test.match @@ -1,3 +1,3 @@ o- pmemblk .............................................................................................................. [Bdevs: 2] - o- pmem_bdev0 $(S) [$(UUID), Size=31.6M, Not claimed] - o- pmem_bdev1 $(S) [$(UUID), Size=31.6M, Not claimed] + o- pmem_bdev0 $(S) [Size=31.6M, Not claimed] + o- pmem_bdev1 $(S) [Size=31.6M, Not claimed] diff --git a/test/spdkcli/match_files/spdkcli_raid.test.match b/test/spdkcli/match_files/spdkcli_raid.test.match index c69e91276..b8403767f 100644 --- a/test/spdkcli/match_files/spdkcli_raid.test.match +++ b/test/spdkcli/match_files/spdkcli_raid.test.match @@ -10,7 +10,7 @@ o- bdevs ....................................................................... o- nvme ............................................................................................................... [Bdevs: 0] o- pmemblk ............................................................................................................ [Bdevs: 0] o- raid_volume ........................................................................................................ [Bdevs: 1] - | o- testraid $(S) [$(UUID), Size=16.0M, Not claimed] + | o- testraid $(S) [Size=16.0M, Not claimed] o- rbd ................................................................................................................ [Bdevs: 0] o- split_disk ......................................................................................................... [Bdevs: 0] o- virtioblk_disk ..................................................................................................... [Bdevs: 0] diff --git a/test/spdkcli/match_files/spdkcli_vhost.test.match b/test/spdkcli/match_files/spdkcli_vhost.test.match index eeabfc47a..b225c40be 100644 --- a/test/spdkcli/match_files/spdkcli_vhost.test.match +++ b/test/spdkcli/match_files/spdkcli_vhost.test.match @@ -1,11 +1,11 @@ o- / ......................................................................................................................... [...] o- bdevs ................................................................................................................... [...] | o- aio .............................................................................................................. [Bdevs: 2] - | | o- sample0 $(S) [$(UUID), Size=$(FP)M, Not claimed] - | | o- sample1 $(S) [$(UUID), Size=$(FP)M, Not claimed] + | | o- sample0 $(S) [Size=$(FP)M, Not claimed] + | | o- sample1 $(S) [Size=$(FP)M, Not claimed] | o- error ............................................................................................................ [Bdevs: 2] - | | o- EE_Malloc1 $(S) [$(UUID), Size=$(FP)M, Not claimed] - | | o- EE_Malloc4 $(S) [$(UUID), Size=$(FP)M, Not claimed] + | | o- EE_Malloc1 $(S) [Size=$(FP)M, Not claimed] + | | o- EE_Malloc4 $(S) [Size=$(FP)M, Not claimed] | o- iscsi ............................................................................................................ [Bdevs: 0] | o- logical_volume ................................................................................................... [Bdevs: 2] | | o- $(UUID) ................................................ [lvs0/lvol$(FP), Size=$(FP)M, Not claimed] @@ -21,15 +21,15 @@ o- / ........................................................................... | | o- null_bdev0 $(S) [$(UUID), Size=$(FP)M, Not claimed] | | o- null_bdev1 $(S) [$(UUID), Size=$(FP)M, Not claimed] | o- nvme ............................................................................................................. [Bdevs: 1] - | | o- Nvme0n1 $(S) [$(UUID), Size=$(S), Claimed] + | | o- Nvme0n1 $(S) [Size=$(S), Claimed] | o- pmemblk .......................................................................................................... [Bdevs: 0] | o- raid_volume ...................................................................................................... [Bdevs: 0] | o- rbd .............................................................................................................. [Bdevs: 0] | o- split_disk ....................................................................................................... [Bdevs: 4] - | | o- Nvme0n1p0 $(S) [$(UUID), Size=$(FP)G, Not claimed] - | | o- Nvme0n1p1 $(S) [$(UUID), Size=$(FP)G, Not claimed] - | | o- Nvme0n1p2 $(S) [$(UUID), Size=$(FP)G, Not claimed] - | | o- Nvme0n1p3 $(S) [$(UUID), Size=$(FP)G, Not claimed] + | | o- Nvme0n1p0 $(S) [Size=$(FP)G, Not claimed] + | | o- Nvme0n1p1 $(S) [Size=$(FP)G, Not claimed] + | | o- Nvme0n1p2 $(S) [Size=$(FP)G, Not claimed] + | | o- Nvme0n1p3 $(S) [Size=$(FP)G, Not claimed] | o- virtioblk_disk ................................................................................................... [Bdevs: 0] | o- virtioscsi_disk .................................................................................................. [Bdevs: 0] o- lvol_stores .................................................................................................. [Lvol stores: 2] diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index 23abc3ef1..952df403a 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -507,6 +507,8 @@ allocate_bdev(char *name) bdev->blockcnt = 1024; bdev->blocklen = 512; + spdk_uuid_generate(&bdev->uuid); + rc = spdk_bdev_register(bdev); poll_threads(); CU_ASSERT(rc == 0);