Re: [PATCH V3 5/6] block/rbd: add write zeroes support

[Peter Lieven]

> Am 26.06.2021 um 17:57 schrieb Ilya Dryomov :
> 
> On Mon, Jun 21, 2021 at 10:49 AM Peter Lieven  wrote:
>> 
>>> Am 18.06.21 um 12:34 schrieb Ilya Dryomov:
>>> On Fri, Jun 18, 2021 at 11:00 AM Peter Lieven  wrote:
 Am 16.06.21 um 14:34 schrieb Ilya Dryomov:
> On Wed, May 19, 2021 at 4:28 PM Peter Lieven  wrote:
>> Signed-off-by: Peter Lieven 
>> ---
>>  block/rbd.c | 37 -
>>  1 file changed, 36 insertions(+), 1 deletion(-)
>> 
>> diff --git a/block/rbd.c b/block/rbd.c
>> index 0d8612a988..ee13f08a74 100644
>> --- a/block/rbd.c
>> +++ b/block/rbd.c
>> @@ -63,7 +63,8 @@ typedef enum {
>>  RBD_AIO_READ,
>>  RBD_AIO_WRITE,
>>  RBD_AIO_DISCARD,
>> -RBD_AIO_FLUSH
>> +RBD_AIO_FLUSH,
>> +RBD_AIO_WRITE_ZEROES
>>  } RBDAIOCmd;
>> 
>>  typedef struct BDRVRBDState {
>> @@ -705,6 +706,10 @@ static int qemu_rbd_open(BlockDriverState *bs, 
>> QDict *options, int flags,
>>  }
>>  }
>> 
>> +#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES
>> +bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
> I wonder if we should also set BDRV_REQ_NO_FALLBACK here since librbd
> does not really have a notion of non-efficient explicit zeroing.
 
 This is only true if thick provisioning is supported which is in Octopus 
 onwards, right?
>>> Since Pacific, I think.
>>> 
 So it would only be correct to set this if thick provisioning is supported 
 otherwise we could
 
 fail with ENOTSUP and then qemu emulates the zeroing with plain writes.
>>> I actually had a question about that.  Why are you returning ENOTSUP
>>> in case BDRV_REQ_MAY_UNMAP is not specified and that can't be fulfilled
>>> because librbd is too old for RBD_WRITE_ZEROES_FLAG_THICK_PROVISION?
>>> 
>>> My understanding has always been that BDRV_REQ_MAY_UNMAP is just
>>> a hint.  Deallocating if BDRV_REQ_MAY_UNMAP is specified is not nice
>>> but should be perfectly acceptable.  It is certainly better than
>>> returning ENOTSUP, particularly if ENOTSUP causes Qemu to do plain
>>> zeroing.
>> 
>> 
>> I think this was introduced to support different provisioning modes. If 
>> BDRV_REQ_MAY_UNMAP is not set
>> 
>> the caller of bdrv_write_zeroes expects that the driver does thick 
>> provisioning. If the driver cannot handle that (efficiently)
>> 
>> qemu does a plain zero write.
>> 
>> 
>> I am still not fully understanding the meaning of the BDRV_REQ_NO_FALLBACK 
>> flag. The original commit states that it was introduced for qemu-img to 
>> efficiently
>> 
>> zero out the target and avoid the slow fallback. When I last worked on 
>> qemu-img convert I remember that there was a call to zero out the target if 
>> bdrv_has_zero_init
>> 
>> is not 1. It seems hat meanwhile a target_is_zero cmdline switch for 
>> qemu-img convert was added to let the user assert that a preexisting target 
>> is zero.
>> 
>> Maybe someone can help here if it would be right to set BDRV_REQ_NO_FALLBACK 
>> for rbd in either of the 2 cases (thick provisioning is support or not)?
> 
> Since no one spoke up I think we should
> 
> a) remove the BDRV_REQ_MAY_UNMAP check in qemu_rbd_co_pwrite_zeroes()
>   and as a consequence always unmap if librbd is too old
> 
>   It's not clear what qemu's expectation is but in general Write
>   Zeroes is allowed to unmap.  The only guarantee is that subsequent
>   reads return zeroes, everything else is a hint.  This is how it is
>   specified in the kernel and in the NVMe spec.
> 
>   In particular, block/nvme.c implements it as follows:
> 
>   if (flags & BDRV_REQ_MAY_UNMAP) {
>   cdw12 |= (1 << 25);
>   }
> 
>   This sets the Deallocate bit.  But if it's not set, the device may
>   still deallocate:
> 
>   """
>   If the Deallocate bit (CDW12.DEAC) is set to '1' in a Write Zeroes
>   command, and the namespace supports clearing all bytes to 0h in the
>   values read (e.g., bits 2:0 in the DLFEAT field are set to 001b)
>   from a deallocated logical block and its metadata (excluding
>   protection information), then for each specified logical block, the
>   controller:
>   - should deallocate that logical block;
> 
>   ...
> 
>   If the Deallocate bit is cleared to '0' in a Write Zeroes command,
>   and the namespace supports clearing all bytes to 0h in the values
>   read (e.g., bits 2:0 in the DLFEAT field are set to 001b) from
>   a deallocated logical block and its metadata (excluding protection
>   information), then, for each specified logical block, the
>   controller:
>   - may deallocate that logical block;
>   """
> 
>   
> https://nvmexpress.org/wp-content/uploads/NVM-Express-NVM-Command-Set-Specification-2021.06.02-Ratified-1.pdf
> 
> b) set BDRV_REQ_NO_FALLBACK in supported_zero_flags
> 
>   Again, it's not clear what qemu expects here, but without it we end
>   up in a ridiculous situation where specifying the "don't allow slow
>   fallback" 

