Commit 1c41caa7 authored by Marcin Spiewak's avatar Marcin Spiewak Committed by Tomasz Zawadzki
Browse files

bdev/nvme: handle uuid generation errors 



This is second patch in series that will implement
error code handling during uuid generation.

The nvme_generate_uuid() function definition is changed,
it takes additional parameter as a pointer to
location where the uuid is stored, and returns error
code instead of the uuid structure.

If in nvme_disk_create() function there is an
error code returned from nvme_generate_uuid(),
an error message is logged and the functions returns
with the error code passed from nvme_generate_uuid().

Now, when we are handlig errors from nvme_generate_uuid(),
an assert that was verifying status of snprintf()
operation is replaced by regular 'if' and returning
error code in case of fail.

Change-Id: Iaf1a76597d9aa98c433ebb9d69ba6f9a22773deb
Signed-off-by: default avatarMarcin Spiewak <marcin.spiewak@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/22291


Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 7b124df3

module/bdev/nvme/bdev_nvme.c

+17 −6
Original line number Diff line number Diff line
@@ -3969,9 +3969,10 @@ nvme_ns_set_ana_state(const struct spdk_nvme_ana_group_descriptor *desc, void *c 
	return 0;

}



static struct spdk_uuid

nvme_generate_uuid(const char *sn, uint32_t nsid)

static int

nvme_generate_uuid(const char *sn, uint32_t nsid, struct spdk_uuid *uuid)

{

	int rc = 0;

	struct spdk_uuid new_uuid, namespace_uuid;

	char merged_str[SPDK_NVME_CTRLR_SN_LEN + NSID_STR_LEN + 1] = {'\0'};

	/* This namespace UUID was generated using uuid_generate() method. */
@@ -3984,13 +3985,18 @@ nvme_generate_uuid(const char *sn, uint32_t nsid) 
	spdk_uuid_set_null(&namespace_uuid);



	size = snprintf(merged_str, sizeof(merged_str), "%s%"PRIu32, sn, nsid);

	assert(size > 0 && (unsigned long)size < sizeof(merged_str));

	if (size <= 0 || (unsigned long)size >= sizeof(merged_str)) {

		return -EINVAL;

	}



	spdk_uuid_parse(&namespace_uuid, namespace_str);



	spdk_uuid_generate_sha1(&new_uuid, &namespace_uuid, merged_str, size);

	rc = spdk_uuid_generate_sha1(&new_uuid, &namespace_uuid, merged_str, size);

	if (rc == 0) {

		memcpy(uuid, &new_uuid, sizeof(struct spdk_uuid));

	}



	return new_uuid;

	return rc;

}



static int
@@ -4006,6 +4012,7 @@ nvme_disk_create(struct spdk_bdev *disk, const char *base_name, 
	enum spdk_nvme_csi		csi;

	uint32_t atomic_bs, phys_bs, bs;

	char sn_tmp[SPDK_NVME_CTRLR_SN_LEN + 1] = {'\0'};

	int rc;



	cdata = spdk_nvme_ctrlr_get_data(ctrlr);

	csi = spdk_nvme_ns_get_csi(ns);
@@ -4036,7 +4043,11 @@ nvme_disk_create(struct spdk_bdev *disk, const char *base_name, 
			disk->uuid = *uuid;

		} else if (g_opts.generate_uuids) {

			spdk_strcpy_pad(sn_tmp, cdata->sn, SPDK_NVME_CTRLR_SN_LEN, '\0');

			disk->uuid = nvme_generate_uuid(sn_tmp, spdk_nvme_ns_get_id(ns));

			rc = nvme_generate_uuid(sn_tmp, spdk_nvme_ns_get_id(ns), &disk->uuid);

			if (rc != 0) {

				SPDK_ERRLOG("UUID generation failed (%s)\n", strerror(rc));

				return rc;

			}

		}

	} else {

		memcpy(&disk->uuid, nguid, sizeof(disk->uuid));

test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c

+12 −5
Original line number Diff line number Diff line
@@ -6375,28 +6375,35 @@ test_uuid_generation(void) 
	char sn3[21] = "                    ";

	char uuid_str[SPDK_UUID_STRING_LEN] = {'\0'};

	struct spdk_uuid uuid1, uuid2;

	int rc;



	/* Test case 1:

	 * Serial numbers are the same, nsids are different.

	 * Compare two generated UUID - they should be different. */

	uuid1 = nvme_generate_uuid(sn1, nsid1);

	uuid2 = nvme_generate_uuid(sn1, nsid2);

	rc = nvme_generate_uuid(sn1, nsid1, &uuid1);

	CU_ASSERT(rc == 0);

	rc = nvme_generate_uuid(sn1, nsid2, &uuid2);

	CU_ASSERT(rc == 0);



	CU_ASSERT((spdk_uuid_compare(&uuid1, &uuid2)) != 0);



	/* Test case 2:

	 * Serial numbers differ only by one character, nsids are the same.

	 * Compare two generated UUID - they should be different. */

	uuid1 = nvme_generate_uuid(sn1, nsid1);

	uuid2 = nvme_generate_uuid(sn2, nsid1);

	rc = nvme_generate_uuid(sn1, nsid1, &uuid1);

	CU_ASSERT(rc == 0);

	rc = nvme_generate_uuid(sn2, nsid1, &uuid2);

	CU_ASSERT(rc == 0);



	CU_ASSERT((spdk_uuid_compare(&uuid1, &uuid2)) != 0);



	/* Test case 3:

	 * Serial number comprises only of space characters.

	 * Validate the generated UUID. */

	uuid1 = nvme_generate_uuid(sn3, nsid1);

	rc = nvme_generate_uuid(sn3, nsid1, &uuid1);

	CU_ASSERT(rc == 0);

	CU_ASSERT((spdk_uuid_fmt_lower(uuid_str, sizeof(uuid_str), &uuid1)) == 0);



}



static void