qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns
@ 2021-01-21 22:09 Minwoo Im
  2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 UTC (permalink / raw)
  To: qemu-devel, qemu-block
  Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz

Hello,

This series is fourth series for the support of NVMe subsystem scheme
with multi-controller and namespace sharing in a subsystem.

This time, I've removed 'detached' scheme introduced in the previous
series out of this series to focus on subsystem scheme in ths series.
The attach/detach scheme will be introduced in the next series with
ns-mgmt functionality.

Here's an example of how-to:

  # Specify a subsystem
  -device nvme-subsys,id=subsys0 \
  -device nvme,serial=foo,id=nvme0,subsys=subsys0 \
  -device nvme,serial=bar,id=nvme1,subsys=subsys0 \
  -device nvme,serial=baz,id=nvme2,subsys=subsys0 \
  -device nvme-ns,id=ns1,drive=drv10,nsid=1,subsys=subsys0 \
  -device nvme-ns,id=ns2,drive=drv11,nsid=2,bus=nvme2 \
  \
  # Not specify a subsystem
  -device nvme,serial=qux,id=nvme3 \
  -device nvme-ns,id=ns3,drive=drv12,nsid=3,bus=nvme3 \

# nvme list -v
  NVM Express Subsystems

  Subsystem        Subsystem-NQN                                                                                    Controllers
  ---------------- ------------------------------------------------------------------------------------------------ ----------------
  nvme-subsys1     nqn.2019-08.org.qemu:subsys0                                                                     nvme0, nvme1, nvme2
  nvme-subsys3     nqn.2019-08.org.qemu:qux                                                                         nvme3

  NVM Express Controllers

  Device   SN                   MN                                       FR       TxPort Address        Subsystem    Namespaces      
  -------- -------------------- ---------------------------------------- -------- ------ -------------- ------------ ----------------
  nvme0    foo                  QEMU NVMe Ctrl                           1.0      pcie   0000:00:06.0   nvme-subsys1 nvme1c0n1
  nvme1    bar                  QEMU NVMe Ctrl                           1.0      pcie   0000:00:07.0   nvme-subsys1 nvme1c1n1
  nvme2    baz                  QEMU NVMe Ctrl                           1.0      pcie   0000:00:08.0   nvme-subsys1 nvme1c2n1, nvme1c2n2
  nvme3    qux                  QEMU NVMe Ctrl                           1.0      pcie   0000:00:09.0   nvme-subsys3 nvme3c3n1

  NVM Express Namespaces

  Device       NSID     Usage                      Format           Controllers     
  ------------ -------- -------------------------- ---------------- ----------------
  nvme1n1      1        134.22  MB / 134.22  MB    512   B +  0 B   nvme0, nvme1, nvme2
  nvme1n2      2        268.44  MB / 268.44  MB    512   B +  0 B   nvme2
  nvme3n1      3        268.44  MB / 268.44  MB    512   B +  0 B   nvme3

Thanks,

Since RFC V3:
  - Exclude 'deatched' scheme from this series.  This will be covered in
    the next series by covering all the ns-related admin commands
    including ZNS and ns-mgmt. (Niklas)
  - Rebased on nvme-next.
  - Remove RFC tag from this V4.

Since RFC V2:
  - Rebased on nvme-next branch with trivial patches from the previous
    version(V2) applied. (Klaus)
  - Fix enumeration type name convention with NvmeIdNs prefix. (Klaus)
  - Put 'cntlid' to NvmeCtrl instance in nvme_init_ctrl() which was
    missed in V2.
  - Added 'detached' parameter to nvme-ns device to decide whether to
    attach or not to controller(s) in the subsystem. (Klaus)
  - Implemented Identify Active Namespace ID List aprt from Identify
    Allocated Namespace ID List by removing fall-thru statement.

Since RFC V1:
  - Updated namespace sharing scheme to be based on nvme-subsys
    hierarchy.

Minwoo Im (6):
  hw/block/nvme: introduce nvme-subsys device
  hw/block/nvme: support to map controller to a subsystem
  hw/block/nvme: add CMIC enum value for Identify Controller
  hw/block/nvme: support for multi-controller in subsystem
  hw/block/nvme: add NMIC enum value for Identify Namespace
  hw/block/nvme: support for shared namespace in subsystem

 hw/block/meson.build   |   2 +-
 hw/block/nvme-ns.c     |  23 +++++++--
 hw/block/nvme-ns.h     |   7 +++
 hw/block/nvme-subsys.c | 109 +++++++++++++++++++++++++++++++++++++++++
 hw/block/nvme-subsys.h |  32 ++++++++++++
 hw/block/nvme.c        |  77 ++++++++++++++++++++++++++---
 hw/block/nvme.h        |   4 ++
 include/block/nvme.h   |   8 +++
 8 files changed, 249 insertions(+), 13 deletions(-)
 create mode 100644 hw/block/nvme-subsys.c
 create mode 100644 hw/block/nvme-subsys.h

