[PATCH v1] block/rbd: Add support for layered encryption

Or Ozeri posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221026084836.1819572-1-oro@il.ibm.com
Maintainers: Ilya Dryomov <idryomov@gmail.com>, Peter Lieven <pl@kamp.de>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
block/rbd.c          | 134 ++++++++++++++++++++++++++++++++++++++++++-
qapi/block-core.json |  33 ++++++++++-
2 files changed, 163 insertions(+), 4 deletions(-)
[PATCH v1] block/rbd: Add support for layered encryption
Posted by Or Ozeri 1 year, 6 months ago
Starting from ceph Reef, RBD has built-in support for layered encryption,
where each ancestor image (in a cloned image setting) can be possibly
encrypted using a unique passphrase.

A new function, rbd_encryption_load2, was added to librbd API.
This new function supports an array of passphrases (via "spec" structs).

This commit extends the qemu rbd driver API to use this new librbd API,
in order to support this new layered encryption feature.

Signed-off-by: Or Ozeri <oro@il.ibm.com>
---
 block/rbd.c          | 134 ++++++++++++++++++++++++++++++++++++++++++-
 qapi/block-core.json |  33 ++++++++++-
 2 files changed, 163 insertions(+), 4 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index f826410f40..09953687c9 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -71,6 +71,16 @@ static const char rbd_luks2_header_verification[
     'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
 };
 
+static const char rbd_layered_luks_header_verification[
+        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
+    'R', 'B', 'D', 'L', 0xBA, 0xBE, 0, 1
+};
+
+static const char rbd_layered_luks2_header_verification[
+        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
+    'R', 'B', 'D', 'L', 0xBA, 0xBE, 0, 2
+};
+
 typedef enum {
     RBD_AIO_READ,
     RBD_AIO_WRITE,
@@ -470,6 +480,9 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
     size_t passphrase_len;
     rbd_encryption_luks1_format_options_t luks_opts;
     rbd_encryption_luks2_format_options_t luks2_opts;
+#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
+    rbd_encryption_luks_format_options_t luks_all_opts;
+#endif
     rbd_encryption_format_t format;
     rbd_encryption_options_t opts;
     size_t opts_size;
@@ -505,6 +518,23 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
             luks2_opts.passphrase_size = passphrase_len;
             break;
         }
