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

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

Am 16.07.2019 um 14:41 hat Max Reitz geschrieben:
> 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.
> 
> > +
> >      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.

You could also view it as having an opening parenthesis where there
should be another semicolon.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]