| Message ID | 20180111195225.4226-3-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | x-blockdev-create for qcow2 | expand |
On Thu, Jan 11, 2018 at 08:52:17PM +0100, 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' ] } > + > + > +## > +# @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. > + '*encrypt': 'QCryptoBlockCreateOptions', > + '*cluster-size': 'size', > + '*preallocation': 'PreallocMode', > + '*lazy-refcounts': 'bool', > + '*refcount-bits': 'int' } } Regards, Daniel
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 <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' ] } > > + > > + > > +## > > +# @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
On Mon, Jan 15, 2018 at 02:38:48PM +0100, Kevin Wolf wrote: > 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 <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' ] } > > > + > > > + > > > +## > > > +# @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. Hmm, when we write the backing chain into the qcow2 header, we only want to write the 1st level of the backing chain. When we are creating the new qcow2 image, we could be pointing to a backing chain that goes many levels deep. So the actual creation process potentially needs to be given the full arbitrarily deep backing file eg in order that we can set 'encrypt.secret' for any encrypted images at at arbitrary level. IOW, I think we need to be able to pass BlockdevOptions here to specify the full deep chain, but then only the 1st level of these BlockdevOptions should get written into the qcow2 file header. Regards, Daniel
Am 15.01.2018 um 14:51 hat Daniel P. Berrange geschrieben: > On Mon, Jan 15, 2018 at 02:38:48PM +0100, Kevin Wolf wrote: > > 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 <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' ] } > > > > + > > > > + > > > > +## > > > > +# @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. > > Hmm, when we write the backing chain into the qcow2 header, we only > want to write the 1st level of the backing chain. That's a good point, too. References in BlockdevOptions are often mandatory, which conflicts with this. > When we are creating the new qcow2 image, we could be pointing to a backing > chain that goes many levels deep. So the actual creation process potentially > needs to be given the full arbitrarily deep backing file eg in order that > we can set 'encrypt.secret' for any encrypted images at at arbitrary level. But we don't even access the images in the backing chain during image creation. Why would we need a secret for them? > IOW, I think we need to be able to pass BlockdevOptions here to specify the > full deep chain, but then only the 1st level of these BlockdevOptions should > get written into the qcow2 file header. But what's the point of even passing the full chain if only the first layer is actually used? Kevin
On Mon, Jan 15, 2018 at 03:07:15PM +0100, Kevin Wolf wrote: > Am 15.01.2018 um 14:51 hat Daniel P. Berrange geschrieben: > > On Mon, Jan 15, 2018 at 02:38:48PM +0100, Kevin Wolf wrote: > > > 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 <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' ] } > > > > > + > > > > > + > > > > > +## > > > > > +# @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. > > > > Hmm, when we write the backing chain into the qcow2 header, we only > > want to write the 1st level of the backing chain. > > That's a good point, too. References in BlockdevOptions are often > mandatory, which conflicts with this. > > > When we are creating the new qcow2 image, we could be pointing to a backing > > chain that goes many levels deep. So the actual creation process potentially > > needs to be given the full arbitrarily deep backing file eg in order that > > we can set 'encrypt.secret' for any encrypted images at at arbitrary level. > > But we don't even access the images in the backing chain during image > creation. Why would we need a secret for them? Oh, i forgot that when qcow2 opens the just created image, it uses the O_NO_IO and O_NO_BACKING flags. So yeah, we're probably ok in actual fact. > > IOW, I think we need to be able to pass BlockdevOptions here to specify the > > full deep chain, but then only the 1st level of these BlockdevOptions should > > get written into the qcow2 file header. > > But what's the point of even passing the full chain if only the first > layer is actually used? Regards, Daniel
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). > + > + > +## > +# @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. > + '*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? Or, maybe we support an optional '*backing-node' that can be used for allowing a default size and backing string if not explicitly overridden? > + '*backing-fmt': 'BlockdevDriver', > + '*encrypt': 'QCryptoBlockCreateOptions', > + '*cluster-size': 'size', > + '*preallocation': 'PreallocMode', > + '*lazy-refcounts': 'bool', > + '*refcount-bits': 'int' } } > + > +## > # @BlockdevCreateDummy: > # > # FIXME To be removed. Only there to make the QAPI generator happy while we're > @@ -3365,7 +3396,7 @@ > 'null-aio': 'BlockdevCreateDummy', > 'null-co': 'BlockdevCreateDummy', > 'parallels': 'BlockdevCreateDummy', > - 'qcow2': 'BlockdevCreateDummy', > + 'qcow2': 'BlockdevCreateOptionsQcow2', > 'qcow': 'BlockdevCreateDummy', > 'qed': 'BlockdevCreateDummy', > 'quorum': 'BlockdevCreateDummy', >
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
On 01/16/2018 02:11 PM, Kevin Wolf wrote: >>> +{ '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. > I can live with a lower-level 'version':'int' for qcow2 creation over QMP. > 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? At a minimum, we'll have to keep the translation syntax for as long as a deprecation cycle with proper documentation is available (at least two releases); keeping it longer than that depends on whether we think the deprecation is worth the cleaner code in the long run. But we do have a deprecation policy, so we can start thinking about using that now so that in another year we can do a release that gets rid of the back-compat code. >>> + '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. Good point. So mandatory size at the QMP layer makes sense (qemu-img can still open multiple images to determine what size to pass to QMP under the hood). > >>> + '*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. Especially since you're convincing me that we DON'T want to open a backing node during this operation, I think backing-str is a bit clearer (of course, that's another place for command-line back-compat glue that we may want to deprecate over time). > >> 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. I'm fine if you can manage the series without having a backing-node argument.
On 2018-01-11 20:52, 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' ] } Just my two cents: I'd prefer 2 and 3 because I've never quite liked that people are supposed to remember some pretty random qemu version numbers anyway. Max
Am 29.01.2018 um 17:57 hat Max Reitz geschrieben: > On 2018-01-11 20:52, 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' ] } > > Just my two cents: I'd prefer 2 and 3 because I've never quite liked > that people are supposed to remember some pretty random qemu version > numbers anyway. Yeah. An enum with '2' and '3' wouldn't work for Eric's desire to have enum values starting in letters, though. I was originally planning to replace it with an int, but then it's not introspectable. An enum with 'v2' and 'v3' then? Kevin
On 2018-01-29 19:06, Kevin Wolf wrote: > Am 29.01.2018 um 17:57 hat Max Reitz geschrieben: >> On 2018-01-11 20:52, 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' ] } >> >> Just my two cents: I'd prefer 2 and 3 because I've never quite liked >> that people are supposed to remember some pretty random qemu version >> numbers anyway. > > Yeah. An enum with '2' and '3' wouldn't work for Eric's desire to have > enum values starting in letters, though. I was originally planning to > replace it with an int, but then it's not introspectable. Aw. > An enum with 'v2' and 'v3' then? Would work for me. Max
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', + '*encrypt': 'QCryptoBlockCreateOptions', + '*cluster-size': 'size', + '*preallocation': 'PreallocMode', + '*lazy-refcounts': 'bool', + '*refcount-bits': 'int' } } + +## # @BlockdevCreateDummy: # # FIXME To be removed. Only there to make the QAPI generator happy while we're @@ -3365,7 +3396,7 @@ 'null-aio': 'BlockdevCreateDummy', 'null-co': 'BlockdevCreateDummy', 'parallels': 'BlockdevCreateDummy', - 'qcow2': 'BlockdevCreateDummy', + 'qcow2': 'BlockdevCreateOptionsQcow2', 'qcow': 'BlockdevCreateDummy', 'qed': 'BlockdevCreateDummy', 'quorum': 'BlockdevCreateDummy',
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- qapi/block-core.json | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)