[PATCH 4/4] nvme: check that EUI/GUID/UUID are globally unique

[Alan Adamson]

I ran into an issue with blktests and my qemu setup when passthru is enabled.  The passthru tests
do not complete.  This was because the UUID for the passthru device is coming from the a device
from the same system since the fabric was setup as a loop and nvme_global_check_duplicate_ids() fails.

This is probably not a valid real life configuration, but since blktests try to test fabrics on a single
system, we have this issue.

I’ve hacked together a fix to manipulate Namespace Identifier’s to get the test to work.  Is there a
why for blktests to hardcode the IDs for the passthru devices?

Alan

diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 5247c24538eb..e3db53b232e6 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -159,6 +159,40 @@ static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
 	return status;
 }
 
+static u16 nvmet_passthru_override_ns_id_desc(struct nvmet_req *req)
+{
+	u16 status = NVME_SC_SUCCESS;
+	char id_desc[2];
+	int i = 0;
+
+	status = nvmet_copy_from_sgl(req, i, id_desc, 2);
+	if (status)
+		return status;
+
+	while (id_desc[1]) {
+		int len;
+
+		len = id_desc[1];
+		i += 4;
+
+		if ((id_desc[0] == 1) || (id_desc[0] == 2) || (id_desc[0] == 3)) {
+			status = nvmet_copy_from_sgl(req, i, id_desc, 2);
+			if (status)
+				return status;
+			id_desc[0]++; /* Change 1st character of the NID */
+			status = nvmet_copy_to_sgl(req, i, id_desc, 2);
+			if (status)
+				return status;
+		} 
+		i += len;
+		status = nvmet_copy_from_sgl(req, i, id_desc, 2);
+		if (status)
+			return status;
+	}
+	return status;
+}
+
+
 static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
 {
 	struct nvmet_req *req = container_of(w, struct nvmet_req, p.work);
@@ -176,7 +210,11 @@ static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
 		case NVME_ID_CNS_NS:
 			nvmet_passthru_override_id_ns(req);
 			break;
+		case NVME_ID_CNS_NS_DESC_LIST:
+			nvmet_passthru_override_ns_id_desc(req);
+			break;
 		}
+
 	} else if (status < 0)
 		status = NVME_SC_INTERNAL;
 
-- 



> On Apr 10, 2022, at 10:54 PM, Christoph Hellwig <hch at lst.de> wrote:
> 
> On Mon, Apr 11, 2022 at 07:05:33AM +0200, Christoph Hellwig wrote:
>>> However, what I'm seeing seems to show that the uuid is same uuid as
>>> well when not using -device nvme-ns but just -device nvme (this is
>>> called legacy now it seems?) without the uuid set you end up in the
>>> situation I described. I just destroyed my guests and started from
>>> scratch a set up using qemu-system-x86_64 v6.2.0 on debian-testing,
>>> and end up in a different situation but it is still a bit perplexing.
>> 
>> With my usual qemu test setup (built from a git a few weeks ago), no
>> uuid shows up unless explicitly set.
> 
> Digging a bit deeper this was "fixed" by:
> 
> 5f4884c4412318a1adc105dea9cc28f7625ce730
> Author: Klaus Jensen <k.jensen at samsung.com>
> Date:   Mon Aug 9 12:34:40 2021 +0200
> 
>    hw/nvme: fix missing variable initializers
> 
>    Coverity found that 'uuid', 'csi' and 'eui64' are uninitialized.
>    While we set most of the fields, we do not explicitly set the rsvd2
>    field in the NvmeIdNsDescr header.
> 		         
>    Fix this by explicitly zero-initializing the variables.
> 
> Note that even with the fix the uuid field is always reported, even
> when it shouldn't - it just is that Linux handles a 0 UUID gracefully.
> 
> I can also not find any code that would assign a different uuid
> when using a different command line syntax, but given how unusable
> the new syntax is I've not actually been able to try it.
> 
> So I think for now we'll just need to disable identifier on qemu.
> 
> It would be great if qemu could switch to a new PCI ID after this is
> fixed as that simplifies the quirking.
>