Re: [Qemu-devel] [PATCH v4] LUKS: support preallocation

[Maxim Levitsky <mlevitsk@redhat.com>]

On Tue, 2019-07-16 at 14:41 +0200, Max Reitz wrote:
> On 16.07.19 10:15, Maxim Levitsky wrote:
> > preallocation=off and preallocation=metadata
> > both allocate luks header only, and preallocation=falloc/full
> > is passed to underlying file.
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534951
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  block/crypto.c       | 29 ++++++++++++++++++++++++++---
> >  qapi/block-core.json |  5 ++++-
> >  2 files changed, 30 insertions(+), 4 deletions(-)
> > 
> > 
> > Changes from V3: updated the blockdev-create description
> 
> Looks good functionally, but there is a syntax problem:
> 
> > diff --git a/block/crypto.c b/block/crypto.c
> > index 8237424ae6..034a645652 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> 
> [...]
> 
> > @@ -515,8 +523,11 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp)
> >          .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(luks_opts),
> >      };
> >  
> > +    if (luks_opts->has_preallocation)
> > +        preallocation = luks_opts->preallocation;
> 
> This lacks curly brackets.
In my defense, I am too much used to this due to kernel programming.
Eventually I will stop missing this.

> 
> > +
> >      ret = block_crypto_co_create_generic(bs, luks_opts->size, &create_opts,
> > -                                         errp);
> > +                                         preallocation, errp);
> >      if (ret < 0) {
> >          goto fail;
> >      }
> 
> [...]
> 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 0d43d4f37c..9c04d83fa2 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4205,13 +4205,16 @@
> >  #
> >  # @file             Node to create the image format on
> >  # @size             Size of the virtual disk in bytes
> > +# @preallocation    Preallocation mode for the new image (default: off;
> > +#                   allowed values: off/metadata/falloc/full (since: 4.2)
> 
> Also, this lacks a closing parenthesis somewhere.
True, I need more coffee.


> 
> Max
> 
> >  #
> >  # Since: 2.12
> >  ##
> >  { 'struct': 'BlockdevCreateOptionsLUKS',
> >    'base': 'QCryptoBlockCreateOptionsLUKS',
> >    'data': { 'file':             'BlockdevRef',
> > -            'size':             'size' } }
> > +            'size':             'size',
> > +            '*preallocation':   'PreallocMode' } }
> >  
> >  ##
> >  # @BlockdevCreateOptionsNfs:
> > 
> 
> 


Best regards,
	Maxim Levitsky