All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] [RFC] LZ4 compression option for SPICE
@ 2015-01-08 10:50 Javier Celaya
  2015-01-08 11:28 ` Gerd Hoffmann
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Javier Celaya @ 2015-01-08 10:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: spice-devel, flex.dev, Gerd Hoffmann, Anthony Liguori

Hello

Recently, SPICE included the lz4 compression algorithm. This patch adds
a command line option to select it. However, SPICE_IMAGE_COMPRESS_LZ4 did not 
exist before the commit that added this compression algorithm, so it should be 
guarded with conditional compilation. How do you think this should be done? 
Wait for the next stable version of spice-server and check for 
SPICE_SERVER_VERSION? Or add a specific flag?

Thank you
---
 ui/spice-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6467fa4..fb6534e 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -359,6 +359,7 @@ static const char *compression_names[] = {
     [ SPICE_IMAGE_COMPRESS_QUIC ]     = "quic",
     [ SPICE_IMAGE_COMPRESS_GLZ ]      = "glz",
     [ SPICE_IMAGE_COMPRESS_LZ ]       = "lz",
+    [ SPICE_IMAGE_COMPRESS_LZ4 ]      = "lz4",
 };
 #define parse_compression(_name)                                        \
     parse_name(_name, "image compression",                              \
--
1.9.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-08 10:50 [Qemu-devel] [PATCH] [RFC] LZ4 compression option for SPICE Javier Celaya
@ 2015-01-08 11:28 ` Gerd Hoffmann
  2015-01-08 11:48 ` [Qemu-devel] [Spice-devel] " Fabio Fantoni
  2015-01-20 16:08 ` Christophe Fergeau
  2 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2015-01-08 11:28 UTC (permalink / raw)
  To: Javier Celaya; +Cc: spice-devel, flex.dev, qemu-devel, Anthony Liguori

On Do, 2015-01-08 at 11:50 +0100, Javier Celaya wrote:
> Hello
> 
> Recently, SPICE included the lz4 compression algorithm. This patch adds
> a command line option to select it. However, SPICE_IMAGE_COMPRESS_LZ4 did not 
> exist before the commit that added this compression algorithm, so it should be 
> guarded with conditional compilation. How do you think this should be done? 
> Wait for the next stable version of spice-server and check for 
> SPICE_SERVER_VERSION? Or add a specific flag?

