Re: [PATCH v2 2/4] hw/block/nvme: support multiple namespaces

2020-08-08 Thread Minwoo Im
On 06/29 23:40, Klaus Jensen wrote:
> From: Klaus Jensen 
> 
> This adds support for multiple namespaces by introducing a new 'nvme-ns'
> device model. The nvme device creates a bus named from the device name
> ('id'). The nvme-ns devices then connect to this and registers
> themselves with the nvme device.
> 
> This changes how an nvme device is created. Example with two namespaces:
> 
>   -drive file=nvme0n1.img,if=none,id=disk1
>   -drive file=nvme0n2.img,if=none,id=disk2
>   -device nvme,serial=deadbeef,id=nvme0
>   -device nvme-ns,drive=disk1,bus=nvme0,nsid=1
>   -device nvme-ns,drive=disk2,bus=nvme0,nsid=2
> 
> The drive property is kept on the nvme device to keep the change
> backward compatible, but the property is now optional. Specifying a
> drive for the nvme device will always create the namespace with nsid 1.
> 
> Signed-off-by: Klaus Jensen 
> Signed-off-by: Klaus Jensen 
> Reviewed-by: Keith Busch 

Reviewed-by: Minwoo Im 



[PATCH v2 2/4] hw/block/nvme: support multiple namespaces

2020-06-29 Thread Klaus Jensen
From: Klaus Jensen 

This adds support for multiple namespaces by introducing a new 'nvme-ns'
device model. The nvme device creates a bus named from the device name
('id'). The nvme-ns devices then connect to this and registers
themselves with the nvme device.

This changes how an nvme device is created. Example with two namespaces:

  -drive file=nvme0n1.img,if=none,id=disk1
  -drive file=nvme0n2.img,if=none,id=disk2
  -device nvme,serial=deadbeef,id=nvme0
  -device nvme-ns,drive=disk1,bus=nvme0,nsid=1
  -device nvme-ns,drive=disk2,bus=nvme0,nsid=2

The drive property is kept on the nvme device to keep the change
backward compatible, but the property is now optional. Specifying a
drive for the nvme device will always create the namespace with nsid 1.

Signed-off-by: Klaus Jensen 
Signed-off-by: Klaus Jensen 
Reviewed-by: Keith Busch 
---
 hw/block/Makefile.objs |   2 +-
 hw/block/nvme-ns.c | 172 +++
 hw/block/nvme-ns.h |  66 +++
 hw/block/nvme.c| 255 ++---
 hw/block/nvme.h|  44 +++
 hw/block/trace-events  |   8 +-
 6 files changed, 431 insertions(+), 116 deletions(-)
 create mode 100644 hw/block/nvme-ns.c
 create mode 100644 hw/block/nvme-ns.h

diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index 8855c2265639..8c159bc56630 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -13,6 +13,6 @@ common-obj-$(CONFIG_SH4) += tc58128.o
 
 obj-$(CONFIG_VIRTIO_BLK) += virtio-blk.o
 obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
-common-obj-$(CONFIG_NVME_PCI) += nvme.o
+common-obj-$(CONFIG_NVME_PCI) += nvme.o nvme-ns.o
 
 obj-y += dataplane/
diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
new file mode 100644
index ..28ce5e011568
--- /dev/null
+++ b/hw/block/nvme-ns.c
@@ -0,0 +1,172 @@
+/*
+ * QEMU NVM Express Virtual Namespace
+ *
+ * Copyright (c) 2019 CNEX Labs
+ * Copyright (c) 2020 Samsung Electronics
+ *
+ * Authors:
+ *  Klaus Jensen  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See the
+ * COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qemu/cutils.h"
+#include "qemu/log.h"
+#include "hw/block/block.h"
+#include "hw/pci/pci.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
+#include "qapi/error.h"
+
+#include "hw/qdev-properties.h"
+#include "hw/qdev-core.h"
+
+#include "nvme.h"
+#include "nvme-ns.h"
+
+static void nvme_ns_init(NvmeNamespace *ns)
+{
+NvmeIdNs *id_ns = >id_ns;
+
+id_ns->lbaf[0].ds = BDRV_SECTOR_BITS;
+
+id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(ns));
+
+/* no thin provisioning */
+id_ns->ncap = id_ns->nsze;
+id_ns->nuse = id_ns->ncap;
+}
+
+static int nvme_ns_init_blk(NvmeCtrl *n, NvmeNamespace *ns, NvmeIdCtrl *id,
+Error **errp)
+{
+uint64_t perm, shared_perm;
+
+Error *local_err = NULL;
+int ret;
+
+perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE;
+shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
+BLK_PERM_GRAPH_MOD;
+
+ret = blk_set_perm(ns->blk, perm, shared_perm, _err);
+if (ret) {
+error_propagate_prepend(errp, local_err,
+"could not set block permissions: ");
+return ret;
+}
+
+ns->size = blk_getlength(ns->blk);
+if (ns->size < 0) {
+error_setg_errno(errp, -ns->size, "could not get blockdev size");
+return -1;
+}
+
+switch (n->conf.wce) {
+case ON_OFF_AUTO_ON:
+n->features.vwc = 1;
+break;
+case ON_OFF_AUTO_OFF:
+n->features.vwc = 0;
+break;
+case ON_OFF_AUTO_AUTO:
+n->features.vwc = blk_enable_write_cache(ns->blk);
+break;
+default:
+abort();
+}
+
+blk_set_enable_write_cache(ns->blk, n->features.vwc);
+
+return 0;
+}
+
+static int nvme_ns_check_constraints(NvmeNamespace *ns, Error **errp)
+{
+if (!ns->blk) {
+error_setg(errp, "block backend not configured");
+return -1;
+}
+
+return 0;
+}
+
+int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error **errp)
+{
+if (nvme_ns_check_constraints(ns, errp)) {
+return -1;
+}
+
+if (nvme_ns_init_blk(n, ns, >id_ctrl, errp)) {
+return -1;
+}
+
+nvme_ns_init(ns);
+if (nvme_register_namespace(n, ns, errp)) {
+return -1;
+}
+
+return 0;
+}
+
+static void nvme_ns_realize(DeviceState *dev, Error **errp)
+{
+NvmeNamespace *ns = NVME_NS(dev);
+BusState *s = qdev_get_parent_bus(dev);
+NvmeCtrl *n = NVME(s->parent);
+Error *local_err = NULL;
+
+if (nvme_ns_setup(n, ns, _err)) {
+error_propagate_prepend(errp, local_err,
+"could not setup namespace: ");
+return;
+}
+}
+
+static Property nvme_ns_props[] = {
+DEFINE_PROP_DRIVE("drive", NvmeNamespace, blk),
+