[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.0 7/7] hw/block/nvme: fix handling of private namespace
From: |
Klaus Jensen |
Subject: |
Re: [PATCH for-6.0 7/7] hw/block/nvme: fix handling of private namespaces |
Date: |
Mon, 5 Apr 2021 15:56:17 +0200 |
On Apr 5 18:02, Gollu Appalanaidu wrote:
> On Wed, Mar 24, 2021 at 09:09:07PM +0100, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > Prior to this patch, if a private nvme-ns device (that is, a namespace
> > that is not linked to a subsystem) is wired up to an nvme-subsys linked
> > nvme controller device, the device fails to verify that the namespace id
> > is unique within the subsystem. NVM Express v1.4b, Section 6.1.6 ("NSID
> > and Namespace Usage") states that because the device supports Namespace
> > Management, "NSIDs *shall* be unique within the NVM subsystem".
> >
> > Additionally, prior to this patch, private namespaces are not known to
> > the subsystem and the namespace is considered exclusive to the
> > controller with which it is initially wired up to. However, this is not
> > the definition of a private namespace; per Section 1.6.33 ("private
> > namespace"), a private namespace is just a namespace that does not
> > support multipath I/O or namespace sharing, which means "that it is only
> > able to be attached to one controller at a time".
> >
> > Fix this by always allocating namespaces in the subsystem (if one is
> > linked to the controller), regardsless of the shared/private status of
> > the namespace. Whether or not the namespace is shareable is controlled
> > by a new `shared` nvme-ns parameter.
> >
> > Finally, this fix allows the nvme-ns `subsys` parameter to be removed,
> > since the `shared` parameter now serves the purpose of attaching the
> > namespace to all controllers in the subsystem upon device realization.
> > It is invalid to have an nvme-ns namespace device with a linked
> > subsystem without the parent nvme controller device also being linked to
> > one and since the nvme-ns devices will unconditionally be "attached" (in
> > QEMU terms that is) to an nvme controller device through an NvmeBus, the
> > nvme-ns namespace device can always get a reference to the subsystem of
> > the controller it is explicitly (using 'bus=' parametr) or implicitly
> > attaching to.
> >
> > Fixes: e570768566b3 ("hw/block/nvme: support for shared namespace in
> > subsystem")
> > Cc: Minwoo Im <minwoo.im.dev@gmail.com>
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> > hw/block/nvme-ns.h | 10 ++--
> > hw/block/nvme-subsys.h | 7 ++-
> > hw/block/nvme.h | 39 +-------------
> > include/block/nvme.h | 1 +
> > hw/block/nvme-ns.c | 74 +++++++++++++++++++++++----
> > hw/block/nvme-subsys.c | 28 -----------
> > hw/block/nvme.c | 112 +++++++++++++----------------------------
> > hw/block/trace-events | 1 -
> > 8 files changed, 106 insertions(+), 166 deletions(-)
> >
<snip>
> >
> > static Property nvme_ns_props[] = {
> > DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
> > - DEFINE_PROP_LINK("subsys", NvmeNamespace, subsys, TYPE_NVME_SUBSYS,
> > - NvmeSubsystem *),
> > DEFINE_PROP_BOOL("detached", NvmeNamespace, params.detached, false),
> > + DEFINE_PROP_BOOL("shared", NvmeNamespace, params.shared, false)i,
>
> Nice change point, hope we need update the usage, removing "subsys" from
> nvme-ns device params and adding "shared" param?
>
Good catch, thanks. Fixing that up for a v2.
signature.asc
Description: PGP signature