[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] block/rbd: Add support for ceph namespaces
From: |
Stefano Garzarella |
Subject: |
Re: [PATCH] block/rbd: Add support for ceph namespaces |
Date: |
Fri, 20 Dec 2019 16:09:31 +0100 |
On Fri, Dec 20, 2019 at 09:56:51AM -0500, Jason Dillaman wrote:
> On Fri, Dec 20, 2019 at 9:11 AM Florian Florensa <address@hidden> wrote:
> >
> > Hello Stefano and Jason,
> >
> > First of all thanks for the quick reply,
> > Response inline belowe
> > > Hi Florian,
> > >
> > > I think we need to add (Since: 5.0).
> >
> > Are you implying by that (Since: 5.0) that we need to specify its
> > availability target is qemu 5.0 ?
Exactly, as Jason suggested is part of documentation,
Following the file, I mean something like that:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0cf68fea14..9ebc020e93 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3657,6 +3657,8 @@
#
# @pool: Ceph pool name.
#
+# @namespace: Rados namespace name in the Ceph pool. (Since: 5.0)
+#
# @image: Image name in the Ceph pool.
#
# @conf: path to Ceph configuration file. Values
>
> FWIW, I took this as just a comment to add some documentation that the
> field is only valid starting w/ qemu v5.
>
> > I guess that maybe a version check would be better ? Like try to do
> > namespaces stuff only if we have a recent enough librbd in the system ?
> > Using something like :
> >
> > int rbd_major;
> >
> > rbd_version(&rbd_major, NULL, NULL);
> > /*
> > * Target only nautilus+ librbd for namespace support
> > */
> > if (rbd_major >= 14) // tar
> > <process namespace>
>
> Unfortunately, those versions weren't updated in the Mimic nor
> Nautilus release so it would still return 1/12 (whoops). I think that
> means you would need to add a probe in "configure" to test for librbd
> namespace support (e.g. test for the existence of the `rbd_list2`
> function or the `rbd_linked_image_spec_t` structure). I'll fix this
> before the forthcoming Octopus release.
>
> > > The patch LGTM, but I'd like to use 'namespace' instead of cryptic
> > > 'nspace'. (as BlockdevOptionsNVMe did)
> > > What do you think?
> > >
> > Yes no worries, I can rename it to 'rbd_namespace' to avoid any possible
> > confusion, is this Ok for you ?
>
> We use "pool_namespace" in the rbd CLI if you are trying to avoid the
> word "namespace".
>
Agree, I'd avoid the 'rbd_' prefix.
Thanks,
Stefano
Re: [PATCH] block/rbd: Add support for ceph namespaces, Eric Blake, 2019/12/20