diff mbox series

spice: Use new SpiceImageCompression definition

Message ID 20181126153036.22414-1-fziglio@redhat.com
State New
Headers show
Series spice: Use new SpiceImageCompression definition | expand

Commit Message

Frediano Ziglio Nov. 26, 2018, 3:30 p.m. UTC 
Definitions were updated by spice-server in patch de66161 included
in 0.12.6 released on 12th June 2015.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 ui/spice-core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Christophe Fergeau Nov. 27, 2018, 12:35 p.m. UTC | #1 
hey,

On Mon, Nov 26, 2018 at 03:30:36PM +0000, Frediano Ziglio wrote:
> Definitions were updated by spice-server in patch de66161 included
> in 0.12.6 released on 12th June 2015.

QEMU's configure only checks for spice-server 0.12.0:

$pkg_config --atleast-version=0.12.0 spice-server

  if $pkg_config --atleast-version=0.12.0 spice-server && \
     $pkg_config --atleast-version=0.12.3 spice-protocol && \
     compile_prog "$spice_cflags" "$spice_libs" ; then
    spice="yes"
    libs_softmmu="$libs_softmmu $spice_libs"
    QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
    spice_protocol_version=$($pkg_config --modversion spice-protocol)
    spice_server_version=$($pkg_config --modversion spice-server)
  else
    if test "$spice" = "yes" ; then
      feature_not_found "spice" \
          "Install spice-server(>=0.12.0) and spice-protocol(>=0.12.3) devel"
    fi
    spice="no"
  fi

I don't know how far back QEMU wants to support spice-server.
Apart from this, the patch looks good to me.

Christophe