-- 
2.17.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device
  2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
  2021-01-21 22:52   ` Keith Busch
  2021-01-21 22:09 ` [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 UTC (permalink / raw)
  To: qemu-devel, qemu-block
  Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz

To support multi-path in QEMU NVMe device model, We need to have NVMe
subsystem hierarchy to map controllers and namespaces to a NVMe
subsystem.

This patch introduced a simple nvme-subsys device model.  The subsystem
will be prepared with subsystem NQN with <subsys_id> provided in
nvme-subsys device:

  ex) -device nvme-subsys,id=subsys0: nqn.2019-08.org.qemu:subsys0

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 hw/block/meson.build   |  2 +-
 hw/block/nvme-subsys.c | 63 ++++++++++++++++++++++++++++++++++++++++++
 hw/block/nvme-subsys.h | 25 +++++++++++++++++
 hw/block/nvme.c        |  3 ++
 4 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 hw/block/nvme-subsys.c
 create mode 100644 hw/block/nvme-subsys.h

diff --git a/hw/block/meson.build b/hw/block/meson.build
index 602ca6c8541d..83ea2d37978d 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -13,7 +13,7 @@ softmmu_ss.add(when: 'CONFIG_SSI_M25P80', if_true: files('m25p80.c'))
 softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
 softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('tc58128.c'))
-softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c'))
+softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c', 'nvme-subsys.c'))
 
 specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
 specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c'))
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
new file mode 100644
index 000000000000..f1dc71d588d9
--- /dev/null
+++ b/hw/block/nvme-subsys.c
@@ -0,0 +1,63 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im <minwoo.im.dev@gmail.com>
+ *
+ * This code is licensed under the GNU GPL v2.  Refer COPYING.
+ */
+
+#include "qemu/units.h"
+#include "qemu/osdep.h"
+#include "qemu/uuid.h"
+#include "qemu/iov.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-core.h"
+#include "hw/block/block.h"
+#include "block/aio.h"
+#include "block/accounting.h"
+#include "sysemu/sysemu.h"
+#include "hw/pci/pci.h"
+#include "nvme.h"
+#include "nvme-subsys.h"
+
+static void nvme_subsys_setup(NvmeSubsystem *subsys)
+{
+    char *subnqn;
+
+    subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
+    strpadcpy((char *)subsys->subnqn, sizeof(subsys->subnqn), subnqn, '\0');
+    g_free(subnqn);
+}
+
+static void nvme_subsys_realize(DeviceState *dev, Error **errp)
+{
+    NvmeSubsystem *subsys = NVME_SUBSYS(dev);
+
+    nvme_subsys_setup(subsys);
+}
+
+static void nvme_subsys_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+
+    dc->realize = nvme_subsys_realize;
+    dc->desc = "Virtual NVMe subsystem";
+}
+
+static const TypeInfo nvme_subsys_info = {
+    .name = TYPE_NVME_SUBSYS,
+    .parent = TYPE_DEVICE,
+    .class_init = nvme_subsys_class_init,
+    .instance_size = sizeof(NvmeSubsystem),
+};
+
+static void nvme_subsys_register_types(void)
+{
+    type_register_static(&nvme_subsys_info);
+}
+
+type_init(nvme_subsys_register_types)
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
new file mode 100644
index 000000000000..40f06a4c7db0
--- /dev/null
+++ b/hw/block/nvme-subsys.h
@@ -0,0 +1,25 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im <minwoo.im.dev@gmail.com>
+ *
+ * This code is licensed under the GNU GPL v2.  Refer COPYING.
+ */
+
+#ifndef NVME_SUBSYS_H
+#define NVME_SUBSYS_H
+
+#define TYPE_NVME_SUBSYS "nvme-subsys"
+#define NVME_SUBSYS(obj) \
+    OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
+
+#define NVME_SUBSYS_MAX_CTRLS   32
+
+typedef struct NvmeCtrl NvmeCtrl;
+typedef struct NvmeNamespace NvmeNamespace;
+typedef struct NvmeSubsystem {
+    DeviceState parent_obj;
+    uint8_t     subnqn[256];
+} NvmeSubsystem;
+
+#endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 21aec90637fa..aabccdf36f4b 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -25,6 +25,7 @@
  *              mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
  *      -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
  *              zoned=<true|false[optional]>
