[Qemu-devel] [PATCH 0/4] Trivial patches about qemu-char

[Qemu-devel] [PATCH 0/4] Trivial patches about qemu-char

[zhanghailiang]

Patch 1 and 2 fix wrong check about in-parameter.
The last two patches convert some open functions to use Error API.

zhanghailiang (4):
  qemu-char: fix parameter check for qemu_chr_open_pipe
  spice-qemu-char: fix check for in-parameter
  qemu-char: convert some open functions to use Error API
  spice-qemu-char: convert open functions to use Error API

 include/ui/qemu-spice.h     |  9 +++----
 qemu-char.c                 | 58 +++++++++++++++++++++++----------------------
 spice-qemu-char.c           | 14 +++++------
 stubs/qemu-chr-open-spice.c |  4 ++--
 4 files changed, 44 insertions(+), 41 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/4] qemu-char: fix parameter check for qemu_chr_open_pipe

[zhanghailiang]

The filename parameter never to be NULL, because in qemu_chr_parse_pipe
it is return value of g_strdup(device), where device will not be
NULL.

We should check its length.
After this patch, when run command:
 qemu-system-x86_64 -chardev pipe,id=pipe1,path=
It will report error:
 chardev: pipe: no filename given

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 qemu-char.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-char.c b/qemu-char.c
index bd0709b..42b1d8f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1084,7 +1084,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
     char filename_out[CHR_MAX_FILENAME_SIZE];
     const char *filename = opts->device;
 
