[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 09/11] block/qcow2: implement blockdev-amend
From: |
Maxim Levitsky |
Subject: |
Re: [PATCH v2 09/11] block/qcow2: implement blockdev-amend |
Date: |
Fri, 08 Nov 2019 17:14:32 +0200 |
On Mon, 2019-10-07 at 10:04 +0200, Markus Armbruster wrote:
> Max Reitz <address@hidden> writes:
>
> > On 13.09.19 00:30, Maxim Levitsky wrote:
> > > Currently only for changing crypto parameters
> >
> > Yep, that elegantly avoids most of the problems we’d have otherwise. :-)
> >
> > > Signed-off-by: Maxim Levitsky <address@hidden>
>
> [...]
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index 4a6db98938..0eb4e45168 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -4294,6 +4294,7 @@
> > > # Driver specific image creation options for qcow2.
> > > #
> > > # @file Node to create the image format on
> > > +# Mandatory for create
> > > # @data-file Node to use as an external data file in which all
> > > guest
> > > # data is stored so that only metadata remains in the
> > > qcow2
> > > # file (since: 4.0)
> > > @@ -4301,6 +4302,7 @@
> > > # standalone (read-only) raw image without looking at
> > > qcow2
> > > # metadata (default: false; since: 4.0)
> > > # @size Size of the virtual disk in bytes
> > > +# Mandatory for create
> > > # @version Compatibility level (default: v3)
> > > # @backing-file File name of the backing file if a backing file
> > > # should be used
> > > @@ -4315,10 +4317,10 @@
> > > # Since: 2.12
> > > ##
> > > { 'struct': 'BlockdevCreateOptionsQcow2',
> > > - 'data': { 'file': 'BlockdevRef',
> > > + 'data': { '*file': 'BlockdevRef',
> > > '*data-file': 'BlockdevRef',
> > > '*data-file-raw': 'bool',
> > > - 'size': 'size',
> > > + '*size': 'size',
> > > '*version': 'BlockdevQcow2Version',
> > > '*backing-file': 'str',
> > > '*backing-fmt': 'BlockdevDriver',
> > >
>
> My comments to the previous patch apply.
>
> > Making these fields optional makes me wonder whether it actually make
> > sense to have both create and amend share a single QAPI structure.
> > Wouldn’t it better to have a separate amend structure that then actually
> > reflects what we support? (This would also solve the problem of how to
> > express in the code what is and what isn’t supported through
> > blockdev-amend.)
>
> Good point. As is, the schema is rather confusing, at least to me. We
> reduce or avoid the confusion in documentation or in code. I'd prefer
> code unless it leads to too much duplication. "Too much" is of course
> subjective. Maxim, would you mind exploring it for us?
Nothing against having a separate amend structure, I actually prefer this,
and I don't think it will complicate the code.
Best regards,
Maxim Levitsky
- Re: [PATCH v2 09/11] block/qcow2: implement blockdev-amend,
Maxim Levitsky <=