+#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
+        case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_ALL: {
+            memset(&luks_all_opts, 0, sizeof(luks_all_opts));
+            format = RBD_ENCRYPTION_FORMAT_LUKS;
+            opts = &luks_all_opts;
+            opts_size = sizeof(luks_all_opts);
+            r = qemu_rbd_convert_luks_options(
+                    qapi_RbdEncryptionOptionsLUKSAll_base(&encrypt->u.luks_all),
+                    &passphrase, &passphrase_len, errp);
+            if (r < 0) {
+                return r;
+            }
+            luks_all_opts.passphrase = passphrase;
+            luks_all_opts.passphrase_size = passphrase_len;
+            break;
+        }
+#endif
         default: {
             r = -ENOTSUP;
             error_setg_errno(
@@ -522,6 +552,87 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
 
     return 0;
 }
+
+#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
+static int qemu_rbd_encryption_load2(rbd_image_t image,
+                                     RbdEncryptionOptions *encrypt,
+                                     Error **errp)
+{
+    int r = 0;
+    int encryption_options_count = 1;
+    int spec_count = 0;
+    int passphrase_count = 0;
+    int i;
+    RbdEncryptionOptions *curr_encrypt;
+    rbd_encryption_spec_t *specs;
+    rbd_encryption_spec_t *curr_spec;
+    rbd_encryption_luks_format_options_t* luks_all_opts;
+    char **passphrases;
+    char **curr_passphrase;
+
+    /* count encryption options */
+    for (curr_encrypt = encrypt; curr_encrypt->has_parent;
+         curr_encrypt = curr_encrypt->parent, ++encryption_options_count) {
+    }
+
+    specs = g_new0(rbd_encryption_spec_t, encryption_options_count);
+    passphrases = g_new0(char*, encryption_options_count);
+
+    curr_encrypt = encrypt;
+    for (i = 0; i < encryption_options_count; ++i) {
+        if (curr_encrypt->format != RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_ALL) {
+            r = -ENOTSUP;
+            error_setg_errno(
+                    errp, -r, "unknown image encryption format: %u",
+                    curr_encrypt->format);
+            goto exit;
+        }
+
+        curr_spec = &specs[i];
+        curr_passphrase = &passphrases[i];
+        curr_spec->format = RBD_ENCRYPTION_FORMAT_LUKS;
+        curr_spec->opts_size = sizeof(rbd_encryption_luks_format_options_t);
+
+        luks_all_opts = g_new0(rbd_encryption_luks_format_options_t, 1);
+        curr_spec->opts = luks_all_opts;
+        ++spec_count;
+        memset(luks_all_opts, 0, sizeof(rbd_encryption_luks_format_options_t));
+
+        r = qemu_rbd_convert_luks_options(
+                qapi_RbdEncryptionOptionsLUKSAll_base(
+                        &curr_encrypt->u.luks_all),
+                curr_passphrase, &luks_all_opts->passphrase_size,
+                errp);
+        if (r < 0) {
+            goto exit;
+        }
+
+        ++passphrase_count;
+        luks_all_opts->passphrase = *curr_passphrase;
+
+        curr_encrypt = curr_encrypt->parent;
+    }
+
+    r = rbd_encryption_load2(image, specs, spec_count);
+    if (r < 0) {
+        error_setg_errno(errp, -r, "encryption load (2) fail");
+        goto exit;
+    }
+
+exit:
+    for (i = 0; i < spec_count; ++i) {
+        luks_all_opts = (rbd_encryption_luks_format_options_t*)(specs[i].opts);
+        if (passphrase_count > 0) {
+            g_free(passphrases[i]);
+            --passphrase_count;
+        }
+        g_free(luks_all_opts);
+    }
+    g_free(passphrases);
+    g_free(specs);
+    return r;
+}
+#endif
 #endif
 
 /* FIXME Deprecate and remove keypairs or make it available in QMP. */
@@ -993,7 +1104,18 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
 
     if (opts->has_encrypt) {
 #ifdef LIBRBD_SUPPORTS_ENCRYPTION
-        r = qemu_rbd_encryption_load(s->image, opts->encrypt, errp);
+        if (opts->encrypt->has_parent) {
+#ifndef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
+            r = -ENOTSUP;
+            error_setg(errp, "RBD library does not support"
+                             " specifying parent encryption");
+            goto failed_post_open;
+#else
+            r = qemu_rbd_encryption_load2(s->image, opts->encrypt, errp);
+#endif
+        } else {
+            r = qemu_rbd_encryption_load(s->image, opts->encrypt, errp);
+        }
         if (r < 0) {
             goto failed_post_open;
         }
@@ -1284,6 +1406,16 @@ static ImageInfoSpecific *qemu_rbd_get_specific_info(BlockDriverState *bs,
         spec_info->u.rbd.data->encryption_format =
                 RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2;
         spec_info->u.rbd.data->has_encryption_format = true;
+    } else if (memcmp(buf, rbd_layered_luks_header_verification,
+               RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN) == 0) {
+        spec_info->u.rbd.data->encryption_format =
+                RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_LAYERED;
+        spec_info->u.rbd.data->has_encryption_format = true;
+    } else if (memcmp(buf, rbd_layered_luks2_header_verification,
+               RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN) == 0) {
+        spec_info->u.rbd.data->encryption_format =
+                RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2_LAYERED;
+        spec_info->u.rbd.data->has_encryption_format = true;
     } else {
         spec_info->u.rbd.data->has_encryption_format = false;
     }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 882b266532..81ac58cd8a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3753,10 +3753,20 @@
 ##
 # @RbdImageEncryptionFormat:
 #
+# luks
+#
+# luks2
+#
+# luks-all: Used for opening either luks or luks2. (Since 7.2)
+#
+# luks-layered: Layered encryption. Only used for info. (Since 7.2)
+#
+# luks2-layered: Layered encryption. Only used for info. (Since 7.2)
+#
 # Since: 6.1
 ##
 { 'enum': 'RbdImageEncryptionFormat',
-  'data': [ 'luks', 'luks2' ] }
+  'data': [ 'luks', 'luks2', 'luks-all', 'luks-layered', 'luks2-layered' ] }
 
 ##
 # @RbdEncryptionOptionsLUKSBase:
@@ -3798,6 +3808,15 @@
   'base': 'RbdEncryptionOptionsLUKSBase',
   'data': { } }
 
