[Qemu-devel] [PATCHv2 1/2] spice: use error_report to report errors

[Qemu-devel] [PATCHv2 1/2] spice: use error_report to report errors

[Christophe Fergeau]

Error message reporting during spice startup wasn't consistent, it was done
with fprintf(stderr, "") but sometimes the message didn't have a trailing
\n. Using error_report make the intent of the message clearer and deal
with the final \n for us.
---
 ui/spice-core.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 1308a3d..a374999 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -229,8 +229,8 @@ static void channel_event(int event, SpiceChannelEventInfo *info)
         add_addr_info(server, (struct sockaddr *)&info->laddr_ext,
                       info->llen_ext);
     } else {
-        fprintf(stderr, "spice: %s, extended address is expected\n",
-                        __func__);
+        error_report("spice: %s, extended address is expected",
+                     __func__);
 #endif
         add_addr_info(client, &info->paddr, info->plen);
         add_addr_info(server, &info->laddr, info->llen);
@@ -346,7 +346,7 @@ static int parse_name(const char *string, const char *optname,
     if (value != -1) {
         return value;
     }
-    fprintf(stderr, "spice: invalid %s: %s\n", optname, string);
+    error_report("spice: invalid %s: %s", optname, string);
     exit(1);
 }
 
@@ -540,7 +540,7 @@ static int add_channel(const char *name, const char *value, void *opaque)
         rc = spice_server_set_channel_security(spice_server, value, security);
     }
     if (rc != 0) {
-        fprintf(stderr, "spice: failed to set channel security for %s\n", value);
+        error_report("spice: failed to set channel security for %s", value);
         exit(1);
     }
     return 0;
@@ -568,15 +568,15 @@ void qemu_spice_init(void)
     port = qemu_opt_get_number(opts, "port", 0);
     tls_port = qemu_opt_get_number(opts, "tls-port", 0);
     if (!port && !tls_port) {
-        fprintf(stderr, "neither port nor tls-port specified for spice.");
+        error_report("neither port nor tls-port specified for spice");
         exit(1);
     }
     if (port < 0 || port > 65535) {
-        fprintf(stderr, "spice port is out of range");
+        error_report("spice port is out of range");
         exit(1);
     }
     if (tls_port < 0 || tls_port > 65535) {
-        fprintf(stderr, "spice tls-port is out of range");
+        error_report("spice tls-port is out of range");
         exit(1);
     }
     password = qemu_opt_get(opts, "password");
@@ -646,11 +646,11 @@ void qemu_spice_init(void)
 #if SPICE_SERVER_VERSION >= 0x000900 /* 0.9.0 */
         if (spice_server_set_sasl_appname(spice_server, "qemu") == -1 ||
             spice_server_set_sasl(spice_server, 1) == -1) {
-            fprintf(stderr, "spice: failed to enable sasl\n");
+            error_report("spice: failed to enable sasl");
             exit(1);
         }
 #else
-        fprintf(stderr, "spice: sasl is not available (spice >= 0.9 required)\n");
+        error_report("spice: sasl is not available (spice >= 0.9 required)");
         exit(1);
 #endif
     }
@@ -700,7 +700,7 @@ void qemu_spice_init(void)
     qemu_opt_foreach(opts, add_channel, NULL, 0);
 
     if (0 != spice_server_init(spice_server, &core_interface)) {
-        fprintf(stderr, "failed to initialize spice server");
+        error_report("failed to initialize spice server");
         exit(1);
     };
     using_spice = 1;