-    if (filename == NULL) {
+    if (filename == NULL || strlen(filename) == 0) {
         fprintf(stderr, "chardev: pipe: no filename given\n");
         return NULL;
     }
-- 
1.7.12.4

Re: [Qemu-devel] [Qemu-trivial] [PATCH 1/4] qemu-char: fix parameter check for qemu_chr_open_pipe

[Michael Tokarev]

01.11.2014 04:50, zhanghailiang wrote:
> The filename parameter never to be NULL, because in qemu_chr_parse_pipe
> it is return value of g_strdup(device), where device will not be
> NULL.
> 
> We should check its length.
> After this patch, when run command:
>  qemu-system-x86_64 -chardev pipe,id=pipe1,path=
> It will report error:
>  chardev: pipe: no filename given
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  qemu-char.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index bd0709b..42b1d8f 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1084,7 +1084,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
>      char filename_out[CHR_MAX_FILENAME_SIZE];
>      const char *filename = opts->device;
>  
> -    if (filename == NULL) {
> +    if (filename == NULL || strlen(filename) == 0) {
>          fprintf(stderr, "chardev: pipe: no filename given\n");
>          return NULL;
>      }

and chr_parse_pipe() looks like:


static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
                                Error **errp)
{
    const char *device = qemu_opt_get(opts, "path");

    if (device == NULL) {
        error_setg(errp, "chardev: pipe: no device path given");
        return;
    }

Maybe we should combine the two checks into one in chr_parse_pipe,
and remove the check in chr_open_pipe entirely?

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH 1/4] qemu-char: fix parameter check for qemu_chr_open_pipe

[zhanghailiang]

On 2014/11/2 13:18, Michael Tokarev wrote:
> 01.11.2014 04:50, zhanghailiang wrote:
>> The filename parameter never to be NULL, because in qemu_chr_parse_pipe
>> it is return value of g_strdup(device), where device will not be
>> NULL.
>>
>> We should check its length.
>> After this patch, when run command:
>>   qemu-system-x86_64 -chardev pipe,id=pipe1,path=
>> It will report error:
>>   chardev: pipe: no filename given
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   qemu-char.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index bd0709b..42b1d8f 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -1084,7 +1084,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
>>       char filename_out[CHR_MAX_FILENAME_SIZE];
>>       const char *filename = opts->device;
>>
>> -    if (filename == NULL) {
>> +    if (filename == NULL || strlen(filename) == 0) {
>>           fprintf(stderr, "chardev: pipe: no filename given\n");
>>           return NULL;
>>       }
>
> and chr_parse_pipe() looks like:
>
>
> static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
>                                  Error **errp)
> {
>      const char *device = qemu_opt_get(opts, "path");
>
>      if (device == NULL) {
>          error_setg(errp, "chardev: pipe: no device path given");
>          return;
>      }
>
> Maybe we should combine the two checks into one in chr_parse_pipe,
> and remove the check in chr_open_pipe entirely?

Good idea, This is just like what qemu_chr_parse_udp does ;) Thanks.

> .
>

[Qemu-devel] [PATCH 2/4] spice-qemu-char: fix check for in-parameter

[zhanghailiang]

For qemu_chr_open_spice_vmc and qemu_chr_open_spice_port, the in-parameter
never to be NULL, because the checks in qemu_chr_parse_spice_vmc
and qemu_chr_parse_spice_port have ensured this.

So we should check the length of the in-parameter.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 spice-qemu-char.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 8106e06..45e7d69 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -290,7 +290,7 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
 {
     const char **psubtype = spice_server_char_device_recognized_subtypes();
 
-    if (type == NULL) {
+    if (type == NULL || strlen(type) == 0) {
         fprintf(stderr, "spice-qemu-char: missing name parameter\n");
         print_allowed_subtypes();
         return NULL;
@@ -315,7 +315,7 @@ CharDriverState *qemu_chr_open_spice_port(const char *name)
     CharDriverState *chr;
     SpiceCharDriver *s;
 
-    if (name == NULL) {
+    if (name == NULL || strlen(name) == 0) {
         fprintf(stderr, "spice-qemu-char: missing name parameter\n");
         return NULL;
     }
-- 
1.7.12.4

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/4] spice-qemu-char: fix check for in-parameter

[Michael Tokarev]

01.11.2014 04:50, zhanghailiang wrote:
> For qemu_chr_open_spice_vmc and qemu_chr_open_spice_port, the in-parameter
> never to be NULL, because the checks in qemu_chr_parse_spice_vmc
> and qemu_chr_parse_spice_port have ensured this.
> 
> So we should check the length of the in-parameter.

The same applies here as to qemu_chr_open in patch 1/4.
But here we've one more thing:

> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  spice-qemu-char.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 8106e06..45e7d69 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -290,7 +290,7 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
>  {
>      const char **psubtype = spice_server_char_device_recognized_subtypes();
>  
> -    if (type == NULL) {
> +    if (type == NULL || strlen(type) == 0) {
>          fprintf(stderr, "spice-qemu-char: missing name parameter\n");

This is 'missing TYPE parameter' not name.  If we merge the check with
qemu_chr_parse_* it will go away.

Thanks,

/mjt

>          print_allowed_subtypes();
>          return NULL;
> @@ -315,7 +315,7 @@ CharDriverState *qemu_chr_open_spice_port(const char *name)
>      CharDriverState *chr;
>      SpiceCharDriver *s;
>  
> -    if (name == NULL) {
> +    if (name == NULL || strlen(name) == 0) {
>          fprintf(stderr, "spice-qemu-char: missing name parameter\n");
>          return NULL;
>      }
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/4] spice-qemu-char: fix check for in-parameter

[zhanghailiang]

On 2014/11/2 14:50, Michael Tokarev wrote:
> 01.11.2014 04:50, zhanghailiang wrote:
>> For qemu_chr_open_spice_vmc and qemu_chr_open_spice_port, the in-parameter
>> never to be NULL, because the checks in qemu_chr_parse_spice_vmc
>> and qemu_chr_parse_spice_port have ensured this.
>>
>> So we should check the length of the in-parameter.
>
> The same applies here as to qemu_chr_open in patch 1/4.
> But here we've one more thing:
>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   spice-qemu-char.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
>> index 8106e06..45e7d69 100644
>> --- a/spice-qemu-char.c
>> +++ b/spice-qemu-char.c
>> @@ -290,7 +290,7 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
>>   {
>>       const char **psubtype = spice_server_char_device_recognized_subtypes();
>>
>> -    if (type == NULL) {
>> +    if (type == NULL || strlen(type) == 0) {
>>           fprintf(stderr, "spice-qemu-char: missing name parameter\n");
>
> This is 'missing TYPE parameter' not name.  If we merge the check with
> qemu_chr_parse_* it will go away.
>

OK, will fix them in V2, Thanks:)

> Thanks,
>
> /mjt
>
>>           print_allowed_subtypes();
>>           return NULL;
>> @@ -315,7 +315,7 @@ CharDriverState *qemu_chr_open_spice_port(const char *name)
>>       CharDriverState *chr;
>>       SpiceCharDriver *s;
>>
>> -    if (name == NULL) {
>> +    if (name == NULL || strlen(name) == 0) {
>>           fprintf(stderr, "spice-qemu-char: missing name parameter\n");
>>           return NULL;
>>       }
>>
>
> .
>

[Qemu-devel] [PATCH 3/4] qemu-char: convert some open functions to use Error API

[zhanghailiang]

Convert several Character backend open functions to use the Error API.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 qemu-char.c | 52 +++++++++++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 42b1d8f..52379cc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1077,7 +1077,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
     return chr;
 }
 
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts, Error **errp)
 {
     int fd_in, fd_out;
     char filename_in[CHR_MAX_FILENAME_SIZE];
@@ -1085,7 +1085,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
     const char *filename = opts->device;
 
     if (filename == NULL || strlen(filename) == 0) {
-        fprintf(stderr, "chardev: pipe: no filename given\n");
+        error_setg(errp, "chardev: pipe: no filename given");
         return NULL;
     }
 
@@ -1145,17 +1145,17 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr)
     fd_chr_close(chr);
 }
 
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts, Error **errp)
 {
     CharDriverState *chr;
 
     if (is_daemonized()) {
-        error_report("cannot use stdio with -daemonize");
+        error_setg(errp, "cannot use stdio with -daemonize");
         return NULL;
     }
 
     if (stdio_in_use) {
-        error_report("cannot use stdio by multiple character devices");
+        error_setg(errp, "cannot use stdio by multiple character devices");
         exit(1);
     }
 
@@ -1393,8 +1393,8 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
     ret->pty = g_strdup(pty_name);
     ret->has_pty = true;
 
-    fprintf(stderr, "char device redirected to %s (label %s)\n",
-            pty_name, id);
+    error_report("char device redirected to %s (label %s)",
+                 pty_name, id);
 
     s = g_malloc0(sizeof(PtyCharDriver));
     chr->opaque = s;
@@ -2062,11 +2062,11 @@ static int win_chr_pipe_poll(void *opaque)
     return 0;
 }
 
-static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
+static void win_chr_pipe_init(CharDriverState *chr, const char *filename,
+                              Error **errp)
 {
     WinCharState *s = chr->opaque;
     OVERLAPPED ov;
-    int ret;
     DWORD size;
     char openname[CHR_MAX_FILENAME_SIZE];
 
@@ -2074,12 +2074,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
 
     s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hsend) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        error_setg(errp, "Failed CreateEvent");
         goto fail;
     }
     s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hrecv) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        error_setg(errp, "Failed CreateEvent");
         goto fail;
     }
 
@@ -2089,7 +2089,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
                               PIPE_WAIT,
                               MAXCONNECT, NSENDBUF, NRECVBUF, NTIMEOUT, NULL);
     if (s->hcom == INVALID_HANDLE_VALUE) {
-        fprintf(stderr, "Failed CreateNamedPipe (%lu)\n", GetLastError());
+        error_setg(errp, "Failed CreateNamedPipe (%lu)", GetLastError());
         s->hcom = NULL;
         goto fail;
     }
@@ -2098,13 +2098,13 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
     ov.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
     ret = ConnectNamedPipe(s->hcom, &ov);
     if (ret) {
-        fprintf(stderr, "Failed ConnectNamedPipe\n");
+        error_setg(errp, "Failed ConnectNamedPipe");
         goto fail;
     }
 
     ret = GetOverlappedResult(s->hcom, &ov, &size, TRUE);
     if (!ret) {
-        fprintf(stderr, "Failed GetOverlappedResult\n");
+        error_setg(errp, "Failed GetOverlappedResult");
         if (ov.hEvent) {
             CloseHandle(ov.hEvent);
             ov.hEvent = NULL;
@@ -2117,19 +2117,19 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
         ov.hEvent = NULL;
     }
     qemu_add_polling_cb(win_chr_pipe_poll, chr);
-    return 0;
+    return;
 
  fail:
     win_chr_close(chr);
-    return -1;
 }
 
 
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts, Error **errp)
 {
     const char *filename = opts->device;
     CharDriverState *chr;
     WinCharState *s;
+    Error *local_err = NULL;
 
     chr = qemu_chr_alloc();
     s = g_malloc0(sizeof(WinCharState));
@@ -2137,9 +2137,11 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
     chr->chr_write = win_chr_write;
     chr->chr_close = win_chr_close;
 
-    if (win_chr_pipe_init(chr, filename) < 0) {
+    win_chr_pipe_init(chr, filename, &local_err);
+    if (local_err) {
         g_free(s);
         g_free(chr);
+        error_propagate(errp, local_err);
         return NULL;
     }
     return chr;
@@ -2299,7 +2301,7 @@ static void win_stdio_close(CharDriverState *chr)
     g_free(chr);
 }
 
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts, Error **errp)
 {
     CharDriverState   *chr;
     WinStdioCharState *stdio;
@@ -2311,7 +2313,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
 
     stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE);
     if (stdio->hStdIn == INVALID_HANDLE_VALUE) {
-        fprintf(stderr, "cannot open stdio: invalid handle\n");
+        error_setg(errp, "cannot open stdio: invalid handle");
         exit(1);
     }
 
@@ -2324,7 +2326,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
     if (is_console) {
         if (qemu_add_wait_object(stdio->hStdIn,
                                  win_stdio_wait_func, chr)) {
-            fprintf(stderr, "qemu_add_wait_object: failed\n");
+            error_setg(errp, "qemu_add_wait_object: failed");
         }
     } else {
         DWORD   dwId;
@@ -2337,12 +2339,12 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
         if (stdio->hInputThread == INVALID_HANDLE_VALUE
             || stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
             || stdio->hInputDoneEvent == INVALID_HANDLE_VALUE) {
-            fprintf(stderr, "cannot create stdio thread or event\n");
+            error_setg(errp, "cannot create stdio thread or event");
             exit(1);
         }
         if (qemu_add_wait_object(stdio->hInputReadyEvent,
                                  win_stdio_thread_wait_func, chr)) {
-            fprintf(stderr, "qemu_add_wait_object: failed\n");
+            error_setg(errp, "qemu_add_wait_object: failed");
         }
     }
 
@@ -4209,7 +4211,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         chr = qmp_chardev_open_parallel(backend->parallel, errp);
         break;
     case CHARDEV_BACKEND_KIND_PIPE:
-        chr = qemu_chr_open_pipe(backend->pipe);
+        chr = qemu_chr_open_pipe(backend->pipe, errp);
         break;
     case CHARDEV_BACKEND_KIND_SOCKET:
         chr = qmp_chardev_open_socket(backend->socket, errp);
@@ -4246,7 +4248,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         chr = chr_testdev_init();
         break;
     case CHARDEV_BACKEND_KIND_STDIO:
-        chr = qemu_chr_open_stdio(backend->stdio);
+        chr = qemu_chr_open_stdio(backend->stdio, errp);
         break;
 #ifdef _WIN32
     case CHARDEV_BACKEND_KIND_CONSOLE:
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 3/4] qemu-char: convert some open functions to use Error API