Given that SPICE_IMAGE_COMPRESS_LZ4 is a enum not define (so you can't
just #ifdef SPICE_IMAGE_COMPRESS_LZ4) using SPICE_SERVER_VERSION looks
best to me.

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-08 10:50 [Qemu-devel] [PATCH] [RFC] LZ4 compression option for SPICE Javier Celaya
  2015-01-08 11:28 ` Gerd Hoffmann
@ 2015-01-08 11:48 ` Fabio Fantoni
  2015-01-08 12:00   ` Javier Celaya
  2015-01-20 16:08 ` Christophe Fergeau
  2 siblings, 1 reply; 15+ messages in thread
From: Fabio Fantoni @ 2015-01-08 11:48 UTC (permalink / raw)
  To: Javier Celaya, qemu-devel; +Cc: spice-devel, flex.dev, Anthony Liguori

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

Il 08/01/2015 11:50, Javier Celaya ha scritto:
> Hello
>
> Recently, SPICE included the lz4 compression algorithm. This patch adds
> a command line option to select it. However, SPICE_IMAGE_COMPRESS_LZ4 did not 
> exist before the commit that added this compression algorithm, so it should be 
> guarded with conditional compilation. How do you think this should be done? 
> Wait for the next stable version of spice-server and check for 
> SPICE_SERVER_VERSION? Or add a specific flag?
>
> Thank you
> ---
>  ui/spice-core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 6467fa4..fb6534e 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -359,6 +359,7 @@ static const char *compression_names[] = {
>      [ SPICE_IMAGE_COMPRESS_QUIC ]     = "quic",
>      [ SPICE_IMAGE_COMPRESS_GLZ ]      = "glz",
>      [ SPICE_IMAGE_COMPRESS_LZ ]       = "lz",
> +    [ SPICE_IMAGE_COMPRESS_LZ4 ]      = "lz4",
>  };
>  #define parse_compression(_name)                                        \
>      parse_name(_name, "image compression",                              \
> --
> 1.9.3

I did a small search now and seems that also other small change to
qemu-options.hx file should be done, search this line:
@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]

>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>



[-- Attachment #2: Firma crittografica S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4297 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-08 11:48 ` [Qemu-devel] [Spice-devel] " Fabio Fantoni
@ 2015-01-08 12:00   ` Javier Celaya
  0 siblings, 0 replies; 15+ messages in thread
From: Javier Celaya @ 2015-01-08 12:00 UTC (permalink / raw)
  To: fantonifabio; +Cc: spice-devel, flex.dev, qemu-devel, Anthony Liguori

Sure, for the help text of the image-compression option, thanks.

El Jueves, 8 de enero de 2015 12:48:57 Fabio Fantoni escribió:
> Il 08/01/2015 11:50, Javier Celaya ha scritto:
> > Hello
> > 
> > Recently, SPICE included the lz4 compression algorithm. This patch adds
> > a command line option to select it. However, SPICE_IMAGE_COMPRESS_LZ4 did
> > not exist before the commit that added this compression algorithm, so it
> > should be guarded with conditional compilation. How do you think this
> > should be done? Wait for the next stable version of spice-server and
> > check for
> > SPICE_SERVER_VERSION? Or add a specific flag?
> > 
> > Thank you
> > ---
> > 
> >  ui/spice-core.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/ui/spice-core.c b/ui/spice-core.c
> > index 6467fa4..fb6534e 100644
> > --- a/ui/spice-core.c
> > +++ b/ui/spice-core.c
> > @@ -359,6 +359,7 @@ static const char *compression_names[] = {
> > 
> >      [ SPICE_IMAGE_COMPRESS_QUIC ]     = "quic",
> >      [ SPICE_IMAGE_COMPRESS_GLZ ]      = "glz",
> >      [ SPICE_IMAGE_COMPRESS_LZ ]       = "lz",
> > 
> > +    [ SPICE_IMAGE_COMPRESS_LZ4 ]      = "lz4",
> > 
> >  };
> >  #define parse_compression(_name)                                        \
> >  
> >      parse_name(_name, "image compression",                              \
> > 
> > --
> > 1.9.3
> 
> I did a small search now and seems that also other small change to
> qemu-options.hx file should be done, search this line:
> @item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]
> 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-08 10:50 [Qemu-devel] [PATCH] [RFC] LZ4 compression option for SPICE Javier Celaya
  2015-01-08 11:28 ` Gerd Hoffmann
  2015-01-08 11:48 ` [Qemu-devel] [Spice-devel] " Fabio Fantoni
@ 2015-01-20 16:08 ` Christophe Fergeau
  2015-01-23 14:06   ` Javier Celaya
  2 siblings, 1 reply; 15+ messages in thread
From: Christophe Fergeau @ 2015-01-20 16:08 UTC (permalink / raw)
  To: Javier Celaya; +Cc: spice-devel, flex.dev, qemu-devel, Anthony Liguori

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

Hey,

On Thu, Jan 08, 2015 at 11:50:13AM +0100, Javier Celaya wrote:
> Hello
> 
> Recently, SPICE included the lz4 compression algorithm. This patch adds
> a command line option to select it. However, SPICE_IMAGE_COMPRESS_LZ4 did not 
> exist before the commit that added this compression algorithm, so it should be 
> guarded with conditional compilation. How do you think this should be done? 
> Wait for the next stable version of spice-server and check for 
> SPICE_SERVER_VERSION? Or add a specific flag?

Version check seems good, we probably can raise spice-server version in
git preemptively so that you can have a working version check now.

Given that lz4 support is optional, spice_server_set_image_compression
should probably error out if we try to set lz4 but it's not supported.

Christophe

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-20 16:08 ` Christophe Fergeau
@ 2015-01-23 14:06   ` Javier Celaya
  2015-01-23 16:25     ` Eric Blake
  0 siblings, 1 reply; 15+ messages in thread
From: Javier Celaya @ 2015-01-23 14:06 UTC (permalink / raw)
  To: Christophe Fergeau
  Cc: spice-devel, Gerd Hoffmann, qemu-devel, Anthony Liguori, devel

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

Hello, this is the patch with the version check. Christophe, can you check 
that the version is the correct one?

El Martes, 20 de enero de 2015 17:08:07 Christophe Fergeau escribió:
> Hey,
> 
> Version check seems good, we probably can raise spice-server version in
> git preemptively so that you can have a working version check now.
> 
> Given that lz4 support is optional, spice_server_set_image_compression
> should probably error out if we try to set lz4 but it's not supported.

Ack

> 
> Christophe

[-- Attachment #2: 0001-Add-lz4-compression-option-for-SPICE.patch --]
[-- Type: text/x-patch, Size: 969 bytes --]

>From 896a3ea776ee95653a7cb8a0c90111a89b57c73f 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.
---
 ui/spice-core.c | 3 +++
 1 file changed, 3 insertions(+)

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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-23 14:06   ` Javier Celaya
@ 2015-01-23 16:25     ` Eric Blake
  2015-01-26  8:48       ` Javier Celaya
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2015-01-23 16:25 UTC (permalink / raw)
  To: Javier Celaya, Christophe Fergeau
  Cc: devel, spice-devel, Gerd Hoffmann, Anthony Liguori, qemu-devel

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

On 01/23/2015 07:06 AM, Javier Celaya wrote:
> Hello, this is the patch with the version check. Christophe, can you check 
> that the version is the correct one?
> 

> 
> Recently, SPICE included the lz4 compression algorithm. This patch adds
> a command line option to select it.
> ---
>  ui/spice-core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 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",

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?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-23 16:25     ` Eric Blake
@ 2015-01-26  8:48       ` Javier Celaya
  2015-01-26 16:25         ` Eric Blake
  0 siblings, 1 reply; 15+ messages in thread
From: Javier Celaya @ 2015-01-26  8:48 UTC (permalink / raw)
  To: Eric Blake, spice-devel; +Cc: devel, Gerd Hoffmann, Anthony Liguori, qemu-devel

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

Sorry, I forgot to patch the command-line help. Hope it helps.

El Viernes, 23 de enero de 2015 09:25:38 Eric Blake escribió:
> On 01/23/2015 07:06 AM, Javier Celaya wrote:
> > Hello, this is the patch with the version check. Christophe, can you check
> > that the version is the correct one?
> > 
> > 
> > 
> > Recently, SPICE included the lz4 compression algorithm. This patch adds
> > a command line option to select it.
> > ---
> > 
> >  ui/spice-core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > 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",
> 
> 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?

[-- Attachment #2: 0001-Add-lz4-compression-option-for-SPICE.patch --]
[-- Type: text/x-patch, Size: 2176 bytes --]

>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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-26  8:48       ` Javier Celaya
@ 2015-01-26 16:25         ` Eric Blake
  2015-01-27  8:26           ` Markus Armbruster
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2015-01-26 16:25 UTC (permalink / raw)
  To: Javier Celaya, spice-devel
  Cc: devel, Gerd Hoffmann, Anthony Liguori, qemu-devel

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

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    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-26 16:25         ` Eric Blake
@ 2015-01-27  8:26           ` Markus Armbruster
  2015-01-28 16:54             ` Javier Celaya
                               ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-01-27  8:26 UTC (permalink / raw)
  To: Eric Blake
  Cc: qemu-devel, Javier Celaya, devel, Gerd Hoffmann, Anthony Liguori,
	spice-devel

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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-27  8:26           ` Markus Armbruster
@ 2015-01-28 16:54             ` Javier Celaya
  2015-01-28 19:08               ` Markus Armbruster
  2015-07-24 14:12             ` Fabio Fantoni
  2015-10-09  6:45             ` Fabio Fantoni
  2 siblings, 1 reply; 15+ messages in thread
From: Javier Celaya @ 2015-01-28 16:54 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, devel, Gerd Hoffmann, Anthony Liguori, spice-devel

>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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-28 16:54             ` Javier Celaya
@ 2015-01-28 19:08               ` Markus Armbruster
  0 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-01-28 19:08 UTC (permalink / raw)
  To: Javier Celaya
  Cc: spice-devel, Gerd Hoffmann, qemu-devel, Anthony Liguori, devel

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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-27  8:26           ` Markus Armbruster
  2015-01-28 16:54             ` Javier Celaya
@ 2015-07-24 14:12             ` Fabio Fantoni
  2015-07-27  6:12               ` Markus Armbruster
  2015-10-09  6:45             ` Fabio Fantoni
  2 siblings, 1 reply; 15+ messages in thread
From: Fabio Fantoni @ 2015-07-24 14:12 UTC (permalink / raw)
  To: Markus Armbruster, Eric Blake
  Cc: spice-devel, qemu-devel, Anthony Liguori, devel

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

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.


[-- Attachment #2: Firma crittografica S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4317 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-07-24 14:12             ` Fabio Fantoni
@ 2015-07-27  6:12               ` Markus Armbruster
  0 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-07-27  6:12 UTC (permalink / raw)
  To: Fabio Fantoni; +Cc: spice-devel, qemu-devel, Anthony Liguori, devel

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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [Spice-devel] [PATCH] [RFC] LZ4 compression option for SPICE
  2015-01-27  8:26           ` Markus Armbruster
  2015-01-28 16:54             ` Javier Celaya
  2015-07-24 14:12             ` Fabio Fantoni
@ 2015-10-09  6:45             ` Fabio Fantoni
  2 siblings, 0 replies; 15+ messages in thread
From: Fabio Fantoni @ 2015-10-09  6:45 UTC (permalink / raw)
  To: Markus Armbruster, Eric Blake
  Cc: spice-devel, qemu-devel, Anthony Liguori, devel

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

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?


[-- Attachment #2: Firma crittografica S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4317 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-10-09  6:45 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-08 10:50 [Qemu-devel] [PATCH] [RFC] LZ4 compression option for SPICE Javier Celaya
2015-01-08 11:28 ` Gerd Hoffmann
2015-01-08 11:48 ` [Qemu-devel] [Spice-devel] " Fabio Fantoni
2015-01-08 12:00   ` Javier Celaya
2015-01-20 16:08 ` Christophe Fergeau
2015-01-23 14:06   ` Javier Celaya
2015-01-23 16:25     ` Eric Blake
2015-01-26  8:48       ` Javier Celaya
2015-01-26 16:25         ` Eric Blake
2015-01-27  8:26           ` Markus Armbruster
2015-01-28 16:54             ` Javier Celaya
2015-01-28 19:08               ` Markus Armbruster
2015-07-24 14:12             ` Fabio Fantoni
2015-07-27  6:12               ` Markus Armbruster
2015-10-09  6:45             ` Fabio Fantoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.