+ *      -device nvme-subsys,id=<subsys_id>
  *
  * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
  * offset 0 in BAR2 and supports only WDS, RDS and SQS for now. By default, the
@@ -38,6 +39,8 @@
  *
  * The PMR will use BAR 4/5 exclusively.
  *
+ * To place controller(s) and namespace(s) to a subsystem, then provide
+ * nvme-subsys device as above.
  *
  * nvme device parameters
  * ~~~~~~~~~~~~~~~~~~~~~~
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem
  2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
  2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
  2021-01-21 23:03   ` Keith Busch
  2021-01-21 22:09 ` [PATCH V4 3/6] hw/block/nvme: add CMIC enum value for Identify Controller Minwoo Im
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 UTC (permalink / raw)
  To: qemu-devel, qemu-block
  Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz

nvme controller(nvme) can be mapped to a NVMe subsystem(nvme-subsys).
This patch maps a controller to a subsystem by adding a parameter
'subsys' to the nvme device.

To map a controller to a subsystem, we need to put nvme-subsys first and
then maps the subsystem to the controller:

  -device nvme-subsys,id=subsys0
  -device nvme,serial=foo,id=nvme0,subsys=subsys0

If 'subsys' property is not given to the nvme controller, then subsystem
NQN will be created with serial (e.g., 'foo' in above example),
Otherwise, it will be based on subsys id (e.g., 'subsys0' in above
example).

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 hw/block/nvme.c | 30 ++++++++++++++++++++++++++----
 hw/block/nvme.h |  3 +++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index aabccdf36f4b..ab0531492ddd 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -23,6 +23,7 @@
  *              max_ioqpairs=<N[optional]>, \
  *              aerl=<N[optional]>, aer_max_queued=<N[optional]>, \
  *              mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
+ *              ,subsys=<subsys_id> \
  *      -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
  *              zoned=<true|false[optional]>
  *      -device nvme-subsys,id=<subsys_id>
@@ -44,6 +45,13 @@
  *
  * nvme device parameters
  * ~~~~~~~~~~~~~~~~~~~~~~
+ * - `subsys`
+ *   NVM Subsystem device. If given, a subsystem NQN will be initialized with
+ *   <subsys_id> given. Otherwise, <serial> will be taken for subsystem NQN.
+ *   Also, it will enable multi controller capability represented in Identify
+ *   Controller data structure in CMIC (Controller Multi-path I/O and Namesapce
+ *   Sharing Capabilities), if given.
+ *
  * - `aerl`
  *   The Asynchronous Event Request Limit (AERL). Indicates the maximum number
  *   of concurrently outstanding Asynchronous Event Request commands support
@@ -4404,11 +4412,25 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
     return 0;
 }
 
+static void nvme_init_subnqn(NvmeCtrl *n)
+{
+    NvmeSubsystem *subsys = n->subsys;
+    NvmeIdCtrl *id = &n->id_ctrl;
+    char *subnqn;
+
+    if (!subsys) {
+        subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", n->params.serial);
+        strpadcpy((char *)id->subnqn, sizeof(id->subnqn), subnqn, '\0');
+        g_free(subnqn);
+    } else {
+        pstrcpy((char *)id->subnqn, sizeof(id->subnqn), (char*)subsys->subnqn);
+    }
+}
+
 static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
 {
     NvmeIdCtrl *id = &n->id_ctrl;
     uint8_t *pci_conf = pci_dev->config;
-    char *subnqn;
 
     id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
     id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
@@ -4455,9 +4477,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN |
                            NVME_CTRL_SGLS_BITBUCKET);
 
-    subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", n->params.serial);
-    strpadcpy((char *)id->subnqn, sizeof(id->subnqn), subnqn, '\0');
-    g_free(subnqn);
+    nvme_init_subnqn(n);
 
     id->psd[0].mp = cpu_to_le16(0x9c4);
     id->psd[0].enlat = cpu_to_le32(0x10);