[Eric Blake]

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

On 10/31/2014 07:50 PM, zhanghailiang wrote:
> Convert several Character backend open functions to use the Error API.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  qemu-char.c | 52 +++++++++++++++++++++++++++-------------------------
>  1 file changed, 27 insertions(+), 25 deletions(-)

> @@ -2337,12 +2339,12 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
>          if (stdio->hInputThread == INVALID_HANDLE_VALUE
>              || stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
>              || stdio->hInputDoneEvent == INVALID_HANDLE_VALUE) {
> -            fprintf(stderr, "cannot create stdio thread or event\n");
> +            error_setg(errp, "cannot create stdio thread or event");
>              exit(1);

This conversion feels wrong.  error_setg() does not report an error, it
just stores the error for a later entity higher in the call stack to
report it.  But exit() means there is no execution of the reporting
code.  Either leave this one alone, or get rid of the exit and instead
properly propagate error status back to the caller and make sure the
caller reports it.

-- 
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: 539 bytes --]

Re: [Qemu-devel] [PATCH 3/4] qemu-char: convert some open functions to use Error API

[zhanghailiang]

On 2014/11/1 23:18, Eric Blake wrote:
> On 10/31/2014 07:50 PM, zhanghailiang wrote:
>> Convert several Character backend open functions to use the Error API.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>>   qemu-char.c | 52 +++++++++++++++++++++++++++-------------------------
>>   1 file changed, 27 insertions(+), 25 deletions(-)
>
>> @@ -2337,12 +2339,12 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
>>           if (stdio->hInputThread == INVALID_HANDLE_VALUE
>>               || stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
>>               || stdio->hInputDoneEvent == INVALID_HANDLE_VALUE) {
>> -            fprintf(stderr, "cannot create stdio thread or event\n");
>> +            error_setg(errp, "cannot create stdio thread or event");
>>               exit(1);
>
> This conversion feels wrong.  error_setg() does not report an error, it
> just stores the error for a later entity higher in the call stack to
> report it.  But exit() means there is no execution of the reporting

OK, I got it, thanks for your explanation;)

