Discussion:
[Spice-devel] [PATCH] spice: Use new SpiceImageCompression definition
Frediano Ziglio
2018-11-26 15:30:36 UTC
Permalink
Definitions were updated by spice-server in patch de66161 included
in 0.12.6 released on 12th June 2015.

Signed-off-by: Frediano Ziglio <***@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
Christophe Fergeau
2018-11-27 12:35:02 UTC
Permalink
hey,
Post by Frediano Ziglio
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
Post by Frediano Ziglio
---
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
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Gerd Hoffmann
2018-11-28 05:48:50 UTC
Permalink
Post by Christophe Fergeau
hey,
Post by Frediano Ziglio
Definitions were updated by spice-server in patch de66161 included
in 0.12.6 released on 12th June 2015.
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
Marc-André Lureau
2018-11-28 08:19:51 UTC
Permalink
Hi
Post by Gerd Hoffmann
Post by Christophe Fergeau
hey,
Post by Frediano Ziglio
Definitions were updated by spice-server in patch de66161 included
in 0.12.6 released on 12th June 2015.
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"
Post by Gerd Hoffmann
thanks,
Gerd
_______________________________________________
Spice-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
Frediano Ziglio
2018-11-28 08:26:56 UTC
Permalink
Post by Marc-André Lureau
Hi
Post by Gerd Hoffmann
Post by Christophe Fergeau
hey,
Post by Frediano Ziglio
Definitions were updated by spice-server in patch de66161 included
in 0.12.6 released on 12th June 2015.
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?
Post by Marc-André Lureau
Post by Gerd Hoffmann
thanks,
Gerd
Frediano
Gerd Hoffmann
2018-11-28 09:33:47 UTC
Permalink
Post by Marc-André Lureau
Hi
Post by Gerd Hoffmann
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
n***@patchew.org
2018-11-28 06:46:09 UTC
Permalink
Hi,

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

Message-id: 20181126153036.22414-1-***@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-***@redhat.com

Loading...