@@ -725,7 +725,7 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
 {
     if (!spice_server) {
         if (QTAILQ_FIRST(&qemu_spice_opts.head) != NULL) {
-            fprintf(stderr, "Oops: spice configured but not active\n");
+            error_report("Oops: spice configured but not active");
             exit(1);
         }
         /*
-- 
1.7.7.6

[Qemu-devel] [PATCHv2 2/2] Error out when tls-channel option is used without TLS

[Christophe Fergeau]

It's currently possible to setup spice channels using TLS when
no TLS port has been specified (ie TLS is disabled). This cannot
work, so better to error out in such a situation.
---
 ui/spice-core.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index a374999..083af4f 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -526,6 +526,11 @@ static int add_channel(const char *name, const char *value, void *opaque)
     int rc;
 
     if (strcmp(name, "tls-channel") == 0) {
+        int *tls_port = opaque;
+        if (!*tls_port) {
+            error_report("spice: tried to setup tls-channel without specifying a TLS port\n");
+            exit(1);
+        }
         security = SPICE_CHANNEL_SECURITY_SSL;
     }
     if (strcmp(name, "plaintext-channel") == 0) {
@@ -697,7 +702,7 @@ void qemu_spice_init(void)
     spice_server_set_playback_compression
         (spice_server, qemu_opt_get_bool(opts, "playback-compression", 1));
 
-    qemu_opt_foreach(opts, add_channel, NULL, 0);
+    qemu_opt_foreach(opts, add_channel, &tls_port, 0);
 
     if (0 != spice_server_init(spice_server, &core_interface)) {
         error_report("failed to initialize spice server");
-- 
1.7.7.6

Re: [Qemu-devel] [PATCHv2 2/2] Error out when tls-channel option is used without TLS

[Gerd Hoffmann]

On 02/24/12 18:13, Christophe Fergeau wrote:
> It's currently possible to setup spice channels using TLS when
> no TLS port has been specified (ie TLS is disabled). This cannot
> work, so better to error out in such a situation.

Applying: Error out when tls-channel option is used without TLS
=== checkpatch complains ===
WARNING: line over 80 characters
#12: FILE: ui/spice-core.c:531:
+            error_report("spice: tried to setup tls-channel without
specifying a TLS port\n");

total: 0 errors, 1 warnings, 19 lines checked

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

[Qemu-devel] [PATCHv3] Error out when tls-channel option is used without TLS

[Christophe Fergeau]

It's currently possible to setup spice channels using TLS when
no TLS port has been specified (ie TLS is disabled). This cannot
work, so better to error out in such a situation.
---
 ui/spice-core.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index a374999..9a7912a 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -526,6 +526,12 @@ static int add_channel(const char *name, const char *value, void *opaque)
     int rc;
 
     if (strcmp(name, "tls-channel") == 0) {
+        int *tls_port = opaque;
+        if (!*tls_port) {
+            error_report("spice: tried to setup tls-channel"
+                         " without specifying a TLS port");
+            exit(1);
+        }
         security = SPICE_CHANNEL_SECURITY_SSL;
     }
     if (strcmp(name, "plaintext-channel") == 0) {
@@ -697,7 +703,7 @@ void qemu_spice_init(void)
     spice_server_set_playback_compression
         (spice_server, qemu_opt_get_bool(opts, "playback-compression", 1));
 
-    qemu_opt_foreach(opts, add_channel, NULL, 0);
+    qemu_opt_foreach(opts, add_channel, &tls_port, 0);
 
     if (0 != spice_server_init(spice_server, &core_interface)) {
         error_report("failed to initialize spice server");
-- 
1.7.7.6

Re: [Qemu-devel] [PATCHv3] Error out when tls-channel option is used without TLS

[Gerd Hoffmann]

On 02/24/12 18:28, Christophe Fergeau wrote:
> It's currently possible to setup spice channels using TLS when
> no TLS port has been specified (ie TLS is disabled). This cannot
> work, so better to error out in such a situation.

Patch added to spice patch queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCHv2 1/2] spice: use error_report to report errors

[Gerd Hoffmann]

On 02/24/12 18:13, Christophe Fergeau wrote:
> Error message reporting during spice startup wasn't consistent, it was done
> with fprintf(stderr, "") but sometimes the message didn't have a trailing
> \n. Using error_report make the intent of the message clearer and deal
> with the final \n for us.

Patch added to spice patch queue.

thanks,
  Gerd