> code.  Either leave this one alone, or get rid of the exit and instead
> properly propagate error status back to the caller and make sure the
> caller reports it.
>

I will keep the exit, but use error_report instead, as other places in qemu.,thanks.

[Qemu-devel] [PATCH 4/4] spice-qemu-char: convert open functions to use Error API

[zhanghailiang]

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 include/ui/qemu-spice.h     |  9 +++++----
 qemu-char.c                 |  4 ++--
 spice-qemu-char.c           | 10 +++++-----
 stubs/qemu-chr-open-spice.c |  4 ++--
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index a93b4b2..45a4737 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -27,7 +27,7 @@
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "monitor/monitor.h"
-
+#include "qapi/error.h"
 extern int using_spice;
 
 void qemu_spice_init(void);
@@ -48,12 +48,13 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
 void do_info_spice_print(Monitor *mon, const QObject *data);
 void do_info_spice(Monitor *mon, QObject **ret_data);
 
-CharDriverState *qemu_chr_open_spice_vmc(const char *type);
+CharDriverState *qemu_chr_open_spice_vmc(const char *type, Error **errp);
 #if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name);
+CharDriverState *qemu_chr_open_spice_port(const char *name, Error **errp);
 void qemu_spice_register_ports(void);
 #else
-static inline CharDriverState *qemu_chr_open_spice_port(const char *name)
+static inline CharDriverState *qemu_chr_open_spice_port(const char *name
+                                                        Error **errp)
 { return NULL; }
 #endif
 
