On Tue, Nov 23, 2021 at 11:11:37AM +0100, Lukasz Maniak wrote:
> On Wed, Nov 10, 2021 at 04:56:29PM +0530, Naveen wrote:
> > From: Naveen Nagar <naveen...@samsung.com>
> >
> > This patch supports namespace management : create and delete operations
> > This patch has been tested with the following command and size of image
> > file for unallocated namespaces is taken as 0GB. ns_create will look into
> > the list of unallocated namespaces and it will initialize the same and
> > return the nsid of the same. A new mandatory field has been added called
> > tnvmcap and we have ensured that the total capacity of namespace created
> > does not exceed tnvmcap
> >
> > -device nvme-subsys,id=subsys0,tnvmcap=8
> > -device nvme,serial=foo,id=nvme0,subsys=subsys0
> > -device nvme,serial=bar,id=nvme1,subsys=subsys0
> >
> > -drive id=ns1,file=ns1.img,if=none
> > -device nvme-ns,drive=ns1,bus=nvme0,nsid=1,zoned=false,shared=true
> > -drive id=ns2,file=ns2.img,if=none
> > -device nvme-ns,drive=ns2,bus=nvme0,nsid=2,zoned=false,shared=true
> > -drive id=ns3,file=ns3.img,if=none
> > -device nvme-ns,drive=ns3,bus=nvme0,nsid=3,zoned=false,shared=true
> > -drive id=ns4,file=ns4.img,if=none
> > -device nvme-ns,drive=ns4,bus=nvme0,nsid=4,zoned=false,shared=true
> >
> > Please review and suggest if any changes are required.
> >
> > Signed-off-by: Naveen Nagar <naveen...@samsung.com>
> >
> > Since v2:
> > -Lukasz Maniak found a bug in namespace attachment and proposed
> > solution is added
> >
>
> Hi Naveen,
>
> The current implementation is inconsistent and thus has a bug related to
> unvmcap support.
>
> Namespaces are pre-allocated after boot, and the initial namespace size
> is the size of the associated blockdev. If the blockdevs are non-zero
> sized then the first deletion of the namespaces associated with them
> will increment unvmcap by their size. This will make unvmcap greater
> than tnvmcap.
>
> While the easiest way would be to prohibit the use of non-zero sized
> blockdev with namespace management, doing so would limit the
> functionality of the namespaces itself, which we would like to avoid.
>
> This fix below addresses issues related to unvmcap and non-zero block
> devices. The unvmcap value will be properly updated on both the first
> and subsequent controllers added to the subsystem regardless of the
> order in which nvme-ns is defined on the command line before or after
> the controller definition. Additionally, if the block device size of any
> namespace causes the unvmcap to be exceeded, an error will be returned
> at the namespace definition point.
>
> The fix is based on v3 based on v6.1.0, as v3 does not apply to master.
>
> ---
> hw/nvme/ctrl.c | 7 +++++++
> hw/nvme/ns.c | 23 ++++++++++++++++++++---
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 63ea2fcb14..dc0ad4155b 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -6594,6 +6594,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
> *pci_dev)
> NvmeIdCtrl *id = &n->id_ctrl;
> uint8_t *pci_conf = pci_dev->config;
> uint64_t cap = ldq_le_p(&n->bar.cap);
> + int i;
>
> 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));
> @@ -6672,6 +6673,12 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
> *pci_dev)
> id->cmic |= NVME_CMIC_MULTI_CTRL;
> id->tnvmcap = n->subsys->params.tnvmcap * GiB;
> id->unvmcap = n->subsys->params.tnvmcap * GiB;
> +
> + for (i = 0; i < ARRAY_SIZE(n->subsys->namespaces); i++) {
> + if (n->subsys->namespaces[i]) {
> + id->unvmcap -= le64_to_cpu(n->subsys->namespaces[i]->size);
> + }
> + }
> }
>
> NVME_CAP_SET_MQES(cap, 0x7ff);
> diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
> index f62a695132..c87d7f5bd6 100644
> --- a/hw/nvme/ns.c
> +++ b/hw/nvme/ns.c
> @@ -140,9 +140,12 @@ lbaf_found:
> return 0;
> }
>
> -static int nvme_ns_init_blk(NvmeNamespace *ns, Error **errp)
> +static int nvme_ns_init_blk(NvmeNamespace *ns, NvmeSubsystem *subsys,
> + Error **errp)
> {
> bool read_only;
> + NvmeCtrl *ctrl;
> + int i;
>
> if (!blkconf_blocksizes(&ns->blkconf, errp)) {
> return -1;
> @@ -164,6 +167,21 @@ static int nvme_ns_init_blk(NvmeNamespace *ns, Error
> **errp)
> return -1;
> }
>
> + if (subsys) {
> + for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
> + ctrl = nvme_subsys_ctrl(subsys, i);
> +
> + if (ctrl) {
> + if (ctrl->id_ctrl.unvmcap < le64_to_cpu(ns->size)) {
> + error_setg(errp, "blockdev size %ld exceeds subsystem's "
> + "unallocated capacity", ns->size);
> + } else {
> + ctrl->id_ctrl.unvmcap -= le64_to_cpu(ns->size);
> + }
> + }
> + }
> + }
> +
> return 0;
> }
>
> @@ -480,7 +498,7 @@ static void nvme_ns_realize(DeviceState *dev, Error
> **errp)
> }
> }
>
> - if (nvme_ns_init_blk(ns, errp)) {
> + if (nvme_ns_init_blk(ns, subsys, errp)) {
> return;
> }
>
> @@ -527,7 +545,6 @@ static void nvme_ns_realize(DeviceState *dev, Error
> **errp)
>
> if (ctrl) {
> nvme_attach_ns(ctrl, ns);
> - ctrl->id_ctrl.unvmcap -= le64_to_cpu(ns->size);
> }
> }
>
> --
> 2.25.1
>
Fixing unvmcap support brought another concern to attention.
Here is another little patch on top of the previous one to truncate the
block device to 0 when the associated namespace is deleted.
Instead, it may fail to re-launch QEMU with the blockdevs from the
previous execution when the sum of the blockdev sizes after namespace
management exceeds unvmcap.
---
hw/nvme/ctrl.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index dc0ad4155b..f84f682d36 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -5320,6 +5320,7 @@ static void nvme_namespace_delete(NvmeCtrl *n,
NvmeNamespace *ns, uint32_t nsid)
{
NvmeCtrl *ctrl;
NvmeSubsystem *subsys = n->subsys;
+ int ret;
subsys->namespaces[nsid] = NULL;
QSLIST_INSERT_HEAD(&subsys->unallocated_namespaces, ns, entry);
@@ -5334,6 +5335,9 @@ static void nvme_namespace_delete(NvmeCtrl *n,
NvmeNamespace *ns, uint32_t nsid)
nvme_ns_attr_changed_aer(ctrl, nsid);
}
}
+
+ ret = nvme_blk_truncate(ns->blkconf.blk, 0, NULL);
+ assert(!ret);
}
static uint16_t nvme_ns_management(NvmeCtrl *n, NvmeRequest *req)
--
2.25.1