+##
+# @RbdEncryptionOptionsLUKSAll:
+#
+# Since: 7.2
+##
+{ 'struct': 'RbdEncryptionOptionsLUKSAll',
+  'base': 'RbdEncryptionOptionsLUKSBase',
+  'data': { } }
+
 ##
 # @RbdEncryptionCreateOptionsLUKS:
 #
@@ -3819,13 +3838,21 @@
 ##
 # @RbdEncryptionOptions:
 #
+# @format: Encryption format.
+#
+# @parent: Parent image encryption options (for cloned images).
+#          Can be left unspecified if all ancestor images are encrypted
+#          the same way as the child image.  (Since 7.2)
+#
 # Since: 6.1
 ##
 { 'union': 'RbdEncryptionOptions',
-  'base': { 'format': 'RbdImageEncryptionFormat' },
+  'base': { 'format': 'RbdImageEncryptionFormat',
+            '*parent': 'RbdEncryptionOptions' },
   'discriminator': 'format',
   'data': { 'luks': 'RbdEncryptionOptionsLUKS',
-            'luks2': 'RbdEncryptionOptionsLUKS2' } }
+            'luks2': 'RbdEncryptionOptionsLUKS2',
+            'luks-all': 'RbdEncryptionOptionsLUKSAll'} }
 
 ##
 # @RbdEncryptionCreateOptions:
-- 
2.25.1
Re: [PATCH v1] block/rbd: Add support for layered encryption
Posted by Ilya Dryomov 1 year, 5 months ago
On Wed, Oct 26, 2022 at 10:48 AM Or Ozeri <oro@il.ibm.com> wrote:
>
> Starting from ceph Reef, RBD has built-in support for layered encryption,
> where each ancestor image (in a cloned image setting) can be possibly
> encrypted using a unique passphrase.
>
> A new function, rbd_encryption_load2, was added to librbd API.
> This new function supports an array of passphrases (via "spec" structs).
>
> This commit extends the qemu rbd driver API to use this new librbd API,
> in order to support this new layered encryption feature.
>
> Signed-off-by: Or Ozeri <oro@il.ibm.com>
> ---
>  block/rbd.c          | 134 ++++++++++++++++++++++++++++++++++++++++++-
>  qapi/block-core.json |  33 ++++++++++-
>  2 files changed, 163 insertions(+), 4 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index f826410f40..09953687c9 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -71,6 +71,16 @@ static const char rbd_luks2_header_verification[
>      'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
>  };
>
> +static const char rbd_layered_luks_header_verification[
> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +    'R', 'B', 'D', 'L', 0xBA, 0xBE, 0, 1
> +};
> +
> +static const char rbd_layered_luks2_header_verification[
> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +    'R', 'B', 'D', 'L', 0xBA, 0xBE, 0, 2
> +};
> +
>  typedef enum {
>      RBD_AIO_READ,
>      RBD_AIO_WRITE,
> @@ -470,6 +480,9 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>      size_t passphrase_len;
>      rbd_encryption_luks1_format_options_t luks_opts;
>      rbd_encryption_luks2_format_options_t luks2_opts;
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
> +    rbd_encryption_luks_format_options_t luks_all_opts;
> +#endif
>      rbd_encryption_format_t format;
>      rbd_encryption_options_t opts;
>      size_t opts_size;
> @@ -505,6 +518,23 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>              luks2_opts.passphrase_size = passphrase_len;
>              break;
>          }
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
> +        case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_ALL: {
> +            memset(&luks_all_opts, 0, sizeof(luks_all_opts));
> +            format = RBD_ENCRYPTION_FORMAT_LUKS;
> +            opts = &luks_all_opts;
> +            opts_size = sizeof(luks_all_opts);
> +            r = qemu_rbd_convert_luks_options(
> +                    qapi_RbdEncryptionOptionsLUKSAll_base(&encrypt->u.luks_all),
> +                    &passphrase, &passphrase_len, errp);
> +            if (r < 0) {
> +                return r;
> +            }
> +            luks_all_opts.passphrase = passphrase;
> +            luks_all_opts.passphrase_size = passphrase_len;
> +            break;
> +        }
> +#endif
>          default: {
>              r = -ENOTSUP;
>              error_setg_errno(
> @@ -522,6 +552,87 @@ static int qemu_rbd_encryption_load(rbd_image_t image,
>
>      return 0;
>  }
> +
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2
> +static int qemu_rbd_encryption_load2(rbd_image_t image,
> +                                     RbdEncryptionOptions *encrypt,
> +                                     Error **errp)
> +{
> +    int r = 0;
> +    int encryption_options_count = 1;
> +    int spec_count = 0;
> +    int passphrase_count = 0;
> +    int i;
> +    RbdEncryptionOptions *curr_encrypt;
> +    rbd_encryption_spec_t *specs;
> +    rbd_encryption_spec_t *curr_spec;
> +    rbd_encryption_luks_format_options_t* luks_all_opts;
> +    char **passphrases;
> +    char **curr_passphrase;
> +
> +    /* count encryption options */
> +    for (curr_encrypt = encrypt; curr_encrypt->has_parent;
> +         curr_encrypt = curr_encrypt->parent, ++encryption_options_count) {

Hi Or,

I would move the increment into the body because empty body loops can
be confusing (and might also rename to encrypt_count to match "encrypt"
and "curr_encrypt" names).

> +    }
> +
> +    specs = g_new0(rbd_encryption_spec_t, encryption_options_count);
> +    passphrases = g_new0(char*, encryption_options_count);

I don't understand the need for this char* array.  Is there a problem
with putting the blob directly into luks_all_opts->passphrase just like
the size is put into luks_all_opts->passphrase_size?

> +
> +    curr_encrypt = encrypt;
> +    for (i = 0; i < encryption_options_count; ++i) {
> +        if (curr_encrypt->format != RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_ALL) {

I don't think librbd imposes this restriction.  It's probably fine
to impose it here to make the implementation simpler but I wanted to
highlight that.

> +            r = -ENOTSUP;
> +            error_setg_errno(
> +                    errp, -r, "unknown image encryption format: %u",
> +                    curr_encrypt->format);
> +            goto exit;
> +        }
> +
> +        curr_spec = &specs[i];

curr_spec and curr_passphrase variables seem completely redundant to
me -- specs[i] is actually shorter to type than curr_spec ;)

> +        curr_passphrase = &passphrases[i];
> +        curr_spec->format = RBD_ENCRYPTION_FORMAT_LUKS;
> +        curr_spec->opts_size = sizeof(rbd_encryption_luks_format_options_t);
> +
> +        luks_all_opts = g_new0(rbd_encryption_luks_format_options_t, 1);
> +        curr_spec->opts = luks_all_opts;
> +        ++spec_count;

What is the purpose of counting specs (and then also, separately,
passphrases)?  Wouldn't encryption_options_count suffice?

If the only reason is cleanup, it should be much more robust to just
blast through the entire specs array and call g_free unconditionally on
both pointers in all slots.

> +        memset(luks_all_opts, 0, sizeof(rbd_encryption_luks_format_options_t));

g_new0 already initializes to zero, so this memset appears to be
redundant.

> +
> +        r = qemu_rbd_convert_luks_options(
> +                qapi_RbdEncryptionOptionsLUKSAll_base(
> +                        &curr_encrypt->u.luks_all),
> +                curr_passphrase, &luks_all_opts->passphrase_size,
> +                errp);
> +        if (r < 0) {
> +            goto exit;
> +        }
> +
> +        ++passphrase_count;
> +        luks_all_opts->passphrase = *curr_passphrase;
> +
> +        curr_encrypt = curr_encrypt->parent;
> +    }
> +
> +    r = rbd_encryption_load2(image, specs, spec_count);
> +    if (r < 0) {
> +        error_setg_errno(errp, -r, "encryption load (2) fail");

Perhaps "layered encryption load fail"?

> +        goto exit;
> +    }
> +
> +exit:
> +    for (i = 0; i < spec_count; ++i) {
> +        luks_all_opts = (rbd_encryption_luks_format_options_t*)(specs[i].opts);
> +        if (passphrase_count > 0) {
> +            g_free(passphrases[i]);
> +            --passphrase_count;
> +        }
> +        g_free(luks_all_opts);
> +    }
> +    g_free(passphrases);
> +    g_free(specs);
> +    return r;
> +}
> +#endif
>  #endif
>
>  /* FIXME Deprecate and remove keypairs or make it available in QMP. */
> @@ -993,7 +1104,18 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
>
>      if (opts->has_encrypt) {
>  #ifdef LIBRBD_SUPPORTS_ENCRYPTION
> -        r = qemu_rbd_encryption_load(s->image, opts->encrypt, errp);
> +        if (opts->encrypt->has_parent) {
> +#ifndef LIBRBD_SUPPORTS_ENCRYPTION_LOAD2

I would flip this to #ifdef to avoid mixing "not supported branch at
the top" and "not supported branch at the bottom" styles in the same
function.

> +            r = -ENOTSUP;
> +            error_setg(errp, "RBD library does not support"
> +                             " specifying parent encryption");
> +            goto failed_post_open;

This goto is redundant.

> +#else
> +            r = qemu_rbd_encryption_load2(s->image, opts->encrypt, errp);
> +#endif
> +        } else {
> +            r = qemu_rbd_encryption_load(s->image, opts->encrypt, errp);
> +        }
>          if (r < 0) {
>              goto failed_post_open;
>          }
> @@ -1284,6 +1406,16 @@ static ImageInfoSpecific *qemu_rbd_get_specific_info(BlockDriverState *bs,
>          spec_info->u.rbd.data->encryption_format =
>                  RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2;
>          spec_info->u.rbd.data->has_encryption_format = true;
> +    } else if (memcmp(buf, rbd_layered_luks_header_verification,
> +               RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN) == 0) {
> +        spec_info->u.rbd.data->encryption_format =
> +                RBD_IMAGE_ENCRYPTION_FORMAT_LUKS_LAYERED;
> +        spec_info->u.rbd.data->has_encryption_format = true;
> +    } else if (memcmp(buf, rbd_layered_luks2_header_verification,
> +               RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN) == 0) {
> +        spec_info->u.rbd.data->encryption_format =
> +                RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2_LAYERED;
> +        spec_info->u.rbd.data->has_encryption_format = true;
>      } else {
>          spec_info->u.rbd.data->has_encryption_format = false;
>      }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 882b266532..81ac58cd8a 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3753,10 +3753,20 @@
>  ##
>  # @RbdImageEncryptionFormat:
>  #
> +# luks
> +#
> +# luks2
> +#
> +# luks-all: Used for opening either luks or luks2. (Since 7.2)
> +#
> +# luks-layered: Layered encryption. Only used for info. (Since 7.2)
> +#
> +# luks2-layered: Layered encryption. Only used for info. (Since 7.2)
> +#
>  # Since: 6.1
>  ##
>  { 'enum': 'RbdImageEncryptionFormat',
> -  'data': [ 'luks', 'luks2' ] }
> +  'data': [ 'luks', 'luks2', 'luks-all', 'luks-layered', 'luks2-layered' ] }

I would rename luks-all (and RbdEncryptionOptionsLUKSAll, etc) to luks-any.

>
>  ##
>  # @RbdEncryptionOptionsLUKSBase:
> @@ -3798,6 +3808,15 @@
>    'base': 'RbdEncryptionOptionsLUKSBase',
>    'data': { } }
>
> +##
> +# @RbdEncryptionOptionsLUKSAll:
> +#
> +# Since: 7.2
> +##
> +{ 'struct': 'RbdEncryptionOptionsLUKSAll',
> +  'base': 'RbdEncryptionOptionsLUKSBase',
> +  'data': { } }
> +
>  ##
>  # @RbdEncryptionCreateOptionsLUKS:
>  #
> @@ -3819,13 +3838,21 @@
>  ##
>  # @RbdEncryptionOptions:
>  #
> +# @format: Encryption format.
> +#
> +# @parent: Parent image encryption options (for cloned images).
> +#          Can be left unspecified if all ancestor images are encrypted
> +#          the same way as the child image.  (Since 7.2)