diff --git a/qemu-char.c b/qemu-char.c
index 52379cc..6de6221 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4257,10 +4257,10 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
 #endif
 #ifdef CONFIG_SPICE
     case CHARDEV_BACKEND_KIND_SPICEVMC:
-        chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
+        chr = qemu_chr_open_spice_vmc(backend->spicevmc->type, errp);
         break;
     case CHARDEV_BACKEND_KIND_SPICEPORT:
-        chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
+        chr = qemu_chr_open_spice_port(backend->spiceport->fqdn, errp);
         break;
 #endif
     case CHARDEV_BACKEND_KIND_VC:
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 45e7d69..b693a7d 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -286,12 +286,12 @@ static CharDriverState *chr_open(const char *subtype,
     return chr;
 }
 
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
+CharDriverState *qemu_chr_open_spice_vmc(const char *type, Error **errp)
 {
     const char **psubtype = spice_server_char_device_recognized_subtypes();
 
     if (type == NULL || strlen(type) == 0) {
-        fprintf(stderr, "spice-qemu-char: missing name parameter\n");
+        error_setg(errp, "spice-qemu-char: missing name parameter");
         print_allowed_subtypes();
         return NULL;
     }
@@ -301,7 +301,7 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
         }
     }
     if (*psubtype == NULL) {
-        fprintf(stderr, "spice-qemu-char: unsupported type: %s\n", type);
+        error_setg(errp, "spice-qemu-char: unsupported type: %s", type);
         print_allowed_subtypes();
         return NULL;
     }
@@ -310,13 +310,13 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
 }
 
 #if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name)
+CharDriverState *qemu_chr_open_spice_port(const char *name, Error **errp)
 {
     CharDriverState *chr;
     SpiceCharDriver *s;
 
     if (name == NULL || strlen(name) == 0) {
-        fprintf(stderr, "spice-qemu-char: missing name parameter\n");
+        error_setg(errp, "spice-qemu-char: missing name parameter");
         return NULL;
     }
 
diff --git a/stubs/qemu-chr-open-spice.c b/stubs/qemu-chr-open-spice.c
index f1c4849..98ac0b7 100644
--- a/stubs/qemu-chr-open-spice.c
+++ b/stubs/qemu-chr-open-spice.c
@@ -1,13 +1,13 @@
 #include "qemu-common.h"
 #include "ui/qemu-spice.h"
 
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
+CharDriverState *qemu_chr_open_spice_vmc(const char *type, Error **errp)
 {
     return NULL;
 }
 
 #if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name)
+CharDriverState *qemu_chr_open_spice_port(const char *name, Error **errp)
 {
     return NULL;
 }
-- 
1.7.12.4