Re: [v2 4/4] block: Support detached LUKS header creation for blockdev-create

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Dec 07, 2023 at 12:37:45AM +0800, Hyman Huang wrote:
> Provide the "detached-mode" option for detached LUKS header
> formatting.
> 
> To format the LUKS header on the pre-creating disk, example
> as follows:
> 
> 1. add a protocol blockdev node of LUKS header
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> > "filename":"/path/to/cipher.gluks" }}'
> 
> 2. add the secret for encrypting the cipher stored in LUKS
>    header above
> $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > "arguments":{"qom-type": "secret", "id":
> > "libvirt-1-storage-secret0", "data": "abc123"}}'
> 
> 3. format the disk node
> $ virsh qemu-monitor-command vm '{"execute":"blockdev-create",
> > "arguments":{"job-id":"job0", "options":{"driver":"luks",
> > "size":0, "file":"libvirt-1-storage", "detached-mode":true,
> > "cipher-alg":"aes-256",
> > "key-secret":"libvirt-3-storage-encryption-secret0"}}}'
> 
> Signed-off-by: Hyman Huang <yong.huang@smartx.com>
> ---
>  block/crypto.c       | 8 +++++++-
>  qapi/block-core.json | 5 ++++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 7d70349463..e77c49bd0c 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -667,10 +667,12 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>      BlockDriverState *bs = NULL;
>      QCryptoBlockCreateOptions create_opts;
>      PreallocMode preallocation = PREALLOC_MODE_OFF;
> +    int64_t size;
>      int ret;
>  
>      assert(create_options->driver == BLOCKDEV_DRIVER_LUKS);
>      luks_opts = &create_options->u.luks;
> +    size = luks_opts->size;
>  
>      bs = bdrv_co_open_blockdev_ref(luks_opts->file, errp);
>      if (bs == NULL) {
> @@ -686,7 +688,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
>          preallocation = luks_opts->preallocation;
>      }
>  
> -    ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
> +    if (luks_opts->detached_mode) {
> +        size = 0;
> +    }
> +
> +    ret = block_crypto_co_create_generic(bs, size, &create_opts,
>                                           preallocation, errp);
>      if (ret < 0) {
>          goto fail;
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 10be08d08f..1e7a7e1b05 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4952,13 +4952,16 @@
>  # @preallocation: Preallocation mode for the new image (since: 4.2)
>  #     (default: off; allowed values: off, metadata, falloc, full)
>  #
> +# @detached-mode: create a detached LUKS header. (since 9.0)
> +#
>  # Since: 2.12
>  ##
>  { 'struct': 'BlockdevCreateOptionsLUKS',
>    'base': 'QCryptoBlockCreateOptionsLUKS',
>    'data': { 'file':             'BlockdevRef',
>              'size':             'size',
> -            '*preallocation':   'PreallocMode' } }
> +            '*preallocation':   'PreallocMode',
> +            '*detached-mode':   'bool'}}

Using a bool flag here is insufficiently flexible. We need to be able to
honour preallocation of the payload device, while using a separate
header.

You need to make the existing 'file' optional, while also adding an
extra optional 'header' field. ie

  { 'struct': 'BlockdevCreateOptionsLUKS',
    'base': 'QCryptoBlockCreateOptionsLUKS',
    'data': { '*file':            'BlockdevRef',
              '*header':          'BlockdevRef',
              'size':             'size',
              '*preallocation':   'PreallocMode' } }


If 'preallocation' is requested, then we must enforce that 'file' is
non-NULL in the code.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|