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

[Christoph Hellwig]

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);