Re: [PATCH v3 1/1] block/rbd: Add support for ceph namespaces

[Florian Florensa <fflorensa@online.net>]

[-- Attachment #1: Type: text/plain, Size: 7198 bytes --]

On Mon, Feb 03, 2020 at 12:17:10PM +0100, Florian Florensa wrote:
Hello,

Any news regarding this ?

Regards,

Florian
> Hello Kevin,
> 
> Just checking in to see if any more work needs to be done on this for a
> possible merge.
> 
> Regards,
> 
> Florian
> On Fri, Jan 10, 2020 at 09:13:12AM -0500, Jason Dillaman wrote:
> > On Fri, Jan 10, 2020 at 6:15 AM Florian Florensa <fflorensa@online.net> wrote:
> > >
> > > Starting from ceph Nautilus, RBD has support for namespaces, allowing
> > > for finer grain ACLs on images inside a pool, and tenant isolation.
> > >
> > > In the rbd cli tool documentation, the new image-spec and snap-spec are :
> > >  - [pool-name/[namespace-name/]]image-name
> > >  - [pool-name/[namespace-name/]]image-name@snap-name
> > >
> > > When using an non namespace's enabled qemu, it complains about not
> > > finding the image called namespace-name/image-name, thus we only need to
> > > parse the image once again to find if there is a '/' in its name, and if
> > > there is, use what is before it as the name of the namespace to later
> > > pass it to rados_ioctx_set_namespace.
> > > rados_ioctx_set_namespace if called with en empty string or a null
> > > pointer as the namespace parameters pretty much does nothing, as it then
> > > defaults to the default namespace.
> > >
> > > The namespace is extracted inside qemu_rbd_parse_filename, stored in the
> > > qdict, and used in qemu_rbd_connect to make it work with both qemu-img,
> > > and qemu itself.
> > >
> > > Signed-off-by: Florian Florensa <fflorensa@online.net>
> > > ---
> > >  block/rbd.c          | 44 +++++++++++++++++++++++++++++++-------------
> > >  qapi/block-core.json |  4 ++++
> > >  2 files changed, 35 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/block/rbd.c b/block/rbd.c
> > > index 027cbcc695..84115d34b4 100644
> > > --- a/block/rbd.c
> > > +++ b/block/rbd.c
> > > @@ -104,6 +104,7 @@ typedef struct BDRVRBDState {
> > >      rbd_image_t image;
> > >      char *image_name;
> > >      char *snap;
> > > +    char *namespace;
> > >      uint64_t image_size;
> > >  } BDRVRBDState;
> > >
> > > @@ -152,7 +153,7 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
> > >      const char *start;
> > >      char *p, *buf;
> > >      QList *keypairs = NULL;
> > > -    char *found_str;
> > > +    char *found_str, *image_name;
> > >
> > >      if (!strstart(filename, "rbd:", &start)) {
> > >          error_setg(errp, "File name must start with 'rbd:'");
> > > @@ -171,18 +172,24 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
> > >      qdict_put_str(options, "pool", found_str);
> > >
> > >      if (strchr(p, '@')) {
> > > -        found_str = qemu_rbd_next_tok(p, '@', &p);
> > > -        qemu_rbd_unescape(found_str);
> > > -        qdict_put_str(options, "image", found_str);
> > > +        image_name = qemu_rbd_next_tok(p, '@', &p);
> > >
> > >          found_str = qemu_rbd_next_tok(p, ':', &p);
> > >          qemu_rbd_unescape(found_str);
> > >          qdict_put_str(options, "snapshot", found_str);
> > >      } else {
> > > -        found_str = qemu_rbd_next_tok(p, ':', &p);
> > > +        image_name = qemu_rbd_next_tok(p, ':', &p);
> > > +    }
> > > +    /* Check for namespace in the image_name */
> > > +    if (strchr(image_name, '/')) {
> > > +        found_str = qemu_rbd_next_tok(image_name, '/', &image_name);
> > >          qemu_rbd_unescape(found_str);
> > > -        qdict_put_str(options, "image", found_str);
> > > +        qdict_put_str(options, "namespace", found_str);
> > > +    } else {
> > > +        qdict_put_str(options, "namespace", "");
> > >      }
> > > +    qemu_rbd_unescape(image_name);
> > > +    qdict_put_str(options, "image", image_name);
> > >      if (!p) {
> > >          goto done;
> > >      }
> > > @@ -343,6 +350,11 @@ static QemuOptsList runtime_opts = {
> > >              .type = QEMU_OPT_STRING,
> > >              .help = "Rados pool name",
> > >          },
> > > +        {
> > > +            .name = "namespace",
> > > +            .type = QEMU_OPT_STRING,
> > > +            .help = "Rados namespace name in the pool",
> > > +        },
> > >          {
> > >              .name = "image",
> > >              .type = QEMU_OPT_STRING,
> > > @@ -467,13 +479,14 @@ static int coroutine_fn qemu_rbd_co_create_opts(const char *filename,
> > >       * schema, but when they come from -drive, they're all QString.
> > >       */
> > >      loc = rbd_opts->location;
> > > -    loc->pool     = g_strdup(qdict_get_try_str(options, "pool"));
> > > -    loc->conf     = g_strdup(qdict_get_try_str(options, "conf"));
> > > -    loc->has_conf = !!loc->conf;
> > > -    loc->user     = g_strdup(qdict_get_try_str(options, "user"));
> > > -    loc->has_user = !!loc->user;
> > > -    loc->image    = g_strdup(qdict_get_try_str(options, "image"));
> > > -    keypairs      = qdict_get_try_str(options, "=keyvalue-pairs");
> > > +    loc->pool        = g_strdup(qdict_get_try_str(options, "pool"));
> > > +    loc->conf        = g_strdup(qdict_get_try_str(options, "conf"));
> > > +    loc->has_conf    = !!loc->conf;
> > > +    loc->user        = g_strdup(qdict_get_try_str(options, "user"));
> > > +    loc->has_user    = !!loc->user;
> > > +    loc->q_namespace = g_strdup(qdict_get_try_str(options, "namespace"));
> > > +    loc->image       = g_strdup(qdict_get_try_str(options, "image"));
> > > +    keypairs         = qdict_get_try_str(options, "=keyvalue-pairs");
> > >
> > >      ret = qemu_rbd_do_create(create_options, keypairs, password_secret, errp);
> > >      if (ret < 0) {
> > > @@ -648,6 +661,11 @@ static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
> > >          error_setg_errno(errp, -r, "error opening pool %s", opts->pool);
> > >          goto failed_shutdown;
> > >      }
> > > +    /*
> > > +     * Set the namespace after opening the io context on the pool,
> > > +     * if nspace == NULL or if nspace == "", it is just as we did nothing
> > > +     */
> > > +    rados_ioctx_set_namespace(*io_ctx, opts->q_namespace);
> > >
> > >      return 0;
> > >
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index fcb52ec24f..c6f187ec9b 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -3661,6 +3661,9 @@
> > >  #
> > >  # @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
> > > @@ -3687,6 +3690,7 @@
> > >  ##
> > >  { 'struct': 'BlockdevOptionsRbd',
> > >    'data': { 'pool': 'str',
> > > +            '*namespace': 'str',
> > >              'image': 'str',
> > >              '*conf': 'str',
> > >              '*snapshot': 'str',
> > > --
> > > 2.24.1
> > >
> > 
> > Reviewed-by: Jason Dillaman <dillaman@redhat.com>
> > 
> > -- 
> > Jason
> > 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]