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

[Kevin Wolf <kwolf@redhat.com>]

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

Am 16.01.2018 um 19:59 hat Eric Blake geschrieben:
> On 01/11/2018 01:52 PM, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  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' ] }
> 
> Enums are allowed to start with digits while struct members are not; so
> you can get away with this naming.  Do we really want the names 0_10 and
> 1_1, or are there better names we could come up with (it already
> undergoes translation such that qemu-img reports 0.10 rather than 0_10).

Yeah, I don't like 0_10/1_1 much.

Either we allow dots in enum values so that we can keep 0.10/1.1, or
something completely different. I was considering 'version': 'int' with
2 and 3 as possible values, after all QMP is already rather low-level.

The question is just what to do with the command line. Will we deprecate
compat=0.10/1.1 there, too, and tell users to switch to whatever new
syntax we invent for QMP? Or are we planning to keep the "translation"
from the old syntax forever?

query-block cheated and just exposed it as a string.

> > +
> > +
> > +##
> > +# @BlockdevCreateOptionsQcow2:
> > +#
> > +# Driver specific image creation options for qcow2.
> > +#
> > +# TODO Describe fields
> 
> Hence this being RFC :)
> 
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'struct': 'BlockdevCreateOptionsQcow2',
> > +  'data': { 'size':             'size',
> 
> Is size mandatory even when we have a backing file specification?  It is
> not mandatory for qemu-img create; but on the other hand, I think I can
> live with requiring the QMP caller to supply a size.

The qemu-img create implementation of this is common code at least, but
we're in driver-specific definitions here, so every driver would have to
call some function to guess the size given a backing file string. With
the straightforward implementation of this series, it is really
mandatory because otherwise you'd get zero-sized images.

Accessing the backing file during image creation is also one of those
things that tend to cause surprises, so if we don't have to, I wouldn't
do that.

> > +            '*compat':          'BlockdevQcow2CompatLevel',
> > +            '*backing-file':    'str',
> 
> Given Dan's comments, perhaps name this one 'backing-str' to make it
> obvious that it is the string written into the qcow2 header, rather than
> the node we open as backing?

If you guys think that this is clearer, I can change it.

> Or, maybe we support an optional '*backing-node' that can be used for
> allowing a default size and backing string if not explicitly
> overridden?

Hm, it would make the interface a bit more complex. I'd try whether we
can do without it.

Kevin

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