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

[Jason Dillaman <jdillama@redhat.com>]

On Thu, Dec 19, 2019 at 8:44 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          | 30 ++++++++++++++++++++++++------
>  qapi/block-core.json |  3 +++
>  2 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index 027cbcc695..e43099fc75 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 *nspace;
>      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, "nspace", found_str);
> +    } else {
> +        qdict_put_str(options, "nspace", "");
>      }
> +    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 = "nspace",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Rados namespace name in the pool",
> +        },
>          {
>              .name = "image",
>              .type = QEMU_OPT_STRING,
> @@ -472,6 +484,7 @@ static int coroutine_fn qemu_rbd_co_create_opts(const char *filename,
>      loc->has_conf = !!loc->conf;
>      loc->user     = g_strdup(qdict_get_try_str(options, "user"));
>      loc->has_user = !!loc->user;
> +    loc->nspace   = g_strdup(qdict_get_try_str(options, "nspace"));
>      loc->image    = g_strdup(qdict_get_try_str(options, "image"));
>      keypairs      = qdict_get_try_str(options, "=keyvalue-pairs");
>
> @@ -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->nspace);
>
>      return 0;
>
> 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.
>  #
> +# @nspace:             Rados namespace name in the Ceph pool.
> +#
>  # @image:              Image name in the Ceph pool.
>  #
>  # @conf:               path to Ceph configuration file.  Values
> @@ -3683,6 +3685,7 @@
>  ##
>  { 'struct': 'BlockdevOptionsRbd',
>    'data': { 'pool': 'str',
> +            'nspace': 'str',
>              'image': 'str',
>              '*conf': 'str',
>              '*snapshot': 'str',
> --
> 2.24.1
>

Thanks for tackling this. I had this and msgr v2 support on my todo
list for QEMU but I haven't had a chance to work on them yet. The
changes look good to me and it works as expected during CLI
play-testing.

Reviewed-by: Jason Dillaman <dillaman@redhat.com>