Re: [PATCH v2] block/rbd: Add support for rbd image encryption

[Ilya Dryomov]

On Sun, Jun 27, 2021 at 1:46 PM Or Ozeri  wrote:
>
> Starting from ceph Pacific, RBD has built-in support for image-level 
> encryption.
> Currently supported formats are LUKS version 1 and 2.
>
> There are 2 new relevant librbd APIs for controlling encryption, both expect 
> an
> open image context:
>
> rbd_encryption_format: formats an image (i.e. writes the LUKS header)
> rbd_encryption_load: loads encryptor/decryptor to the image IO stack
>
> This commit extends the qemu rbd driver API to support the above.
>
> Signed-off-by: Or Ozeri 
> ---
> v2: handle encryption info only for the case where encryption is not loaded
> ---
>  block/rbd.c  | 361 ++-
>  qapi/block-core.json | 110 -
>  2 files changed, 465 insertions(+), 6 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index f098a89c7b..8edd1be49e 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -73,6 +73,18 @@
>  #define LIBRBD_USE_IOVEC 0
>  #endif
>
> +#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8
> +
> +static const char rbd_luks_header_verification[
> +RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1
> +};
> +
> +static const char rbd_luks2_header_verification[
> +RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
> +};
> +
>  typedef enum {
>  RBD_AIO_READ,
>  RBD_AIO_WRITE,
> @@ -341,6 +353,203 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
>  }
>  }
>
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION
> +static int qemu_rbd_convert_luks_options(
> +RbdEncryptionOptionsLUKSBase *luks_opts,
> +char **passphrase,
> +size_t *passphrase_len,
> +Error **errp)
> +{
> +return qcrypto_secret_lookup(luks_opts->key_secret, (uint8_t 
> **)passphrase,
> + passphrase_len, errp);
> +}
> +
> +static int qemu_rbd_convert_luks_create_options(
> +RbdEncryptionCreateOptionsLUKSBase *luks_opts,
> +rbd_encryption_algorithm_t *alg,
> +char **passphrase,
> +size_t *passphrase_len,
> +Error **errp)
> +{
> +int r = 0;
> +
> +r = qemu_rbd_convert_luks_options(
> +qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),
> +passphrase, passphrase_len, errp);
> +if (r < 0) {
> +return r;
> +}
> +
> +if (luks_opts->has_cipher_alg) {
> +switch (luks_opts->cipher_alg) {
> +case QCRYPTO_CIPHER_ALG_AES_128: {
> +*alg = RBD_ENCRYPTION_ALGORITHM_AES128;
> +break;
> +}
> +case QCRYPTO_CIPHER_ALG_AES_256: {
> +*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
> +break;
> +}
> +default: {
> +r = -ENOTSUP;
> +error_setg_errno(errp, -r, "unknown encryption algorithm: 
> %u",
> + luks_opts->cipher_alg);
> +return r;
> +}
> +}
> +} else {
> +/* default alg */
> +*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
> +}
> +
> +return 0;
> +}
> +
> +static int qemu_rbd_encryption_format(rbd_image_t image,
> +  RbdEncryptionCreateOptions *encrypt,
> +  Error **errp)
> +{
> +int r = 0;
> +g_autofree char *passphrase = NULL;
> +size_t passphrase_len;
> +rbd_encryption_format_t format;
> +rbd_encryption_options_t opts;
> +rbd_encryption_luks1_format_options_t luks_opts;
> +rbd_encryption_luks2_format_options_t luks2_opts;
> +size_t opts_size;
> +uint64_t raw_size, effective_size;
> +
> +r = rbd_get_size(image, _size);
> +if (r < 0) {
> +error_setg_errno(errp, -r, "cannot get raw image size");
> +return r;
> +}
> +
> +switch (encrypt->format) {
> +case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS: {
> +memset(_opts, 0, sizeof(luks_opts));
> +format = RBD_ENCRYPTION_FORMAT_LUKS1;
> +opts = _opts;
> +opts_size = sizeof(luks_opts);
> +r = qemu_rbd_convert_luks_create_options(
> +
> qapi_RbdEncryptionCreateOptionsLUKS_base(>u.luks),
> +_opts.alg, , _len, errp);
> +if (r < 0) {
> +return r;
> +}
> +luks_opts.passphrase = passphrase;
> +luks_opts.passphrase_size = passphrase_len;
> +break;
> +}
> +case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: {
> +memset(_opts, 0, sizeof(luks2_opts));
> +format = RBD_ENCRYPTION_FORMAT_LUKS2;
> +opts = _opts;
> +opts_size = sizeof(luks2_opts);
> +r = qemu_rbd_convert_luks_create_options(
> +qapi_RbdEncryptionCreateOptionsLUKS2_base(
> +>u.luks2),
> +   

[PATCH v2] block/rbd: Add support for rbd image encryption

[Or Ozeri]

Starting from ceph Pacific, RBD has built-in support for image-level encryption.
Currently supported formats are LUKS version 1 and 2.

There are 2 new relevant librbd APIs for controlling encryption, both expect an
open image context:

rbd_encryption_format: formats an image (i.e. writes the LUKS header)
rbd_encryption_load: loads encryptor/decryptor to the image IO stack

This commit extends the qemu rbd driver API to support the above.

Signed-off-by: Or Ozeri 
---
v2: handle encryption info only for the case where encryption is not loaded
---
 block/rbd.c  | 361 ++-
 qapi/block-core.json | 110 -
 2 files changed, 465 insertions(+), 6 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index f098a89c7b..8edd1be49e 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -73,6 +73,18 @@
 #define LIBRBD_USE_IOVEC 0
 #endif
 
+#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8
+
+static const char rbd_luks_header_verification[
+RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
+'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1
+};
+
+static const char rbd_luks2_header_verification[
+RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
+'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
+};
+
 typedef enum {
 RBD_AIO_READ,
 RBD_AIO_WRITE,
@@ -341,6 +353,203 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
 }
 }
 
+#ifdef LIBRBD_SUPPORTS_ENCRYPTION
+static int qemu_rbd_convert_luks_options(
+RbdEncryptionOptionsLUKSBase *luks_opts,
+char **passphrase,
+size_t *passphrase_len,
+Error **errp)
+{
+return qcrypto_secret_lookup(luks_opts->key_secret, (uint8_t **)passphrase,
+ passphrase_len, errp);
+}
+
+static int qemu_rbd_convert_luks_create_options(
+RbdEncryptionCreateOptionsLUKSBase *luks_opts,
+rbd_encryption_algorithm_t *alg,
+char **passphrase,
+size_t *passphrase_len,
+Error **errp)
+{
+int r = 0;
+
+r = qemu_rbd_convert_luks_options(
+qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),
+passphrase, passphrase_len, errp);
+if (r < 0) {
+return r;
+}
+
+if (luks_opts->has_cipher_alg) {
+switch (luks_opts->cipher_alg) {
+case QCRYPTO_CIPHER_ALG_AES_128: {
+*alg = RBD_ENCRYPTION_ALGORITHM_AES128;
+break;
+}
+case QCRYPTO_CIPHER_ALG_AES_256: {
+*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
+break;
+}
+default: {
+r = -ENOTSUP;
+error_setg_errno(errp, -r, "unknown encryption algorithm: %u",
+ luks_opts->cipher_alg);
+return r;
+}
+}
+} else {
+/* default alg */
+*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
+}
+
+return 0;
+}
+
+static int qemu_rbd_encryption_format(rbd_image_t image,
+  RbdEncryptionCreateOptions *encrypt,
+  Error **errp)
+{
+int r = 0;
+g_autofree char *passphrase = NULL;
+size_t passphrase_len;
+rbd_encryption_format_t format;
+rbd_encryption_options_t opts;
+rbd_encryption_luks1_format_options_t luks_opts;
+rbd_encryption_luks2_format_options_t luks2_opts;
+size_t opts_size;
+uint64_t raw_size, effective_size;
+
+r = rbd_get_size(image, _size);
+if (r < 0) {
+error_setg_errno(errp, -r, "cannot get raw image size");
+return r;
+}
+
+switch (encrypt->format) {
+case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS: {
+memset(_opts, 0, sizeof(luks_opts));
+format = RBD_ENCRYPTION_FORMAT_LUKS1;
+opts = _opts;
+opts_size = sizeof(luks_opts);
+r = qemu_rbd_convert_luks_create_options(
+qapi_RbdEncryptionCreateOptionsLUKS_base(>u.luks),
+_opts.alg, , _len, errp);
+if (r < 0) {
+return r;
+}
+luks_opts.passphrase = passphrase;
+luks_opts.passphrase_size = passphrase_len;
+break;
+}
+case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: {
+memset(_opts, 0, sizeof(luks2_opts));
+format = RBD_ENCRYPTION_FORMAT_LUKS2;
+opts = _opts;
+opts_size = sizeof(luks2_opts);
+r = qemu_rbd_convert_luks_create_options(
+qapi_RbdEncryptionCreateOptionsLUKS2_base(
+>u.luks2),
+_opts.alg, , _len, errp);
+if (r < 0) {
+return r;
+}
+luks2_opts.passphrase = passphrase;
+luks2_opts.passphrase_size = passphrase_len;
+break;
+}
+default: {
+r = -ENOTSUP;
+error_setg_errno(
+errp, -r, 

Re: [PATCH] block/rbd: Add support for rbd image encryption

[Ilya Dryomov]

On Sun, Jun 27, 2021 at 1:09 PM Or Ozeri  wrote:
>
> Should I still leave the encryption format part of the state, and just not 
> report it? or should I also remove it from the state?

I'd remove it, reverting to the previous version of the patch
except instead of fetching the size with rbd_get_size() and storing
it in a local variable keep using s->image_size.

Thanks,

Ilya

RE: [PATCH] block/rbd: Add support for rbd image encryption

[Or Ozeri]

Should I still leave the encryption format part of the state, and just not report it? or should I also remove it from the state?-"Ilya Dryomov"  wrote: -To: "Or Ozeri" From: "Ilya Dryomov" Date: 06/27/2021 02:00PMCc: qemu-de...@nongnu.org, qemu-block@nongnu.org, kw...@redhat.com, "Mykola Golub" , "Danny Harnik" , "Daniel P. Berrangé" Subject: [EXTERNAL] Re: [PATCH] block/rbd: Add support for rbd image encryptionOn Sun, Jun 27, 2021 at 10:44 AM Or Ozeri  wrote:>> Ilya,>> I fixed according to your suggestions, except for the passphrase zeroing.> Looks like it's not a one-liner, but rather a long list of ifdefs to try and cover all possible platforms/compilers (this is what I've seen they do in k5-int.h).> I didn't want to copy this into rbd.c.> I guess that the right solution would be adding a qemu utility function (not sure where exactly), but anyways perhaps this, like the changes I previously made to raw_format.c, should be made in different patch.Hi Or,Yes, given that there doesn't seem to be an existing straightforwardAPI for it, I don't think it is a blocker.  Just something to keep inmind.You also implemented one change that I didn't suggest -- storingthe encryption status in BDRVRBDState.  BTW it is a good practiceto include the version in the subject (e.g. [PATCH v3] ...) anda per-version changelog in the description.The way the encryption format is reported in this patch actually begsmore questions because now I think we need to differentiate between anencrypted image for which the encryption profile has been loaded andone for which it hasn't been loaded and probably also report the rawsize...The previous approach of reporting the encryption format only for "raw"images was a good starting point IMO.  I'd keep the bit that switchesfrom rbd_get_size() to s->image_size and drop everything else for now.>> Thanks,> Or>> -"Or Ozeri"  wrote: -> To: qemu-de...@nongnu.org> From: "Or Ozeri" > Date: 06/27/2021 11:31AM> Cc: qemu-block@nongnu.org, o...@il.ibm.com, kw...@redhat.com, to.my.troc...@gmail.com, dan...@il.ibm.com, berra...@redhat.com, idryo...@gmail.com> Subject: [PATCH] block/rbd: Add support for rbd image encryption>> Starting from ceph Pacific, RBD has built-in support for image-level encryption.> Currently supported formats are LUKS version 1 and 2.>> There are 2 new relevant librbd APIs for controlling encryption, both expect an> open image context:>> rbd_encryption_format: formats an image (i.e. writes the LUKS header)> rbd_encryption_load: loads encryptor/decryptor to the image IO stack>> This commit extends the qemu rbd driver API to support the above.>> Signed-off-by: Or Ozeri > --->  block/rbd.c          | 380 ++->  qapi/block-core.json | 110 ->  2 files changed, 484 insertions(+), 6 deletions(-)>> diff --git a/block/rbd.c b/block/rbd.c> index f098a89c7b..dadecaf3da 100644> --- a/block/rbd.c> +++ b/block/rbd.c> @@ -73,6 +73,18 @@>  #define LIBRBD_USE_IOVEC 0>  #endif>> +#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8> +> +static const char rbd_luks_header_verification[> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {> +    'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1> +};> +> +static const char rbd_luks2_header_verification[> +        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {> +    'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2> +};> +>  typedef enum {>      RBD_AIO_READ,>      RBD_AIO_WRITE,> @@ -106,6 +118,7 @@ typedef struct BDRVRBDState {>      char *snap;>      char *namespace;>      uint64_t image_size;> +    ImageInfoSpecificRbd image_info;>  } BDRVRBDState;>>  static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,> @@ -341,6 +354,207 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)>      }>  }>> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION> +static int qemu_rbd_convert_luks_options(> +        RbdEncryptionOptionsLUKSBase *luks_opts,> +        char **passphrase,> +        size_t *passphrase_len,> +        Error **errp)> +{> +    return qcrypto_secret_lookup(luks_opts->key_secret, (uint8_t **)passphrase,> +                                 passphrase_len, errp);> +}> +> +static int qemu_rbd_convert_luks_create_options(> +        RbdEncryptionCreateOptionsLUKSBase *luks_opts,> +        rbd_encryption_algorithm_t *alg,> +        char **passphrase,> +        size_t *passphrase_len,> +        Error **errp)> +{> +    int r = 0;> +> +    r = qemu_rbd_convert_luks_options(> +            qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),> +            passphrase, passphrase_len, errp);> +    if (r < 0) {> +        return r;> +    }> +> +    if (luks_opts->has_cipher_alg) {> +        switch (luks_opts->cipher_alg) {> +            case QCRYPTO_CIPHER_ALG_AES_128: {> +                *alg = 

Re: [PATCH] block/rbd: Add support for rbd image encryption

[Ilya Dryomov]

On Sun, Jun 27, 2021 at 10:44 AM Or Ozeri  wrote:
>
> Ilya,
>
> I fixed according to your suggestions, except for the passphrase zeroing.
> Looks like it's not a one-liner, but rather a long list of ifdefs to try and 
> cover all possible platforms/compilers (this is what I've seen they do in 
> k5-int.h).
> I didn't want to copy this into rbd.c.
> I guess that the right solution would be adding a qemu utility function (not 
> sure where exactly), but anyways perhaps this, like the changes I previously 
> made to raw_format.c, should be made in different patch.

Hi Or,

Yes, given that there doesn't seem to be an existing straightforward
API for it, I don't think it is a blocker.  Just something to keep in
mind.

You also implemented one change that I didn't suggest -- storing
the encryption status in BDRVRBDState.  BTW it is a good practice
to include the version in the subject (e.g. [PATCH v3] ...) and
a per-version changelog in the description.

The way the encryption format is reported in this patch actually begs
more questions because now I think we need to differentiate between an
encrypted image for which the encryption profile has been loaded and
one for which it hasn't been loaded and probably also report the raw
size...

The previous approach of reporting the encryption format only for "raw"
images was a good starting point IMO.  I'd keep the bit that switches
from rbd_get_size() to s->image_size and drop everything else for now.

>
> Thanks,
> Or
>
> -"Or Ozeri"  wrote: -
> To: qemu-de...@nongnu.org
> From: "Or Ozeri" 
> Date: 06/27/2021 11:31AM
> Cc: qemu-block@nongnu.org, o...@il.ibm.com, kw...@redhat.com, 
> to.my.troc...@gmail.com, dan...@il.ibm.com, berra...@redhat.com, 
> idryo...@gmail.com
> Subject: [PATCH] block/rbd: Add support for rbd image encryption
>
> Starting from ceph Pacific, RBD has built-in support for image-level 
> encryption.
> Currently supported formats are LUKS version 1 and 2.
>
> There are 2 new relevant librbd APIs for controlling encryption, both expect 
> an
> open image context:
>
> rbd_encryption_format: formats an image (i.e. writes the LUKS header)
> rbd_encryption_load: loads encryptor/decryptor to the image IO stack
>
> This commit extends the qemu rbd driver API to support the above.
>
> Signed-off-by: Or Ozeri 
> ---
>  block/rbd.c  | 380 ++-
>  qapi/block-core.json | 110 -
>  2 files changed, 484 insertions(+), 6 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index f098a89c7b..dadecaf3da 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -73,6 +73,18 @@
>  #define LIBRBD_USE_IOVEC 0
>  #endif
>
> +#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8
> +
> +static const char rbd_luks_header_verification[
> +RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1
> +};
> +
> +static const char rbd_luks2_header_verification[
> +RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
> +'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
> +};
> +
>  typedef enum {
>  RBD_AIO_READ,
>  RBD_AIO_WRITE,
> @@ -106,6 +118,7 @@ typedef struct BDRVRBDState {
>  char *snap;
>  char *namespace;
>  uint64_t image_size;
> +ImageInfoSpecificRbd image_info;
>  } BDRVRBDState;
>
>  static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
> @@ -341,6 +354,207 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
>  }
>  }
>
> +#ifdef LIBRBD_SUPPORTS_ENCRYPTION
> +static int qemu_rbd_convert_luks_options(
> +RbdEncryptionOptionsLUKSBase *luks_opts,
> +char **passphrase,
> +size_t *passphrase_len,
> +Error **errp)
> +{
> +return qcrypto_secret_lookup(luks_opts->key_secret, (uint8_t 
> **)passphrase,
> + passphrase_len, errp);
> +}
> +
> +static int qemu_rbd_convert_luks_create_options(
> +RbdEncryptionCreateOptionsLUKSBase *luks_opts,
> +rbd_encryption_algorithm_t *alg,
> +char **passphrase,
> +size_t *passphrase_len,
> +Error **errp)
> +{
> +int r = 0;
> +
> +r = qemu_rbd_convert_luks_options(
> +qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),
> +passphrase, passphrase_len, errp);
> +if (r < 0) {
> +return r;
> +}
> +
> +if (luks_opts->has_cipher_alg) {
> +switch (luks_opts->cipher_alg) {
> +case QCRYPTO_CIPHER_ALG_AES_128: {
> +*alg = RBD_ENCRYPTION_ALGORITHM_AES128;
> +break;
> +}
> +case QCRYPTO_CIPHER_ALG_AES_256: {
> +*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
> +break;
> +}
> +default: {
> +r = -ENOTSUP;
> +error_setg_errno(errp, -r, "unknown encryption algorithm: 
> %u",
> + luks_opts->cipher_alg);
> +return r;
> +}
> +}

Re: [PATCH] block/rbd: Add support for rbd image encryption

[Or Ozeri]

Ilya,I fixed according to your suggestions, except for the passphrase zeroing.Looks like it's not a one-liner, but rather a long list of ifdefs to try and cover all possible platforms/compilers (this is what I've seen they do in k5-int.h).I didn't want to copy this into rbd.c.I guess that the right solution would be adding a qemu utility function (not sure where exactly), but anyways perhaps this, like the changes I previously made to raw_format.c, should be made in different patch.Thanks,Or-"Or Ozeri"  wrote: -To: qemu-de...@nongnu.orgFrom: "Or Ozeri" Date: 06/27/2021 11:31AMCc: qemu-block@nongnu.org, o...@il.ibm.com, kw...@redhat.com, to.my.troc...@gmail.com, dan...@il.ibm.com, berra...@redhat.com, idryo...@gmail.comSubject: [PATCH] block/rbd: Add support for rbd image encryptionStarting from ceph Pacific, RBD has built-in support for image-level encryption.Currently supported formats are LUKS version 1 and 2.There are 2 new relevant librbd APIs for controlling encryption, both expect anopen image context:rbd_encryption_format: formats an image (i.e. writes the LUKS header)rbd_encryption_load: loads encryptor/decryptor to the image IO stackThis commit extends the qemu rbd driver API to support the above.Signed-off-by: Or Ozeri --- block/rbd.c          | 380 ++- qapi/block-core.json | 110 - 2 files changed, 484 insertions(+), 6 deletions(-)diff --git a/block/rbd.c b/block/rbd.cindex f098a89c7b..dadecaf3da 100644--- a/block/rbd.c+++ b/block/rbd.c@@ -73,6 +73,18 @@ #define LIBRBD_USE_IOVEC 0 #endif +#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8++static const char rbd_luks_header_verification[+        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {+    'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1+};++static const char rbd_luks2_header_verification[+        RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {+    'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2+};+ typedef enum {     RBD_AIO_READ,     RBD_AIO_WRITE,@@ -106,6 +118,7 @@ typedef struct BDRVRBDState {     char *snap;     char *namespace;     uint64_t image_size;+    ImageInfoSpecificRbd image_info; } BDRVRBDState;  static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,@@ -341,6 +354,207 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)     } } +#ifdef LIBRBD_SUPPORTS_ENCRYPTION+static int qemu_rbd_convert_luks_options(+        RbdEncryptionOptionsLUKSBase *luks_opts,+        char **passphrase,+        size_t *passphrase_len,+        Error **errp)+{+    return qcrypto_secret_lookup(luks_opts->key_secret, (uint8_t **)passphrase,+                                 passphrase_len, errp);+}++static int qemu_rbd_convert_luks_create_options(+        RbdEncryptionCreateOptionsLUKSBase *luks_opts,+        rbd_encryption_algorithm_t *alg,+        char **passphrase,+        size_t *passphrase_len,+        Error **errp)+{+    int r = 0;++    r = qemu_rbd_convert_luks_options(+            qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),+            passphrase, passphrase_len, errp);+    if (r < 0) {+        return r;+    }++    if (luks_opts->has_cipher_alg) {+        switch (luks_opts->cipher_alg) {+            case QCRYPTO_CIPHER_ALG_AES_128: {+                *alg = RBD_ENCRYPTION_ALGORITHM_AES128;+                break;+            }+            case QCRYPTO_CIPHER_ALG_AES_256: {+                *alg = RBD_ENCRYPTION_ALGORITHM_AES256;+                break;+            }+            default: {+                r = -ENOTSUP;+                error_setg_errno(errp, -r, "unknown encryption algorithm: %u",+                                 luks_opts->cipher_alg);+                return r;+            }+        }+    } else {+        /* default alg */+        *alg = RBD_ENCRYPTION_ALGORITHM_AES256;+    }++    return 0;+}++static int qemu_rbd_encryption_format(rbd_image_t image,+                                      RbdEncryptionCreateOptions *encrypt,+                                      Error **errp)+{+    int r = 0;+    g_autofree char *passphrase = NULL;+    size_t passphrase_len;+    rbd_encryption_format_t format;+    rbd_encryption_options_t opts;+    rbd_encryption_luks1_format_options_t luks_opts;+    rbd_encryption_luks2_format_options_t luks2_opts;+    size_t opts_size;+    uint64_t raw_size, effective_size;++    r = rbd_get_size(image, _size);+    if (r < 0) {+        error_setg_errno(errp, -r, "cannot get raw image size");+        return r;+    }++    switch (encrypt->format) {+        case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS: {+            memset(_opts, 0, sizeof(luks_opts));+            format = RBD_ENCRYPTION_FORMAT_LUKS1;+            opts = _opts;+            opts_size = sizeof(luks_opts);+            r = qemu_rbd_convert_luks_create_options(+                    qapi_RbdEncryptionCreateOptionsLUKS_base(>u.luks),+                    _opts.alg, , _len, errp);+            if (r < 0) {+                return r;+            }+     

[PATCH] block/rbd: Add support for rbd image encryption

[Or Ozeri]

Starting from ceph Pacific, RBD has built-in support for image-level encryption.
Currently supported formats are LUKS version 1 and 2.

There are 2 new relevant librbd APIs for controlling encryption, both expect an
open image context:

rbd_encryption_format: formats an image (i.e. writes the LUKS header)
rbd_encryption_load: loads encryptor/decryptor to the image IO stack

This commit extends the qemu rbd driver API to support the above.

Signed-off-by: Or Ozeri 
---
 block/rbd.c  | 380 ++-
 qapi/block-core.json | 110 -
 2 files changed, 484 insertions(+), 6 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index f098a89c7b..dadecaf3da 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -73,6 +73,18 @@
 #define LIBRBD_USE_IOVEC 0
 #endif
 
+#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8
+
+static const char rbd_luks_header_verification[
+RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
+'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1
+};
+
+static const char rbd_luks2_header_verification[
+RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {
+'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2
+};
+
 typedef enum {
 RBD_AIO_READ,
 RBD_AIO_WRITE,
@@ -106,6 +118,7 @@ typedef struct BDRVRBDState {
 char *snap;
 char *namespace;
 uint64_t image_size;
+ImageInfoSpecificRbd image_info;
 } BDRVRBDState;
 
 static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
@@ -341,6 +354,207 @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
 }
 }
 
+#ifdef LIBRBD_SUPPORTS_ENCRYPTION
+static int qemu_rbd_convert_luks_options(
+RbdEncryptionOptionsLUKSBase *luks_opts,
+char **passphrase,
+size_t *passphrase_len,
+Error **errp)
+{
+return qcrypto_secret_lookup(luks_opts->key_secret, (uint8_t **)passphrase,
+ passphrase_len, errp);
+}
+
+static int qemu_rbd_convert_luks_create_options(
+RbdEncryptionCreateOptionsLUKSBase *luks_opts,
+rbd_encryption_algorithm_t *alg,
+char **passphrase,
+size_t *passphrase_len,
+Error **errp)
+{
+int r = 0;
+
+r = qemu_rbd_convert_luks_options(
+qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts),
+passphrase, passphrase_len, errp);
+if (r < 0) {
+return r;
+}
+
+if (luks_opts->has_cipher_alg) {
+switch (luks_opts->cipher_alg) {
+case QCRYPTO_CIPHER_ALG_AES_128: {
+*alg = RBD_ENCRYPTION_ALGORITHM_AES128;
+break;
+}
+case QCRYPTO_CIPHER_ALG_AES_256: {
+*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
+break;
+}
+default: {
+r = -ENOTSUP;
+error_setg_errno(errp, -r, "unknown encryption algorithm: %u",
+ luks_opts->cipher_alg);
+return r;
+}
+}
+} else {
+/* default alg */
+*alg = RBD_ENCRYPTION_ALGORITHM_AES256;
+}
+
+return 0;
+}
+
+static int qemu_rbd_encryption_format(rbd_image_t image,
+  RbdEncryptionCreateOptions *encrypt,
+  Error **errp)
+{
+int r = 0;
+g_autofree char *passphrase = NULL;
+size_t passphrase_len;
+rbd_encryption_format_t format;
+rbd_encryption_options_t opts;
+rbd_encryption_luks1_format_options_t luks_opts;
+rbd_encryption_luks2_format_options_t luks2_opts;
+size_t opts_size;
+uint64_t raw_size, effective_size;
+
+r = rbd_get_size(image, _size);
+if (r < 0) {
+error_setg_errno(errp, -r, "cannot get raw image size");
+return r;
+}
+
+switch (encrypt->format) {
+case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS: {
+memset(_opts, 0, sizeof(luks_opts));
+format = RBD_ENCRYPTION_FORMAT_LUKS1;
+opts = _opts;
+opts_size = sizeof(luks_opts);
+r = qemu_rbd_convert_luks_create_options(
+qapi_RbdEncryptionCreateOptionsLUKS_base(>u.luks),
+_opts.alg, , _len, errp);
+if (r < 0) {
+return r;
+}
+luks_opts.passphrase = passphrase;
+luks_opts.passphrase_size = passphrase_len;
+break;
+}
+case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: {
+memset(_opts, 0, sizeof(luks2_opts));
+format = RBD_ENCRYPTION_FORMAT_LUKS2;
+opts = _opts;
+opts_size = sizeof(luks2_opts);
+r = qemu_rbd_convert_luks_create_options(
+qapi_RbdEncryptionCreateOptionsLUKS2_base(
+>u.luks2),
+_opts.alg, , _len, errp);
+if (r < 0) {
+return r;
+}
+luks2_opts.passphrase = passphrase;
+