> 
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  ui/spice-core.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index ebaae24643..2e6a255a35 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -331,12 +331,12 @@ static const char *stream_video_names[] = {
>                 stream_video_names, ARRAY_SIZE(stream_video_names))
>  
>  static const char *compression_names[] = {
> -    [ SPICE_IMAGE_COMPRESS_OFF ]      = "off",
> -    [ SPICE_IMAGE_COMPRESS_AUTO_GLZ ] = "auto_glz",
> -    [ SPICE_IMAGE_COMPRESS_AUTO_LZ ]  = "auto_lz",
> -    [ SPICE_IMAGE_COMPRESS_QUIC ]     = "quic",
> -    [ SPICE_IMAGE_COMPRESS_GLZ ]      = "glz",
> -    [ SPICE_IMAGE_COMPRESS_LZ ]       = "lz",
> +    [ SPICE_IMAGE_COMPRESSION_OFF ]      = "off",
> +    [ SPICE_IMAGE_COMPRESSION_AUTO_GLZ ] = "auto_glz",
> +    [ SPICE_IMAGE_COMPRESSION_AUTO_LZ ]  = "auto_lz",
> +    [ SPICE_IMAGE_COMPRESSION_QUIC ]     = "quic",
> +    [ SPICE_IMAGE_COMPRESSION_GLZ ]      = "glz",
> +    [ SPICE_IMAGE_COMPRESSION_LZ ]       = "lz",
>  };
>  #define parse_compression(_name)                                        \
>      parse_name(_name, "image compression",                              \
> @@ -643,7 +643,7 @@ void qemu_spice_init(void)
>          *x509_cert_file = NULL,
>          *x509_cacert_file = NULL;
>      int port, tls_port, addr_flags;
> -    spice_image_compression_t compression;
> +    SpiceImageCompression compression;
>      spice_wan_compression_t wan_compr;
>      bool seamless_migration;
>  
> @@ -754,7 +754,7 @@ void qemu_spice_init(void)
>  #endif
>      }
>  
> -    compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
> +    compression = SPICE_IMAGE_COMPRESSION_AUTO_GLZ;
>      str = qemu_opt_get(opts, "image-compression");
>      if (str) {
>          compression = parse_compression(str);
> -- 
> 2.17.2
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Gerd Hoffmann Nov. 28, 2018, 5:48 a.m. UTC | #2 
On Tue, Nov 27, 2018 at 01:35:02PM +0100, Christophe Fergeau wrote:
> hey,
> 
> On Mon, Nov 26, 2018 at 03:30:36PM +0000, Frediano Ziglio wrote:
> > Definitions were updated by spice-server in patch de66161 included
> > in 0.12.6 released on 12th June 2015.
> 
> QEMU's configure only checks for spice-server 0.12.0:

> I don't know how far back QEMU wants to support spice-server.
> Apart from this, the patch looks good to me.

0.12.6 is more than three years old, so this or something newer should
be available in most distros meanwhile.  Raising the bar to that version
looks ok to me (separate patch please, and please also drop #ifdefs we
don't need any more then).

thanks,
  Gerd
no-reply@patchew.org Nov. 28, 2018, 6:46 a.m. UTC | #3 
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20181126153036.22414-1-fziglio@redhat.com
Subject: [Qemu-devel] [PATCH] spice: Use new SpiceImageCompression definition
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
0db479d spice: Use new SpiceImageCompression definition

=== OUTPUT BEGIN ===
Checking PATCH 1/1: spice: Use new SpiceImageCompression definition...
ERROR: space prohibited after that open square bracket '['
#26: FILE: ui/spice-core.c:334:
+    [ SPICE_IMAGE_COMPRESSION_OFF ]      = "off",

ERROR: space prohibited before that close square bracket ']'
#26: FILE: ui/spice-core.c:334:
+    [ SPICE_IMAGE_COMPRESSION_OFF ]      = "off",

ERROR: space prohibited after that open square bracket '['
#27: FILE: ui/spice-core.c:335:
+    [ SPICE_IMAGE_COMPRESSION_AUTO_GLZ ] = "auto_glz",

ERROR: space prohibited before that close square bracket ']'
#27: FILE: ui/spice-core.c:335:
+    [ SPICE_IMAGE_COMPRESSION_AUTO_GLZ ] = "auto_glz",

ERROR: space prohibited after that open square bracket '['
#28: FILE: ui/spice-core.c:336:
+    [ SPICE_IMAGE_COMPRESSION_AUTO_LZ ]  = "auto_lz",

ERROR: space prohibited before that close square bracket ']'
#28: FILE: ui/spice-core.c:336:
+    [ SPICE_IMAGE_COMPRESSION_AUTO_LZ ]  = "auto_lz",

ERROR: space prohibited after that open square bracket '['
#29: FILE: ui/spice-core.c:337:
+    [ SPICE_IMAGE_COMPRESSION_QUIC ]     = "quic",

ERROR: space prohibited before that close square bracket ']'
#29: FILE: ui/spice-core.c:337:
+    [ SPICE_IMAGE_COMPRESSION_QUIC ]     = "quic",

ERROR: space prohibited after that open square bracket '['
#30: FILE: ui/spice-core.c:338:
+    [ SPICE_IMAGE_COMPRESSION_GLZ ]      = "glz",

ERROR: space prohibited before that close square bracket ']'
#30: FILE: ui/spice-core.c:338:
+    [ SPICE_IMAGE_COMPRESSION_GLZ ]      = "glz",

ERROR: space prohibited after that open square bracket '['
#31: FILE: ui/spice-core.c:339:
+    [ SPICE_IMAGE_COMPRESSION_LZ ]       = "lz",

ERROR: space prohibited before that close square bracket ']'
#31: FILE: ui/spice-core.c:339:
+    [ SPICE_IMAGE_COMPRESSION_LZ ]       = "lz",

total: 12 errors, 0 warnings, 34 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Marc-André Lureau Nov. 28, 2018, 8:19 a.m. UTC | #4 
Hi
On Wed, Nov 28, 2018 at 9:48 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Tue, Nov 27, 2018 at 01:35:02PM +0100, Christophe Fergeau wrote:
> > hey,
> >
> > On Mon, Nov 26, 2018 at 03:30:36PM +0000, Frediano Ziglio wrote:
> > > Definitions were updated by spice-server in patch de66161 included
> > > in 0.12.6 released on 12th June 2015.
> >
> > QEMU's configure only checks for spice-server 0.12.0:
>
> > I don't know how far back QEMU wants to support spice-server.
> > Apart from this, the patch looks good to me.
>
> 0.12.6 is more than three years old, so this or something newer should
> be available in most distros meanwhile.  Raising the bar to that version
> looks ok to me (separate patch please, and please also drop #ifdefs we
> don't need any more then).

See https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg00486.html
"bump spice-server required version to 0.12.6"

> thanks,
>   Gerd
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Frediano Ziglio Nov. 28, 2018, 8:26 a.m. UTC | #5 
> 
> Hi
> On Wed, Nov 28, 2018 at 9:48 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Tue, Nov 27, 2018 at 01:35:02PM +0100, Christophe Fergeau wrote:
> > > hey,
> > >
> > > On Mon, Nov 26, 2018 at 03:30:36PM +0000, Frediano Ziglio wrote:
> > > > Definitions were updated by spice-server in patch de66161 included
> > > > in 0.12.6 released on 12th June 2015.
> > >
> > > QEMU's configure only checks for spice-server 0.12.0:
> >
> > > I don't know how far back QEMU wants to support spice-server.
> > > Apart from this, the patch looks good to me.
> >
> > 0.12.6 is more than three years old, so this or something newer should
> > be available in most distros meanwhile.  Raising the bar to that version
> > looks ok to me (separate patch please, and please also drop #ifdefs we
> > don't need any more then).
> 
> See https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg00486.html
> "bump spice-server required version to 0.12.6"
> 

Opss..

https://lists.freedesktop.org/archives/spice-devel/2018-November/046279.html

slightly different, can you merge them?

> > thanks,
> >   Gerd
> >

Frediano
Gerd Hoffmann Nov. 28, 2018, 9:33 a.m. UTC | #6 
On Wed, Nov 28, 2018 at 12:19:51PM +0400, Marc-André Lureau wrote:
> Hi
> On Wed, Nov 28, 2018 at 9:48 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > 0.12.6 is more than three years old, so this or something newer should
> > be available in most distros meanwhile.  Raising the bar to that version
> > looks ok to me (separate patch please, and please also drop #ifdefs we
> > don't need any more then).
> 
> See https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg00486.html
> "bump spice-server required version to 0.12.6"

Whoops.  Looks like that one slipped through for some reason.

sorry,
  Gerd
diff mbox series

Patch

diff --git a/ui/spice-core.c b/ui/spice-core.c
index ebaae24643..2e6a255a35 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -331,12 +331,12 @@  static const char *stream_video_names[] = {
                stream_video_names, ARRAY_SIZE(stream_video_names))
 
 static const char *compression_names[] = {
-    [ SPICE_IMAGE_COMPRESS_OFF ]      = "off",
-    [ SPICE_IMAGE_COMPRESS_AUTO_GLZ ] = "auto_glz",
-    [ SPICE_IMAGE_COMPRESS_AUTO_LZ ]  = "auto_lz",
-    [ SPICE_IMAGE_COMPRESS_QUIC ]     = "quic",
-    [ SPICE_IMAGE_COMPRESS_GLZ ]      = "glz",
-    [ SPICE_IMAGE_COMPRESS_LZ ]       = "lz",
+    [ SPICE_IMAGE_COMPRESSION_OFF ]      = "off",
+    [ SPICE_IMAGE_COMPRESSION_AUTO_GLZ ] = "auto_glz",
+    [ SPICE_IMAGE_COMPRESSION_AUTO_LZ ]  = "auto_lz",
+    [ SPICE_IMAGE_COMPRESSION_QUIC ]     = "quic",
+    [ SPICE_IMAGE_COMPRESSION_GLZ ]      = "glz",
+    [ SPICE_IMAGE_COMPRESSION_LZ ]       = "lz",
 };
 #define parse_compression(_name)                                        \
     parse_name(_name, "image compression",                              \
@@ -643,7 +643,7 @@  void qemu_spice_init(void)
         *x509_cert_file = NULL,
         *x509_cacert_file = NULL;
     int port, tls_port, addr_flags;
-    spice_image_compression_t compression;
+    SpiceImageCompression compression;
     spice_wan_compression_t wan_compr;
     bool seamless_migration;
 
@@ -754,7 +754,7 @@  void qemu_spice_init(void)
 #endif
     }
 
-    compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
+    compression = SPICE_IMAGE_COMPRESSION_AUTO_GLZ;
     str = qemu_opt_get(opts, "image-compression");
     if (str) {
         compression = parse_compression(str);