I would also add "... or not encrypted" here.

Thanks,

                Ilya
RE: [PATCH v1] block/rbd: Add support for layered encryption
Posted by Or Ozeri 1 year, 5 months ago

> -----Original Message-----
> From: Ilya Dryomov <idryomov@gmail.com>
> Sent: 11 November 2022 15:01
> To: Or Ozeri <ORO@il.ibm.com>
> Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; Danny Harnik
> <DANNYH@il.ibm.com>
> Subject: [EXTERNAL] Re: [PATCH v1] block/rbd: Add support for layered
> encryption
> 
> I don't understand the need for this char* array.  Is there a problem with
> putting the blob directly into luks_all_opts->passphrase just like the size is
> put into luks_all_opts->passphrase_size?
> 

luks_all_opts->passphrase has a const modifier.
Re: [PATCH v1] block/rbd: Add support for layered encryption
Posted by Ilya Dryomov 1 year, 5 months ago
On Sun, Nov 13, 2022 at 11:16 AM Or Ozeri <ORO@il.ibm.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ilya Dryomov <idryomov@gmail.com>
> > Sent: 11 November 2022 15:01
> > To: Or Ozeri <ORO@il.ibm.com>
> > Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; Danny Harnik
> > <DANNYH@il.ibm.com>
> > Subject: [EXTERNAL] Re: [PATCH v1] block/rbd: Add support for layered
> > encryption
> >
> > I don't understand the need for this char* array.  Is there a problem with
> > putting the blob directly into luks_all_opts->passphrase just like the size is
> > put into luks_all_opts->passphrase_size?
> >
>
> luks_all_opts->passphrase has a const modifier.