@@ -4545,6 +4565,8 @@ static Property nvme_props[] = {
     DEFINE_BLOCK_PROPERTIES(NvmeCtrl, namespace.blkconf),
     DEFINE_PROP_LINK("pmrdev", NvmeCtrl, pmr.dev, TYPE_MEMORY_BACKEND,
                      HostMemoryBackend *),
+    DEFINE_PROP_LINK("subsys", NvmeCtrl, subsys, TYPE_NVME_SUBSYS,
+                     NvmeSubsystem *),
     DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
     DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
     DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index dee6092bd45f..04d4684601fd 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -2,6 +2,7 @@
 #define HW_NVME_H
 
 #include "block/nvme.h"
+#include "nvme-subsys.h"
 #include "nvme-ns.h"
 
 #define NVME_MAX_NAMESPACES 256
@@ -170,6 +171,8 @@ typedef struct NvmeCtrl {
 
     uint8_t     zasl;
 
+    NvmeSubsystem   *subsys;
+
     NvmeNamespace   namespace;
     NvmeNamespace   *namespaces[NVME_MAX_NAMESPACES];
     NvmeSQueue      **sq;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 3/6] hw/block/nvme: add CMIC enum value for Identify Controller
  2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
  2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
  2021-01-21 22:09 ` [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
  2021-01-21 22:09 ` [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 UTC (permalink / raw)
  To: qemu-devel, qemu-block
  Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz

Added Controller Multi-path I/O and Namespace Sharing Capabilities
(CMIC) field to support multi-controller in the following patches.

This field is in Identify Controller data structure in [76].

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 include/block/nvme.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index e4b918064df9..d6415a869c1c 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1034,6 +1034,10 @@ enum NvmeIdCtrlLpa {
     NVME_LPA_EXTENDED = 1 << 2,
 };
 
+enum NvmeIdCtrlCmic {
+    NVME_CMIC_MULTI_CTRL    = 1 << 1,
+};
+
 #define NVME_CTRL_SQES_MIN(sqes) ((sqes) & 0xf)
 #define NVME_CTRL_SQES_MAX(sqes) (((sqes) >> 4) & 0xf)
 #define NVME_CTRL_CQES_MIN(cqes) ((cqes) & 0xf)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem
  2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
                   ` (2 preceding siblings ...)
  2021-01-21 22:09 ` [PATCH V4 3/6] hw/block/nvme: add CMIC enum value for Identify Controller Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
  2021-01-21 23:17   ` Keith Busch
  2021-01-21 22:09 ` [PATCH V4 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace Minwoo Im
  2021-01-21 22:09 ` [PATCH V4 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im
  5 siblings, 1 reply; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 UTC (permalink / raw)
  To: qemu-devel, qemu-block
  Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz

We have nvme-subsys and nvme devices mapped together.  To support
multi-controller scheme to this setup, controller identifier(id) has to
be managed.  Earlier, cntlid(controller id) used to be always 0 because
we didn't have any subsystem scheme that controller id matters.

This patch introduced 'cntlid' attribute to the nvme controller
instance(NvmeCtrl) and make it allocated by the nvme-subsys device
mapped to the controller.  If nvme-subsys is not given to the
controller, then it will always be 0 as it was.

Added 'ctrls' array in the nvme-subsys instance to manage attached
controllers to the subsystem with a limit(32).  This patch didn't take
list for the controllers to make it seamless with nvme-ns device.

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 hw/block/nvme-subsys.c | 21 +++++++++++++++++++++
 hw/block/nvme-subsys.h |  4 ++++
 hw/block/nvme.c        | 34 ++++++++++++++++++++++++++++++++--
 hw/block/nvme.h        |  1 +
 4 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index f1dc71d588d9..a01003136b12 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -22,6 +22,27 @@
 #include "nvme.h"
 #include "nvme-subsys.h"
 
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
+{
+    NvmeSubsystem *subsys = n->subsys;
+    int cntlid;
+
+    for (cntlid = 0; cntlid < ARRAY_SIZE(subsys->ctrls); cntlid++) {
+        if (!subsys->ctrls[cntlid]) {
+            break;
+        }
+    }
+
+    if (cntlid == ARRAY_SIZE(subsys->ctrls)) {
+        error_setg(errp, "no more free controller id");
+        return -1;
+    }
+
+    subsys->ctrls[cntlid] = n;
+
+    return cntlid;
+}
+
 static void nvme_subsys_setup(NvmeSubsystem *subsys)
 {
     char *subnqn;
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 40f06a4c7db0..4eba50d96a1d 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -20,6 +20,10 @@ typedef struct NvmeNamespace NvmeNamespace;
 typedef struct NvmeSubsystem {
     DeviceState parent_obj;
     uint8_t     subnqn[256];
+
+    NvmeCtrl    *ctrls[NVME_SUBSYS_MAX_CTRLS];
 } NvmeSubsystem;
 
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
+
 #endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index ab0531492ddd..225f0d3f3a27 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4427,16 +4427,21 @@ static void nvme_init_subnqn(NvmeCtrl *n)
     }
 }
 
-static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
+static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t cntlid)
 {
     NvmeIdCtrl *id = &n->id_ctrl;
     uint8_t *pci_conf = pci_dev->config;
 
+    n->cntlid = cntlid;
+
     id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
     id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
     strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
     strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
     strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
+
+    id->cntlid = cntlid;
+
     id->rab = 6;
     id->ieee[0] = 0x00;
     id->ieee[1] = 0x02;
@@ -4483,6 +4488,10 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     id->psd[0].enlat = cpu_to_le32(0x10);
     id->psd[0].exlat = cpu_to_le32(0x4);
 
+    if (n->subsys) {
+        id->cmic |= NVME_CMIC_MULTI_CTRL;
+    }
+
     NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
     NVME_CAP_SET_CQR(n->bar.cap, 1);
     NVME_CAP_SET_TO(n->bar.cap, 0xf);
@@ -4497,11 +4506,28 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     n->bar.intmc = n->bar.intms = 0;
 }
 
+static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
+{
+    int cntlid;
+
+    if (!n->subsys) {
+        return 0;
+    }
+
+    cntlid = nvme_subsys_register_ctrl(n, errp);
+    if (cntlid < 0) {
+        return -1;
+    }
+
+    return cntlid;
+}
+
 static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 {
     NvmeCtrl *n = NVME(pci_dev);
     NvmeNamespace *ns;
     Error *local_err = NULL;
+    int cntlid;
 
     nvme_check_constraints(n, &local_err);
     if (local_err) {
@@ -4517,7 +4543,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
-    nvme_init_ctrl(n, pci_dev);
+    cntlid = nvme_init_subsys(n, errp);
+    if (cntlid < 0) {
+        return;
+    }
+    nvme_init_ctrl(n, pci_dev, cntlid);
 
     /* setup a namespace if the controller drive property was given */
     if (n->namespace.blkconf.blk) {
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 04d4684601fd..b8f5f2d6ffb8 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -134,6 +134,7 @@ typedef struct NvmeCtrl {
     NvmeBus      bus;
     BlockConf    conf;
 
+    uint16_t    cntlid;
     bool        qs_created;
     uint32_t    page_size;
     uint16_t    page_bits;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace
  2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
                   ` (3 preceding siblings ...)
  2021-01-21 22:09 ` [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
  2021-01-21 22:09 ` [PATCH V4 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im
  5 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 UTC (permalink / raw)
  To: qemu-devel, qemu-block
  Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz

Added Namespace Multi-path I/O and Namespace Sharing Capabilities (NMIC)
field to support shared namespace from controller(s).

This field is in Identify Namespace data structure in [30].

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 include/block/nvme.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index d6415a869c1c..ad68cdc2b92d 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1203,6 +1203,10 @@ enum NvmeNsIdentifierType {
     NVME_NIDT_CSI               = 0x04,
 };
 
+enum NvmeIdNsNmic {
+    NVME_NMIC_NS_SHARED         = 1 << 0,
+};
+
 enum NvmeCsi {
     NVME_CSI_NVM                = 0x00,
     NVME_CSI_ZONED              = 0x02,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 6/6] hw/block/nvme: support for shared namespace in subsystem
  2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
                   ` (4 preceding siblings ...)
  2021-01-21 22:09 ` [PATCH V4 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace Minwoo Im
@ 2021-01-21 22:09 ` Minwoo Im
  5 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 22:09 UTC (permalink / raw)
  To: qemu-devel, qemu-block
  Cc: Keith Busch, Klaus Jensen, Minwoo Im, Kevin Wolf, Max Reitz

nvme-ns device is registered to a nvme controller device during the
initialization in nvme_register_namespace() in case that 'bus' property
is given which means it's mapped to a single controller.

This patch introduced a new property 'subsys' just like the controller
device instance did to map a namespace to a NVMe subsystem.

If 'subsys' property is given to the nvme-ns device, it will belong to
the specified subsystem and will be attached to all controllers in that
subsystem by enabling shared namespace capability in NMIC(Namespace
Multi-path I/O and Namespace Capabilities) in Identify Namespace.

Usage:

  -device nvme-subsys,id=subsys0
  -device nvme,serial=foo,id=nvme0,subsys=subsys0
  -device nvme,serial=bar,id=nvme1,subsys=subsys0
  -device nvme,serial=baz,id=nvme2,subsys=subsys0
  -device nvme-ns,id=ns1,drive=<drv>,nsid=1,subsys=subsys0  # Shared
  -device nvme-ns,id=ns2,drive=<drv>,nsid=2,bus=nvme2       # Non-shared

  In the above example, 'ns1' will be shared to 'nvme0' and 'nvme1' in
  the same subsystem.  On the other hand, 'ns2' will be attached to the
  'nvme2' only as a private namespace in that subsystem.

All the namespace with 'subsys' parameter will attach all controllers in
the subsystem to the namespace by default.

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 hw/block/nvme-ns.c     | 23 ++++++++++++++++++-----
 hw/block/nvme-ns.h     |  7 +++++++
 hw/block/nvme-subsys.c | 25 +++++++++++++++++++++++++
 hw/block/nvme-subsys.h |  3 +++
 hw/block/nvme.c        | 10 +++++++++-
 5 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 62b25cf69bfa..9b493f2ead03 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -63,6 +63,10 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
     id_ns->npda = id_ns->npdg = npdg - 1;
 
+    if (nvme_ns_shared(ns)) {
+        id_ns->nmic |= NVME_NMIC_NS_SHARED;
+    }
+
     return 0;
 }
 
@@ -365,16 +369,25 @@ static void nvme_ns_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (nvme_register_namespace(n, ns, errp)) {
-        error_propagate_prepend(errp, local_err,
-                                "could not register namespace: ");
-        return;
+    if (ns->subsys) {
+        if (nvme_subsys_register_ns(ns, errp)) {
+            error_propagate_prepend(errp, local_err,
+                                    "could not setup namespace to subsys: ");
+            return;
+        }
+    } else {
+        if (nvme_register_namespace(n, ns, errp)) {
+            error_propagate_prepend(errp, local_err,
+                                    "could not register namespace: ");
+            return;
+        }
     }
-
 }
 
 static Property nvme_ns_props[] = {
     DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
+    DEFINE_PROP_LINK("subsys", NvmeNamespace, subsys, TYPE_NVME_SUBSYS,
+                     NvmeSubsystem *),
     DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
     DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
     DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index 293ac990e3f6..929e78861903 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -47,6 +47,8 @@ typedef struct NvmeNamespace {
     const uint32_t *iocs;
     uint8_t      csi;
 
+    NvmeSubsystem   *subsys;
+
     NvmeIdNsZoned   *id_ns_zoned;
     NvmeZone        *zone_array;
     QTAILQ_HEAD(, NvmeZone) exp_open_zones;
@@ -77,6 +79,11 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
     return -1;
 }
 
+static inline bool nvme_ns_shared(NvmeNamespace *ns)
+{
+    return !!ns->subsys;
+}
+
 static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
 {
     NvmeIdNs *id_ns = &ns->id_ns;
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index a01003136b12..e7efdcae7d0d 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -43,6 +43,31 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
     return cntlid;
 }
 
+int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp)
+{
+    NvmeSubsystem *subsys = ns->subsys;
+    NvmeCtrl *n;
+    int i;
+
+    if (subsys->namespaces[nvme_nsid(ns)]) {
+        error_setg(errp, "namespace %d already registerd to subsy %s",
+                   nvme_nsid(ns), subsys->parent_obj.id);
+        return -1;
+    }
+
+    subsys->namespaces[nvme_nsid(ns)] = ns;
+
+    for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
+        n = subsys->ctrls[i];
+
+        if (n && nvme_register_namespace(n, ns, errp)) {
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
 static void nvme_subsys_setup(NvmeSubsystem *subsys)
 {
     char *subnqn;
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 4eba50d96a1d..ccf6a71398d3 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -14,6 +14,7 @@
     OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
 
 #define NVME_SUBSYS_MAX_CTRLS   32
+#define NVME_SUBSYS_MAX_NAMESPACES  32
 
 typedef struct NvmeCtrl NvmeCtrl;
 typedef struct NvmeNamespace NvmeNamespace;
@@ -22,8 +23,10 @@ typedef struct NvmeSubsystem {
     uint8_t     subnqn[256];
 
     NvmeCtrl    *ctrls[NVME_SUBSYS_MAX_CTRLS];
+    NvmeNamespace *namespaces[NVME_SUBSYS_MAX_NAMESPACES];
 } NvmeSubsystem;
 
 int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
+int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp);
 
 #endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 225f0d3f3a27..b123514c6262 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -25,7 +25,8 @@
  *              mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
  *              ,subsys=<subsys_id> \
  *      -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
- *              zoned=<true|false[optional]>
+ *              zoned=<true|false[optional]>, \
+ *              subsys=<subsys_id>
  *      -device nvme-subsys,id=<subsys_id>
  *
  * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
@@ -70,6 +71,13 @@
  *   data size being in effect. By setting this property to 0, users can make
  *   ZASL to be equal to MDTS. This property only affects zoned namespaces.
  *
+ * nvme namespace device parameters
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * - `subsys`
+ *   NVM Subsystem device.  If given, this namespace will be attached to all
+ *   controllers in the subsystem. Otherwise, `bus` must be given to attach
+ *   this namespace to a specified single controller as a non-shared namespace.
+ *
  * Setting `zoned` to true selects Zoned Command Set at the namespace.
  * In this case, the following namespace properties are available to configure
  * zoned operation:
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device
  2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
@ 2021-01-21 22:52   ` Keith Busch
  2021-01-21 23:40     ` Minwoo Im
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2021-01-21 22:52 UTC (permalink / raw)
  To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz

On Fri, Jan 22, 2021 at 07:09:03AM +0900, Minwoo Im wrote:
> +static void nvme_subsys_setup(NvmeSubsystem *subsys)
> +{
> +    char *subnqn;
> +
> +    subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
> +    strpadcpy((char *)subsys->subnqn, sizeof(subsys->subnqn), subnqn, '\0');
> +    g_free(subnqn);

Instead of the duplication and copy, you could format the string
directly into the destination:

    snprintf(subsys->subnqn, sizeof(subsys->subnqn), "nqn.2019-08.org.qemu:%s",
             subsys->parent_obj.id);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem
  2021-01-21 22:09 ` [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
@ 2021-01-21 23:03   ` Keith Busch
  2021-01-21 23:41     ` Minwoo Im
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2021-01-21 23:03 UTC (permalink / raw)
  To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz

On Fri, Jan 22, 2021 at 07:09:04AM +0900, Minwoo Im wrote:
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -23,6 +23,7 @@
>   *              max_ioqpairs=<N[optional]>, \
>   *              aerl=<N[optional]>, aer_max_queued=<N[optional]>, \
>   *              mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
> + *              ,subsys=<subsys_id> \

For consistency, the ',' goes in the preceeding line.

>   *      -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
>   *              zoned=<true|false[optional]>
>   *      -device nvme-subsys,id=<subsys_id>

> @@ -4404,11 +4412,25 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
>      return 0;
>  }
>  
> +static void nvme_init_subnqn(NvmeCtrl *n)
> +{
> +    NvmeSubsystem *subsys = n->subsys;
> +    NvmeIdCtrl *id = &n->id_ctrl;
> +    char *subnqn;
> +
> +    if (!subsys) {
> +        subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", n->params.serial);
> +        strpadcpy((char *)id->subnqn, sizeof(id->subnqn), subnqn, '\0');
> +        g_free(subnqn);

    snprintf(id->subnqn, sizeof(id->subnqn), "nqn.2019-08.org.qemu:%s", n->params.serial);

> +    } else {
> +        pstrcpy((char *)id->subnqn, sizeof(id->subnqn), (char*)subsys->subnqn);
> +    }
> +}


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem
  2021-01-21 22:09 ` [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
@ 2021-01-21 23:17   ` Keith Busch
  2021-01-21 23:41     ` Minwoo Im
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2021-01-21 23:17 UTC (permalink / raw)
  To: Minwoo Im; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz

On Fri, Jan 22, 2021 at 07:09:06AM +0900, Minwoo Im wrote:
> -static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> +static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t cntlid)
>  {
>      NvmeIdCtrl *id = &n->id_ctrl;
>      uint8_t *pci_conf = pci_dev->config;
>  
> +    n->cntlid = cntlid;

I don't think 'cntlid' is important enough to be a function parameter.
You can just set it within the 'NvmeCtrl' struct before calling this
function like all the other properties.

> @@ -4517,7 +4543,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
>          return;
>      }
>  
> -    nvme_init_ctrl(n, pci_dev);
> +    cntlid = nvme_init_subsys(n, errp);
> +    if (cntlid < 0) {

    error_propogate();

> +        return;
> +    }
> +    nvme_init_ctrl(n, pci_dev, cntlid);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device
  2021-01-21 22:52   ` Keith Busch
@ 2021-01-21 23:40     ` Minwoo Im
  0 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 23:40 UTC (permalink / raw)
  To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz

On 21-01-21 14:52:02, Keith Busch wrote:
> On Fri, Jan 22, 2021 at 07:09:03AM +0900, Minwoo Im wrote:
> > +static void nvme_subsys_setup(NvmeSubsystem *subsys)
> > +{
> > +    char *subnqn;
> > +
> > +    subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
> > +    strpadcpy((char *)subsys->subnqn, sizeof(subsys->subnqn), subnqn, '\0');
> > +    g_free(subnqn);
> 
> Instead of the duplication and copy, you could format the string
> directly into the destination:
> 
>     snprintf(subsys->subnqn, sizeof(subsys->subnqn), "nqn.2019-08.org.qemu:%s",
>              subsys->parent_obj.id);

Oh, Thanks. Will fix it!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem
  2021-01-21 23:03   ` Keith Busch
@ 2021-01-21 23:41     ` Minwoo Im
  0 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 23:41 UTC (permalink / raw)
  To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz

On 21-01-21 15:03:38, Keith Busch wrote:
> On Fri, Jan 22, 2021 at 07:09:04AM +0900, Minwoo Im wrote:
> > --- a/hw/block/nvme.c
> > +++ b/hw/block/nvme.c
> > @@ -23,6 +23,7 @@
> >   *              max_ioqpairs=<N[optional]>, \
> >   *              aerl=<N[optional]>, aer_max_queued=<N[optional]>, \
> >   *              mdts=<N[optional]>,zoned.append_size_limit=<N[optional]> \
> > + *              ,subsys=<subsys_id> \
> 
> For consistency, the ',' goes in the preceeding line.

I have no idea what happened here :(.  Will fix it. Thanks!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem
  2021-01-21 23:17   ` Keith Busch
@ 2021-01-21 23:41     ` Minwoo Im
  0 siblings, 0 replies; 13+ messages in thread
From: Minwoo Im @ 2021-01-21 23:41 UTC (permalink / raw)
  To: Keith Busch; +Cc: Klaus Jensen, Kevin Wolf, qemu-devel, qemu-block, Max Reitz

On 21-01-21 15:17:16, Keith Busch wrote:
> On Fri, Jan 22, 2021 at 07:09:06AM +0900, Minwoo Im wrote:
> > -static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
> > +static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t cntlid)
> >  {
> >      NvmeIdCtrl *id = &n->id_ctrl;
> >      uint8_t *pci_conf = pci_dev->config;
> >  
> > +    n->cntlid = cntlid;
> 
> I don't think 'cntlid' is important enough to be a function parameter.
> You can just set it within the 'NvmeCtrl' struct before calling this
> function like all the other properties.

Okay.  Rather than adding a parameter to this function,
nvme_init_subsys() may take this job to assign cntlid to the controller
instance first.  Let me fix one!

> > @@ -4517,7 +4543,11 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
> >          return;
> >      }
> >  
> > -    nvme_init_ctrl(n, pci_dev);
> > +    cntlid = nvme_init_subsys(n, errp);
> > +    if (cntlid < 0) {
> 
>     error_propogate();

Thanks for catching this.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-01-21 23:42 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 22:09 [PATCH V4 0/6] hw/block/nvme: support multi-path for ctrl/ns Minwoo Im
2021-01-21 22:09 ` [PATCH V4 1/6] hw/block/nvme: introduce nvme-subsys device Minwoo Im
2021-01-21 22:52   ` Keith Busch
2021-01-21 23:40     ` Minwoo Im
2021-01-21 22:09 ` [PATCH V4 2/6] hw/block/nvme: support to map controller to a subsystem Minwoo Im
2021-01-21 23:03   ` Keith Busch
2021-01-21 23:41     ` Minwoo Im
2021-01-21 22:09 ` [PATCH V4 3/6] hw/block/nvme: add CMIC enum value for Identify Controller Minwoo Im
2021-01-21 22:09 ` [PATCH V4 4/6] hw/block/nvme: support for multi-controller in subsystem Minwoo Im
2021-01-21 23:17   ` Keith Busch
2021-01-21 23:41     ` Minwoo Im
2021-01-21 22:09 ` [PATCH V4 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace Minwoo Im
2021-01-21 22:09 ` [PATCH V4 6/6] hw/block/nvme: support for shared namespace in subsystem Minwoo Im

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).