Re: [Qemu-devel] [PATCH 1/6] luks: Separate image file creation from formatting

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

On Fri, Mar 09, 2018 at 06:27:08PM +0100, Kevin Wolf wrote:
> The crypto driver used to create the image file in a callback from the
> crypto subsystem. If we want to implement .bdrv_co_create, this needs to
> go away because that callback will get a reference to an already
> existing block node.
> 
> Move the image file creation to block_crypto_create_generic().
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/crypto.c | 37 +++++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index e6095e7807..77871640cc 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -71,8 +71,6 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
>  
>  
>  struct BlockCryptoCreateData {
> -    const char *filename;
> -    QemuOpts *opts;
>      BlockBackend *blk;
>      uint64_t size;
>  };
> @@ -103,27 +101,13 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
>                                        Error **errp)
>  {
>      struct BlockCryptoCreateData *data = opaque;
> -    int ret;
>  
>      /* User provided size should reflect amount of space made
>       * available to the guest, so we must take account of that
>       * which will be used by the crypto header
>       */
> -    data->size += headerlen;
> -
> -    qemu_opt_set_number(data->opts, BLOCK_OPT_SIZE, data->size, &error_abort);
> -    ret = bdrv_create_file(data->filename, data->opts, errp);
> -    if (ret < 0) {
> -        return -1;
> -    }
> -
> -    data->blk = blk_new_open(data->filename, NULL, NULL,
> -                             BDRV_O_RDWR | BDRV_O_PROTOCOL, errp);
> -    if (!data->blk) {
> -        return -1;
> -    }
> -
> -    return 0;
> +    return blk_truncate(data->blk, data->size + headerlen, PREALLOC_MODE_OFF,
> +                        errp);
>  }

I wonder if we should pass in the value of the PREALLOC_MODE from the
block_crypto_create_generic() function via BlockCryptoCreateData, so
that we honour it. By using PREALLOC_MODE_OFF here, all LUKS volumes
created with PREALLOC_MODE_FULL are going to have several MB of
unallocated extents at the end of the file.

> 
>  
> @@ -333,11 +317,10 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,
>      struct BlockCryptoCreateData data = {
>          .size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
>                           BDRV_SECTOR_SIZE),
> -        .opts = opts,
> -        .filename = filename,
>      };
>      QDict *cryptoopts;
>  
> +    /* Parse options */
>      cryptoopts = qemu_opts_to_qdict(opts, NULL);
>  
>      create_opts = block_crypto_create_opts_init(format, cryptoopts, errp);
> @@ -345,6 +328,20 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,
>          return -1;
>      }
>  
> +    /* Create protocol layer */
> +    ret = bdrv_create_file(filename, opts, errp);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    data.blk = blk_new_open(filename, NULL, NULL,
> +                            BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
> +                            errp);
> +    if (!data.blk) {
> +        return -EINVAL;
> +    }
> +
> +    /* Create format layer */
>      crypto = qcrypto_block_create(create_opts, NULL,
>                                    block_crypto_init_func,
>                                    block_crypto_write_func,
> -- 
> 2.13.6
> 

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 :|