diff mbox series

[PULL,18/30] hw/block/nvme: update nsid when registered

Message ID 20201027104932.558087-19-its@irrelevant.dk
State New
Headers show
Series [PULL,01/30] hw/block/nvme: fix typo in trace event | expand

Commit Message

Klaus Jensen Oct. 27, 2020, 10:49 a.m. UTC 
From: Klaus Jensen <k.jensen@samsung.com>

If the user does not specify an nsid parameter on the nvme-ns device,
nvme_register_namespace will find the first free namespace id and assign
that.

This fix makes sure the assigned id is saved.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 hw/block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Max Reitz Nov. 4, 2020, 9:32 a.m. UTC | #1 
On 27.10.20 11:49, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> If the user does not specify an nsid parameter on the nvme-ns device,
> nvme_register_namespace will find the first free namespace id and assign
> that.
> 
> This fix makes sure the assigned id is saved.
> 
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> ---
>  hw/block/nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5768a6804f41..2225b944f935 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -2578,7 +2578,7 @@ int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
>          for (int i = 1; i <= n->num_namespaces; i++) {
>              NvmeNamespace *ns = nvme_ns(n, i);
>              if (!ns) {
> -                nsid = i;
> +                nsid = ns->params.nsid = i;

Coverity reports that @ns is NULL here.  I think the problem is that we
want to access the *ns given to nvme_register_namespace() here, but it’s
shadowed by another @ns in this for () loop.

Max

>                  break;
>              }
>          }
>
Klaus Jensen Nov. 4, 2020, 9:52 a.m. UTC | #2 
On Nov  4 10:32, Max Reitz wrote:
> On 27.10.20 11:49, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > If the user does not specify an nsid parameter on the nvme-ns device,
> > nvme_register_namespace will find the first free namespace id and assign
> > that.
> > 
> > This fix makes sure the assigned id is saved.
> > 
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > ---
> >  hw/block/nvme.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > index 5768a6804f41..2225b944f935 100644
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -2578,7 +2578,7 @@ int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
> >          for (int i = 1; i <= n->num_namespaces; i++) {
> >              NvmeNamespace *ns = nvme_ns(n, i);
> >              if (!ns) {
> > -                nsid = i;
> > +                nsid = ns->params.nsid = i;
> 
> Coverity reports that @ns is NULL here.  I think the problem is that we
> want to access the *ns given to nvme_register_namespace() here, but it’s
> shadowed by another @ns in this for () loop.
> 

Sure enough. Thanks!

I'll send a fix.
diff mbox series

Patch

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5768a6804f41..2225b944f935 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2578,7 +2578,7 @@  int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
         for (int i = 1; i <= n->num_namespaces; i++) {
             NvmeNamespace *ns = nvme_ns(n, i);
             if (!ns) {
-                nsid = i;
+                nsid = ns->params.nsid = i;
                 break;
             }
         }