Re: [PATCH 08/14] nvmet: implement the changed namespaces log

[Christoph Hellwig <hch@lst.de>]

On Tue, May 29, 2018 at 04:59:05PM +0000, Verkamp, Daniel wrote:
> > +	} else if (ctrl->nr_changed_ns == NVME_MAX_CHANGED_NAMESPACES) {
> > +		ctrl->changed_ns_list[0] = cpu_to_le32(0xffffffff);
> > +	}
> 
> Unless I'm missing it happening somewhere else, the list-full case that sets element 0 to 0xffffffff should also explicitly zero out the rest of the list to satisfy the "remainder of the list shall be zero-filled" wording in the spec, since the other changed_ns_list entries will be filled with non-zero NSIDs when we get here.

True.  We actually zero out unused elements already, but that doesn't
catch the ctrl->nr_changed_ns == NVME_MAX_CHANGED_NAMESPACES special
case.  This relative patch should fix it:

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index d7b6293e830b..7b69c348d608 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -136,7 +136,10 @@ static void nvmet_execute_get_log_changed_ns(struct nvmet_req *req)
 		goto out;
 
 	mutex_lock(&ctrl->lock);
-	len = ctrl->nr_changed_ns * sizeof(__le32);
+	if (ctrl->nr_changed_ns == NVME_MAX_CHANGED_NAMESPACES)
+		len = sizeof(__le32);
+	else
+		len = ctrl->nr_changed_ns * sizeof(__le32);
 	status = nvmet_copy_to_sgl(req, 0, ctrl->changed_ns_list, len);
 	if (!status)
 		status = nvmet_zero_sgl(req, len, req->data_len - len);