| Message ID | 158031087.SthlJV7jIF@javinlg |
|---|---|
| State | New |
| Headers | show |
On 01/26/2015 01:48 AM, Javier Celaya wrote: > Sorry, I forgot to patch the command-line help. Hope it helps. > >>> >>> Recently, SPICE included the lz4 compression algorithm. This patch adds >>> a command line option to select it. >> >> How is libvirt going to introspect whether the command line supports >> this option? Is there some QMP command that lists the set of valid >> compression formats understood by a given qemu binary? No, patching the command line --help does NOT help libvirt. It needs to be discoverable via QMP to be introspectible, as scraping --help output is not machine-friendly. (That said, you DO want to expose it in --help output; I'm just complaining that --help output alone is not enough).
Eric Blake <eblake@redhat.com> writes: > On 01/26/2015 01:48 AM, Javier Celaya wrote: >> Sorry, I forgot to patch the command-line help. Hope it helps. >> > >>>> >>>> Recently, SPICE included the lz4 compression algorithm. This patch adds >>>> a command line option to select it. > >>> >>> How is libvirt going to introspect whether the command line supports >>> this option? Is there some QMP command that lists the set of valid >>> compression formats understood by a given qemu binary? > > No, patching the command line --help does NOT help libvirt. It needs to > be discoverable via QMP to be introspectible, as scraping --help output > is not machine-friendly. (That said, you DO want to expose it in --help > output; I'm just complaining that --help output alone is not enough). We should really, really, really provide access to (the relevant subset of) the QAPI schema over QMP! Until we get that, useful progress is delayed by problems like this one, and we keep growing special-purpose solutions to problems like this one.
From what I've seen in QEMU and libvirt's code, I would say that discovering whether the spice server supports LZ4 is not a matter of adding a new QMP command. That would not scale very well with new command line options. I would suggest something more general. For instance, having the command query- command-line-options return also the allowed values for string parameters. That would output "lz4" for parameter "image-compression" in option "spice", among the other compression methods. Another possibility would be a new QMP command that returns the capabilities of the spice server, as defined in the spice protocol. They include the SPICE_DISPLAY_CAP_LZ4_COMPRESSION capability, if the spice server supports LZ4. In any case, I think it is out of the scope of this patch. El Martes, 27 de enero de 2015 09:26:11 Markus Armbruster escribió: > Eric Blake <eblake@redhat.com> writes: > > On 01/26/2015 01:48 AM, Javier Celaya wrote: > >> Sorry, I forgot to patch the command-line help. Hope it helps. > >> > >>>> Recently, SPICE included the lz4 compression algorithm. This patch adds > >>>> a command line option to select it. > >>> > >>> How is libvirt going to introspect whether the command line supports > >>> this option? Is there some QMP command that lists the set of valid > >>> compression formats understood by a given qemu binary? > > > > No, patching the command line --help does NOT help libvirt. It needs to > > be discoverable via QMP to be introspectible, as scraping --help output > > is not machine-friendly. (That said, you DO want to expose it in --help > > output; I'm just complaining that --help output alone is not enough). > > We should really, really, really provide access to (the relevant subset > of) the QAPI schema over QMP! Until we get that, useful progress is > delayed by problems like this one, and we keep growing special-purpose > solutions to problems like this one.
Please don't top-quote. Javier Celaya <javier.celaya@flexvm.es> writes: > El Martes, 27 de enero de 2015 09:26:11 Markus Armbruster escribió: >> Eric Blake <eblake@redhat.com> writes: >> > On 01/26/2015 01:48 AM, Javier Celaya wrote: >> >> Sorry, I forgot to patch the command-line help. Hope it helps. >> >> >> >>>> Recently, SPICE included the lz4 compression algorithm. This patch adds >> >>>> a command line option to select it. >> >>> >> >>> How is libvirt going to introspect whether the command line supports >> >>> this option? Is there some QMP command that lists the set of valid >> >>> compression formats understood by a given qemu binary? >> > >> > No, patching the command line --help does NOT help libvirt. It needs to >> > be discoverable via QMP to be introspectible, as scraping --help output >> > is not machine-friendly. (That said, you DO want to expose it in --help >> > output; I'm just complaining that --help output alone is not enough). >> >> We should really, really, really provide access to (the relevant subset >> of) the QAPI schema over QMP! Until we get that, useful progress is >> delayed by problems like this one, and we keep growing special-purpose >> solutions to problems like this one. > >>From what I've seen in QEMU and libvirt's code, I would say that discovering > whether the spice server supports LZ4 is not a matter of adding a new QMP > command. That would not scale very well with new command line options. I would > suggest something more general. For instance, having the command query- > command-line-options return also the allowed values for string parameters. > That would output "lz4" for parameter "image-compression" in option "spice", > among the other compression methods. Another possibility would be a new QMP > command that returns the capabilities of the spice server, as defined in the > spice protocol. They include the SPICE_DISPLAY_CAP_LZ4_COMPRESSION capability, > if the spice server supports LZ4. In any case, I think it is out of the scope > of this patch. We already have a language to describe interfaces: QAPI schema. It already covers QMP. I propose to reuse it for QMP introspection instead of inventing yet another language. Unfortunately, it doesn't cover the command line. Recasting the command line in QAPI sounds sensible to me, but it's non-trivial, and so far nobody has volunteered to try. So we'll likely need a stop-gap, like extending query-command-line-options. I don't like growing more external interface languages, but I don't have better ideas.
Il 27/01/2015 09:26, Markus Armbruster ha scritto: > Eric Blake <eblake@redhat.com> writes: > >> On 01/26/2015 01:48 AM, Javier Celaya wrote: >>> Sorry, I forgot to patch the command-line help. Hope it helps. >>> >>>>> Recently, SPICE included the lz4 compression algorithm. This patch adds >>>>> a command line option to select it. >>>> How is libvirt going to introspect whether the command line supports >>>> this option? Is there some QMP command that lists the set of valid >>>> compression formats understood by a given qemu binary? >> No, patching the command line --help does NOT help libvirt. It needs to >> be discoverable via QMP to be introspectible, as scraping --help output >> is not machine-friendly. (That said, you DO want to expose it in --help >> output; I'm just complaining that --help output alone is not enough). > We should really, really, really provide access to (the relevant subset > of) the QAPI schema over QMP! Until we get that, useful progress is > delayed by problems like this one, and we keep growing special-purpose > solutions to problems like this one. > _______________________________________________ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > > > Hi, any news about spice lz4 support still missed in qemu upstream? Thanks for any reply.
Fabio Fantoni <fantonifabio@tiscali.it> writes: > Il 27/01/2015 09:26, Markus Armbruster ha scritto: >> Eric Blake <eblake@redhat.com> writes: >> >>> On 01/26/2015 01:48 AM, Javier Celaya wrote: >>>> Sorry, I forgot to patch the command-line help. Hope it helps. >>>> >>>>>> Recently, SPICE included the lz4 compression algorithm. This patch adds >>>>>> a command line option to select it. >>>>> How is libvirt going to introspect whether the command line supports >>>>> this option? Is there some QMP command that lists the set of valid >>>>> compression formats understood by a given qemu binary? >>> No, patching the command line --help does NOT help libvirt. It needs to >>> be discoverable via QMP to be introspectible, as scraping --help output >>> is not machine-friendly. (That said, you DO want to expose it in --help >>> output; I'm just complaining that --help output alone is not enough). >> We should really, really, really provide access to (the relevant subset >> of) the QAPI schema over QMP! Until we get that, useful progress is >> delayed by problems like this one, and we keep growing special-purpose >> solutions to problems like this one. >> _______________________________________________ >> Spice-devel mailing list >> Spice-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/spice-devel >> >> >> > > Hi, any news about spice lz4 support still missed in qemu upstream? > > Thanks for any reply. I'm committed to delivering QMP introspection in the next development cycle. That'll answer "does this version support X in QMP" questions, which may in turn be a usable indicator for "does it support X' on the command line", at least for some X. If the SPICE LZ4 feature is such an X, the patch can then go in without further work.
Il 27/01/2015 09:26, Markus Armbruster ha scritto: > Eric Blake <eblake@redhat.com> writes: > >> On 01/26/2015 01:48 AM, Javier Celaya wrote: >>> Sorry, I forgot to patch the command-line help. Hope it helps. >>> >>>>> Recently, SPICE included the lz4 compression algorithm. This patch adds >>>>> a command line option to select it. >>>> How is libvirt going to introspect whether the command line supports >>>> this option? Is there some QMP command that lists the set of valid >>>> compression formats understood by a given qemu binary? >> No, patching the command line --help does NOT help libvirt. It needs to >> be discoverable via QMP to be introspectible, as scraping --help output >> is not machine-friendly. (That said, you DO want to expose it in --help >> output; I'm just complaining that --help output alone is not enough). > We should really, really, really provide access to (the relevant subset > of) the QAPI schema over QMP! Until we get that, useful progress is > delayed by problems like this one, and we keep growing special-purpose > solutions to problems like this one. > _______________________________________________ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > Hi, spice 0.12.6 with lz4 support has been released, is there any possibility to add it also in qemu upstream to make possible selecting it "Out of the box" please?
From 26864da931603791c70c570b9b46f21657ebcaec Mon Sep 17 00:00:00 2001 From: Javier Celaya <javier.celaya@flexvm.es> Date: Wed, 7 Jan 2015 11:53:24 +0100 Subject: [PATCH] Add lz4 compression option for SPICE. Recently, SPICE included the lz4 compression algorithm. This patch adds a command line option to select it. --- qemu-options.hx | 4 ++-- ui/spice-core.c | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 22cf3b9..f5c54ba 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -953,7 +953,7 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice, " [,tls-channel=[main|display|cursor|inputs|record|playback]]\n" " [,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n" " [,sasl][,password=<secret>][,disable-ticketing]\n" - " [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n" + " [,image-compression=[auto_glz|auto_lz|quic|glz|lz|lz4|off]]\n" " [,jpeg-wan-compression=[auto|never|always]]\n" " [,zlib-glz-wan-compression=[auto|never|always]]\n" " [,streaming-video=[off|all|filter]][,disable-copy-paste]\n" @@ -1028,7 +1028,7 @@ channels. The special name "default" can be used to set the default mode. For channels which are not explicitly forced into one mode the spice client is allowed to pick tls/plaintext as he pleases. -@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off] +@item image-compression=[auto_glz|auto_lz|quic|glz|lz|lz4|off] Configure image compression (lossless). Default is auto_glz. diff --git a/ui/spice-core.c b/ui/spice-core.c index 6467fa4..dadcae9 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -359,6 +359,9 @@ static const char *compression_names[] = { [ SPICE_IMAGE_COMPRESS_QUIC ] = "quic", [ SPICE_IMAGE_COMPRESS_GLZ ] = "glz", [ SPICE_IMAGE_COMPRESS_LZ ] = "lz", +#if SPICE_SERVER_VERSION >= 0x000c07 + [ SPICE_IMAGE_COMPRESS_LZ4 ] = "lz4", +#endif }; #define parse_compression(_name) \ parse_name(_name, "image compression", \ -- 1.9.3