Hi Or,

That's really not a reason to make a dynamic memory allocation.  You
can just cast that const away but I suspect that the underlying issue
is that a const is missing somewhere else.  At the end of the day, QEMU
allocates a buffer for the passphrase when it's fetched via the secret
API -- that pointer should assign to const char* just fine.

Thanks,

                Ilya
RE: [PATCH v1] block/rbd: Add support for layered encryption
Posted by Or Ozeri 1 year, 5 months ago
I tried casting to non-const and it seems to work. Changed in v3 now.
I did not know that a const modifier could simply be cast out :)

> -----Original Message-----
> From: Ilya Dryomov <idryomov@gmail.com>
> Sent: 15 November 2022 14:00
> To: Or Ozeri <ORO@il.ibm.com>
> Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; Danny Harnik
> <DANNYH@il.ibm.com>
> Subject: [EXTERNAL] Re: [PATCH v1] block/rbd: Add support for layered
> encryption
> 
> On Sun, Nov 13, 2022 at 11:16 AM Or Ozeri <ORO@il.ibm.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Ilya Dryomov <idryomov@gmail.com>
> > > Sent: 11 November 2022 15:01
> > > To: Or Ozeri <ORO@il.ibm.com>
> > > Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; Danny Harnik
> > > <DANNYH@il.ibm.com>
> > > Subject: [EXTERNAL] Re: [PATCH v1] block/rbd: Add support for
> > > layered encryption
> > >
> > > I don't understand the need for this char* array.  Is there a
> > > problem with putting the blob directly into
> > > luks_all_opts->passphrase just like the size is put into luks_all_opts-
> >passphrase_size?
> > >
> >
> > luks_all_opts->passphrase has a const modifier.
> 
> Hi Or,
> 
> That's really not a reason to make a dynamic memory allocation.  You can just
> cast that const away but I suspect that the underlying issue is that a const is
> missing somewhere else.  At the end of the day, QEMU allocates a buffer for
> the passphrase when it's fetched via the secret API -- that pointer should
> assign to const char* just fine.
> 
> Thanks,
> 
>                 Ilya