| Message ID | 20210909095159.122174-1-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/nvme: select first free NSID for legacy drive configuration | expand |
On Sep 9 11:51, Hannes Reinecke wrote: > If a legacy 'drive' argument is passed to the controller we cannot > assume that '1' will be a free NSID, as the subsys might already > have attached a namespace to this NSID. So select the first free > one. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > hw/nvme/ctrl.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > index 757cdff038..2c69031ca9 100644 > --- a/hw/nvme/ctrl.c > +++ b/hw/nvme/ctrl.c > @@ -6546,8 +6546,15 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) > > /* setup a namespace if the controller drive property was given */ > if (n->namespace.blkconf.blk) { > + int i; > ns = &n->namespace; > - ns->params.nsid = 1; > + for (i = 1; i <= NVME_MAX_NAMESPACES; i++) { > + if (nvme_ns(n, i) || nvme_subsys_ns(n->subsys, i)) { > + continue; > + } > + ns->params.nsid = i; > + break; > + } > > if (nvme_ns_setup(ns, errp)) { > return; > -- > 2.26.2 > Did you actually hit this? Because then then property checking logic is bad... The device is not supposed to allow nvme,drive= combined with a subsystem property. In nvme_check_constraints(): if (n->namespace.blkconf.blk && n->subsys) { /* error out */ return; }
On 9/9/21 12:52 PM, Klaus Jensen wrote: > On Sep 9 11:51, Hannes Reinecke wrote: >> If a legacy 'drive' argument is passed to the controller we cannot >> assume that '1' will be a free NSID, as the subsys might already >> have attached a namespace to this NSID. So select the first free >> one. >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> hw/nvme/ctrl.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c >> index 757cdff038..2c69031ca9 100644 >> --- a/hw/nvme/ctrl.c >> +++ b/hw/nvme/ctrl.c >> @@ -6546,8 +6546,15 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) >> >> /* setup a namespace if the controller drive property was given */ >> if (n->namespace.blkconf.blk) { >> + int i; >> ns = &n->namespace; >> - ns->params.nsid = 1; >> + for (i = 1; i <= NVME_MAX_NAMESPACES; i++) { >> + if (nvme_ns(n, i) || nvme_subsys_ns(n->subsys, i)) { >> + continue; >> + } >> + ns->params.nsid = i; >> + break; >> + } >> >> if (nvme_ns_setup(ns, errp)) { >> return; >> -- >> 2.26.2 >> > > Did you actually hit this? > > Because then then property checking logic is bad... The device is not > supposed to allow nvme,drive= combined with a subsystem property. In > nvme_check_constraints(): > > if (n->namespace.blkconf.blk && n->subsys) { > /* error out */ > return; > } > > Ah. Missed that. Do ignore my patch then. Cheers, Hannes
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 757cdff038..2c69031ca9 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -6546,8 +6546,15 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) /* setup a namespace if the controller drive property was given */ if (n->namespace.blkconf.blk) { + int i; ns = &n->namespace; - ns->params.nsid = 1; + for (i = 1; i <= NVME_MAX_NAMESPACES; i++) { + if (nvme_ns(n, i) || nvme_subsys_ns(n->subsys, i)) { + continue; + } + ns->params.nsid = i; + break; + } if (nvme_ns_setup(ns, errp)) { return;
If a legacy 'drive' argument is passed to the controller we cannot assume that '1' will be a free NSID, as the subsys might already have attached a namespace to this NSID. So select the first free one. Signed-off-by: Hannes Reinecke <hare@suse.de> --- hw/nvme/ctrl.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)