Message ID | 1498733831-15254-3-git-send-email-pl@kamp.de (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 43 insertions(+), 1 deletion(-) How does this make sense as a runtime option? What would happen if the image contains one compression format and I specify another one on the command line or in blockdev-add? Shouldn't it just be a create-time option and when you run qemu, it uses whatever format that image has? Kevin
Am 10.07.2017 um 15:10 schrieb Kevin Wolf: > Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 43 insertions(+), 1 deletion(-) > How does this make sense as a runtime option? What would happen if the > image contains one compression format and I specify another one on the > command line or in blockdev-add? > > Shouldn't it just be a create-time option and when you run qemu, it uses > whatever format that image has? I was asked to add it here. It is indeed only a create option and has no other effect. Peter
On Mon, Jul 10, 2017 at 03:24:02PM +0200, Peter Lieven wrote: > Am 10.07.2017 um 15:10 schrieb Kevin Wolf: > > Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: > > > Signed-off-by: Peter Lieven <pl@kamp.de> > > > --- > > > qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- > > > 1 file changed, 43 insertions(+), 1 deletion(-) > > How does this make sense as a runtime option? What would happen if the > > image contains one compression format and I specify another one on the > > command line or in blockdev-add? > > > > Shouldn't it just be a create-time option and when you run qemu, it uses > > whatever format that image has? > > I was asked to add it here. It is indeed only a create option and has no > other effect. I think there was some mis-understanding. I suggested adding QAPI schema to define the options needed during create, and then use QAPI functions when processing the create. In patch 4 you've just used the traditional qemu opts functions to query the parameters, so there's no validation that what's being processed actually matches the QAPI schema. The QAPI schema additions for create would need to be defined indepenantly of the runtime options too. Regards, Daniel
Am 10.07.2017 um 15:24 hat Peter Lieven geschrieben: > Am 10.07.2017 um 15:10 schrieb Kevin Wolf: > >Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: > >>Signed-off-by: Peter Lieven <pl@kamp.de> > >>--- > >> qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- > >> 1 file changed, 43 insertions(+), 1 deletion(-) > >How does this make sense as a runtime option? What would happen if the > >image contains one compression format and I specify another one on the > >command line or in blockdev-add? > > > >Shouldn't it just be a create-time option and when you run qemu, it uses > >whatever format that image has? > > I was asked to add it here. It is indeed only a create option and has > no other effect. But if it's only a create option, why is it added to blockdev-add? Kevin
Am 10.07.2017 um 15:30 schrieb Kevin Wolf: > Am 10.07.2017 um 15:24 hat Peter Lieven geschrieben: >> Am 10.07.2017 um 15:10 schrieb Kevin Wolf: >>> Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>>> --- >>>> qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 43 insertions(+), 1 deletion(-) >>> How does this make sense as a runtime option? What would happen if the >>> image contains one compression format and I specify another one on the >>> command line or in blockdev-add? >>> >>> Shouldn't it just be a create-time option and when you run qemu, it uses >>> whatever format that image has? >> I was asked to add it here. It is indeed only a create option and has >> no other effect. > But if it's only a create option, why is it added to blockdev-add? As Daniel wrote its maybe a misunderstanding. He pointed me to the crypto extension that was recently added and the structs there. As I said I might need help with the QAPI stuff... Peter
Am 10.07.2017 um 15:30 schrieb Kevin Wolf: > Am 10.07.2017 um 15:24 hat Peter Lieven geschrieben: >> Am 10.07.2017 um 15:10 schrieb Kevin Wolf: >>> Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>>> --- >>>> qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 43 insertions(+), 1 deletion(-) >>> How does this make sense as a runtime option? What would happen if the >>> image contains one compression format and I specify another one on the >>> command line or in blockdev-add? >>> >>> Shouldn't it just be a create-time option and when you run qemu, it uses >>> whatever format that image has? >> I was asked to add it here. It is indeed only a create option and has >> no other effect. > But if it's only a create option, why is it added to blockdev-add? Is there already a separate schemata for the create options? Thanks, Peter
Am 13.07.2017 um 10:45 hat Peter Lieven geschrieben: > Am 10.07.2017 um 15:30 schrieb Kevin Wolf: > >Am 10.07.2017 um 15:24 hat Peter Lieven geschrieben: > >>Am 10.07.2017 um 15:10 schrieb Kevin Wolf: > >>>Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: > >>>>Signed-off-by: Peter Lieven <pl@kamp.de> > >>>>--- > >>>> qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- > >>>> 1 file changed, 43 insertions(+), 1 deletion(-) > >>>How does this make sense as a runtime option? What would happen if the > >>>image contains one compression format and I specify another one on the > >>>command line or in blockdev-add? > >>> > >>>Shouldn't it just be a create-time option and when you run qemu, it uses > >>>whatever format that image has? > >>I was asked to add it here. It is indeed only a create option and has > >>no other effect. > >But if it's only a create option, why is it added to blockdev-add? > > Is there already a separate schemata for the create options? If you mean a separate schema file, no. Putting it in the same file isn't a problem either, but hooking it up in structs used by blockdev-add (or other QMP commands) probably is. So if you want to follow Dan's suggestion, I think you just need to put the new type into the schema, without referencing it anywhere except in qcow2's .bdrv_create C implementation. Kevin
On Thu, Jul 13, 2017 at 10:52:41AM +0200, Kevin Wolf wrote: > Am 13.07.2017 um 10:45 hat Peter Lieven geschrieben: > > Am 10.07.2017 um 15:30 schrieb Kevin Wolf: > > >Am 10.07.2017 um 15:24 hat Peter Lieven geschrieben: > > >>Am 10.07.2017 um 15:10 schrieb Kevin Wolf: > > >>>Am 29.06.2017 um 12:57 hat Peter Lieven geschrieben: > > >>>>Signed-off-by: Peter Lieven <pl@kamp.de> > > >>>>--- > > >>>> qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- > > >>>> 1 file changed, 43 insertions(+), 1 deletion(-) > > >>>How does this make sense as a runtime option? What would happen if the > > >>>image contains one compression format and I specify another one on the > > >>>command line or in blockdev-add? > > >>> > > >>>Shouldn't it just be a create-time option and when you run qemu, it uses > > >>>whatever format that image has? > > >>I was asked to add it here. It is indeed only a create option and has > > >>no other effect. > > >But if it's only a create option, why is it added to blockdev-add? > > > > Is there already a separate schemata for the create options? > > If you mean a separate schema file, no. Putting it in the same file > isn't a problem either, but hooking it up in structs used by > blockdev-add (or other QMP commands) probably is. > > So if you want to follow Dan's suggestion, I think you just need to put > the new type into the schema, without referencing it anywhere except in > qcow2's .bdrv_create C implementation. Yep, that's exactly what I intended by my suggestion Regards, Daniel
diff --git a/qapi/block-core.json b/qapi/block-core.json index f85c223..1574ffb 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2282,6 +2282,43 @@ 'mode': 'Qcow2OverlapCheckMode' } } ## +# @Qcow2CompressFormat: +# @zlib: standard zlib deflate compression +# +# Since: 2.10 +## +{ 'enum': 'Qcow2CompressFormat', + 'data': [ 'zlib' ] } + +## +# @Qcow2CompressZLib: +# +# Since: 2.10 +## +{ 'struct': 'Qcow2CompressZLib', + 'data': { } } + +## +# @Qcow2Compress: +# +# Specifies the compression format and compression level that should +# be used for compressed Qcow2 clusters. +# +# @format: specifies the compression format to use. (defaults to zlib) +# +# @level: specifies the compression level. 0 = default compression, +# 1 = fastest compression, x = highest compresion (x may very between +# different compression formats) +# +# Since: 2.10 +## +{ 'union': 'Qcow2Compress', + 'base': { 'format': 'Qcow2CompressFormat', + 'level': 'uint8' }, + 'discriminator': 'format', + 'data': { 'zlib': 'Qcow2CompressZLib' } } + +## # @BlockdevOptionsQcow2: # # Driver specific block device options for qcow2. @@ -2316,6 +2353,10 @@ # caches. The interval is in seconds. The default value # is 0 and it disables this feature (since 2.5) # +# @compress: which format and compression level to use for +# compressed clusters. Defaults to zlib with default +# compression level (since 2.10) +# # Since: 2.9 ## { 'struct': 'BlockdevOptionsQcow2', @@ -2328,7 +2369,8 @@ '*cache-size': 'int', '*l2-cache-size': 'int', '*refcount-cache-size': 'int', - '*cache-clean-interval': 'int' } } + '*cache-clean-interval': 'int', + '*compress': 'Qcow2Compress' } } ##
Signed-off-by: Peter Lieven <pl@kamp.de> --- qapi/block-core.json | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-)