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

[Kevin Wolf <kwolf@redhat.com>]

Am 12.03.2018 um 12:35 hat Daniel P. Berrangé geschrieben:
> 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.

That would be an easy way to implement preallocation support. But you
can't create LUKS volumes with PREALLOC_MODE_FULL today because it
doesn't support preallocation at all (there's no option for it in
&block_crypto_create_opts_luks), so implementing that is matter for a
different patch.

Kevin