On Apr 4 10:52, alan.adam...@oracle.com wrote: > I'm running into a issue with the latest qemu-nvme with v10.0.0-rc2 with > regards to multiple controllers behind a subsystem. When I setup a > subsystem with 2 controllers, each with a private/non-shared namespace, the > two private/non-shared namespaces all get attached to one of the > controllers. > > I'm sending out diffs that resolve the problem but would like to get some > feedback before sending a formal patch. >
Hi Alan,
Thanks for reporting this! This is definitely a regression caused by the
csi refactoring I did.
Few comments below, but I'd like to try to get this into 10.0. Timeline
is tight, so I'll send out a patch with my suggestings below.
> @@ -7751,17 +7752,23 @@ static int nvme_start_ctrl(NvmeCtrl *n)
>
> nvme_set_timestamp(n, 0ULL);
>
> - /* verify that the command sets of attached namespaces are supported */
> - for (int i = 1; i <= NVME_MAX_NAMESPACES; i++) {
> - NvmeNamespace *ns = nvme_subsys_ns(n->subsys, i);
> + if (n->subsys) {
> + for (int i = 1; i <= NVME_MAX_NAMESPACES; i++) {
> + NvmeNamespace *ns = n->subsys->namespaces[i].namespace;
>
> - if (ns && nvme_csi_supported(n, ns->csi) && !ns->params.detached) {
> - if (!ns->attached || ns->params.shared) {
> - nvme_attach_ns(n, ns);
> + if (!ns) {
> + continue;
> }
> + if (!(n->subsys->namespaces[i].ctrl == n) && !ns->params.shared) {
> + continue;
> + }
> + if (nvme_csi_supported(n, ns->csi) && !ns->params.detached) {
> + if (!ns->attached || ns->params.shared) {
> + nvme_attach_ns(n, ns);
> + }
> + }
> }
> }
> -
> nvme_update_dsm_limits(n, NULL);
>
> return 0;
Yeah, this is where things went wrong.
> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> index 6f782ba18826..bea3b96a6dfa 100644
> --- a/hw/nvme/nvme.h
> +++ b/hw/nvme/nvme.h
> @@ -97,6 +97,11 @@ typedef struct NvmeEnduranceGroup {
> } fdp;
> } NvmeEnduranceGroup;
>
> +typedef struct Namespaces {
> + NvmeCtrl *ctrl;
> + NvmeNamespace *namespace;
> +} Namespaces;
Let's just add an NvmeCtrl* in struct NvmeNamespace. If set, the
namespace is private.
> diff --git a/hw/nvme/subsys.c b/hw/nvme/subsys.c
> index 2ae56f12a596..d5751564c05c 100644
> --- a/hw/nvme/subsys.c
> +++ b/hw/nvme/subsys.c
> @@ -92,13 +92,19 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
>
> subsys->ctrls[cntlid] = n;
>
> - for (nsid = 1; nsid < ARRAY_SIZE(subsys->namespaces); nsid++) {
> - NvmeNamespace *ns = subsys->namespaces[nsid];
> - if (ns && ns->params.shared && !ns->params.detached) {
> - nvme_attach_ns(n, ns);
> + for (nsid = 1; nsid <= NVME_MAX_NAMESPACES; nsid++) {
> + NvmeNamespace *ns = subsys->namespaces[nsid].namespace;
> +
> + if (!ns) {
> + continue;
> + }
> + if (!(subsys->namespaces[nsid].ctrl == n) && !ns->params.shared) {
> + continue;
> }
> + if (ns->params.shared && !ns->params.detached) {
> + nvme_attach_ns(n, ns);
> + }
> }
> -
> return cntlid;
> }
The code here that attach namespaces when the controller is registers
actually need to go away. This is a leftover.
signature.asc
Description: PGP signature
