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.
Ok, thanks for the advice, I'll modify it in the next version.
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 :|