Re: [Qemu-block] [Qemu-devel] [RFC PATCH 02/10] block/qapi: Add qcow2 create options to schema

[Kevin Wolf]

Am 12.01.2018 um 11:53 hat Daniel P. Berrange geschrieben:
> On Thu, Jan 11, 2018 at 08:52:17PM +0100, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> >  qapi/block-core.json | 33 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 32 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 1749376c61..9341f6708d 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -3320,6 +3320,37 @@
> >  { 'command': 'blockdev-del', 'data': { 'node-name': 'str' } }
> >  
> >  ##
> > +# @BlockdevQcow2CompatLevel:
> > +# @0_10:    The original QCOW2 format as introduced in qemu 0.10 (version 
> > 2)
> > +# @1_1:     The extended QCOW2 format as introduced in qemu 1.1 (version 3)
> > +#
> > +# Since: 2.10
> > +##
> > +{ 'enum': 'BlockdevQcow2CompatLevel',
> > +  'data': [ '0_10', '1_1' ] }
> > +
> > +
> > +##
> > +# @BlockdevCreateOptionsQcow2:
> > +#
> > +# Driver specific image creation options for qcow2.
> > +#
> > +# TODO Describe fields
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'struct': 'BlockdevCreateOptionsQcow2',
> > +  'data': { 'size':             'size',
> > +            '*compat':          'BlockdevQcow2CompatLevel',
> > +            '*backing-file':    'str',
> > +            '*backing-fmt':     'BlockdevDriver',
> 
> For anything non-trivial, the caller is going to have to stuff a
> JSON string into 'backing-file' value. It feels like we should
> be referencing 'BlockdevOptions' here in some manner.

Hm, that's an interesting question. For the image creation, this is
really treated as a string that is directly written into the image file,
without being parsed, so 'str' is the more correct type in this context.
However, when the backing file gets loaded, that string is in fact
parsed and we expect it to describe the same thing as BlockdevOptions.

If we get BlockdevOptions here, qemu would have to convert them into a
json:{...} string before writing the header of the new image.
Compatibility code would become a bit more complex because we'd have to
convert the existing string into BlockdevOptions, only to convert it
back to a string before we write it to the image file. And finally, the
1023 character limit of qcow2 becomes kind of unpredicatble when you
don't pass the string yourself.

So considering all of that, I still think that 'str' is the better
option here.

Kevin