qemu-char: Print strerror message on failure

The only way for chardev drivers to communicate an error was to return a NULL
pointer, which resulted in an error message that said _that_ something went
wrong, but not _why_.

This patch changes the interface to return 0/-errno and updates
qemu_chr_open_opts to use strerror to display a more helpful error message.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 console.c         |    8 ++-
 console.h         |    2 +-
 hw/baum.c         |    7 +-
 hw/msmouse.c      |    5 +-
 hw/msmouse.h      |    2 +-
 qemu-char.c       |  165 +++++++++++++++++++++++++++++++----------------------
 spice-qemu-char.c |    9 ++-
 ui/qemu-spice.h   |    2 +-
 8 files changed, 117 insertions(+), 83 deletions(-)

Am 01.06.2011 13:29, schrieb Kevin Wolf:
> The only way for chardev drivers to communicate an error was to return a NULL
> pointer, which resulted in an error message that said _that_ something went
> wrong, but not _why_.
> 
> This patch changes the interface to return 0/-errno and updates
> qemu_chr_open_opts to use strerror to display a more helpful error message.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Ping?

Kevin Wolf <kwolf@redhat.com> writes:

> The only way for chardev drivers to communicate an error was to return a NULL
> pointer, which resulted in an error message that said _that_ something went
> wrong, but not _why_.
>
> This patch changes the interface to return 0/-errno and updates
> qemu_chr_open_opts to use strerror to display a more helpful error message.

Returning the result through a pointer is awkward.  What about stuffing
the error code into errno?

Am 21.06.2011 18:09, schrieb Markus Armbruster:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
>> The only way for chardev drivers to communicate an error was to return a NULL
>> pointer, which resulted in an error message that said _that_ something went
>> wrong, but not _why_.
>>
>> This patch changes the interface to return 0/-errno and updates
>> qemu_chr_open_opts to use strerror to display a more helpful error message.
> 
> Returning the result through a pointer is awkward.  What about stuffing
> the error code into errno?

I generally like it better to return error codes explicitly instead of
storing them in some global variable where it tends to be trampled over
before you print the message. But if people prefer it that way, I can
redo the patch.

Kevin

On 06/21/2011 11:18 AM, Kevin Wolf wrote:
> Am 21.06.2011 18:09, schrieb Markus Armbruster:
>> Kevin Wolf<kwolf@redhat.com>  writes:
>>
>>> The only way for chardev drivers to communicate an error was to return a NULL
>>> pointer, which resulted in an error message that said _that_ something went
>>> wrong, but not _why_.
>>>
>>> This patch changes the interface to return 0/-errno and updates
>>> qemu_chr_open_opts to use strerror to display a more helpful error message.
>>
>> Returning the result through a pointer is awkward.  What about stuffing
>> the error code into errno?
>
> I generally like it better to return error codes explicitly instead of
> storing them in some global variable where it tends to be trampled over
> before you print the message. But if people prefer it that way, I can
> redo the patch.

I think passing a pointer to an int to receive the error is best.  That 
way a NULL can be passed to indicate that the caller doesn't care.

This matches the Error model that we're proposing else where.

Regards,

Anthony Liguori

>
> Kevin
>

On 06/01/2011 06:29 AM, Kevin Wolf wrote:
> The only way for chardev drivers to communicate an error was to return a NULL
> pointer, which resulted in an error message that said _that_ something went
> wrong, but not _why_.
>
> This patch changes the interface to return 0/-errno and updates
> qemu_chr_open_opts to use strerror to display a more helpful error message.
>
> Signed-off-by: Kevin Wolf<kwolf@redhat.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   console.c         |    8 ++-
>   console.h         |    2 +-
>   hw/baum.c         |    7 +-
>   hw/msmouse.c      |    5 +-
>   hw/msmouse.h      |    2 +-
>   qemu-char.c       |  165 +++++++++++++++++++++++++++++++----------------------
>   spice-qemu-char.c |    9 ++-
>   ui/qemu-spice.h   |    2 +-
>   8 files changed, 117 insertions(+), 83 deletions(-)
>
> diff --git a/console.c b/console.c
> index 871c1d4..314d625 100644
> --- a/console.c
> +++ b/console.c
> @@ -1507,7 +1507,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
>           chr->init(chr);
>   }
>
> -CharDriverState *text_console_init(QemuOpts *opts)
> +int text_console_init(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr;
>       TextConsole *s;
> @@ -1539,7 +1539,7 @@ CharDriverState *text_console_init(QemuOpts *opts)
>
>       if (!s) {
>           free(chr);
> -        return NULL;
> +        return -EBUSY;
>       }
>
>       s->chr = chr;
> @@ -1547,7 +1547,9 @@ CharDriverState *text_console_init(QemuOpts *opts)
>       s->g_height = height;
>       chr->opaque = s;
>       chr->chr_set_echo = text_console_set_echo;
> -    return chr;
> +
> +    *_chr = chr;
> +    return 0;
>   }
>
>   void text_consoles_set_display(DisplayState *ds)
> diff --git a/console.h b/console.h
> index 64d1f09..c09537b 100644
> --- a/console.h
> +++ b/console.h
> @@ -354,7 +354,7 @@ void vga_hw_text_update(console_ch_t *chardata);
>
>   int is_graphic_console(void);
>   int is_fixedsize_console(void);
> -CharDriverState *text_console_init(QemuOpts *opts);
> +int text_console_init(QemuOpts *opts, CharDriverState **_chr);
>   void text_consoles_set_display(DisplayState *ds);
>   void console_select(unsigned int index);
>   void console_color_init(DisplayState *ds);
> diff --git a/hw/baum.c b/hw/baum.c
> index 2aaf5ff..33a22a7 100644
> --- a/hw/baum.c
> +++ b/hw/baum.c
> @@ -576,7 +576,7 @@ static void baum_close(struct CharDriverState *chr)
>       qemu_free(baum);
>   }
>
> -CharDriverState *chr_baum_init(QemuOpts *opts)
> +int chr_baum_init(QemuOpts *opts, CharDriverState **_chr)
>   {
>       BaumDriverState *baum;
>       CharDriverState *chr;
> @@ -629,7 +629,8 @@ CharDriverState *chr_baum_init(QemuOpts *opts)
>
>       qemu_chr_generic_open(chr);
>
> -    return chr;
> +    *_chr = chr;
> +    return 0;
>
>   fail:
>       qemu_free_timer(baum->cellCount_timer);
> @@ -638,5 +639,5 @@ fail_handle:
>       qemu_free(handle);
>       qemu_free(chr);
>       qemu_free(baum);
> -    return NULL;
> +    return -EIO;
>   }
> diff --git a/hw/msmouse.c b/hw/msmouse.c
> index 05f893c..67c6cd4 100644
> --- a/hw/msmouse.c
> +++ b/hw/msmouse.c
> @@ -64,7 +64,7 @@ static void msmouse_chr_close (struct CharDriverState *chr)
>       qemu_free (chr);
>   }
>
> -CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts)
> +int qemu_chr_open_msmouse(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr;
>
> @@ -74,5 +74,6 @@ CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts)
>
>       qemu_add_mouse_event_handler(msmouse_event, chr, 0, "QEMU Microsoft Mouse");
>
> -    return chr;
> +    *_chr = chr;
> +    return 0;
>   }
> diff --git a/hw/msmouse.h b/hw/msmouse.h
> index 456cb21..8b853b3 100644
> --- a/hw/msmouse.h
> +++ b/hw/msmouse.h
> @@ -1,2 +1,2 @@
>   /* msmouse.c */
> -CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts);
> +int qemu_chr_open_msmouse(QemuOpts *opts, CharDriverState **_chr);
> diff --git a/qemu-char.c b/qemu-char.c
> index 5e04a20..a8e4094 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -220,13 +220,15 @@ static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>       return len;
>   }
>
> -static CharDriverState *qemu_chr_open_null(QemuOpts *opts)
> +static int qemu_chr_open_null(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr;
>
>       chr = qemu_mallocz(sizeof(CharDriverState));
>       chr->chr_write = null_chr_write;
> -    return chr;
> +
> +    *_chr= chr;
> +    return 0;
>   }
>
>   /* MUX driver for serial I/O splitting */
> @@ -635,18 +637,21 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
>       return chr;
>   }
>
> -static CharDriverState *qemu_chr_open_file_out(QemuOpts *opts)
> +static int qemu_chr_open_file_out(QemuOpts *opts, CharDriverState **_chr)
>   {
>       int fd_out;
>
>       TFR(fd_out = qemu_open(qemu_opt_get(opts, "path"),
>                         O_WRONLY | O_TRUNC | O_CREAT | O_BINARY, 0666));
> -    if (fd_out<  0)
> -        return NULL;
> -    return qemu_chr_open_fd(-1, fd_out);
> +    if (fd_out<  0) {
> +        return -errno;
> +    }
> +
> +    *_chr = qemu_chr_open_fd(-1, fd_out);
> +    return 0;
>   }
>
> -static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts)
> +static int qemu_chr_open_pipe(QemuOpts *opts, CharDriverState **_chr)
>   {
>       int fd_in, fd_out;
>       char filename_in[256], filename_out[256];
> @@ -654,7 +659,7 @@ static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts)
>
>       if (filename == NULL) {
>           fprintf(stderr, "chardev: pipe: no filename given\n");
> -        return NULL;
> +        return -EINVAL;
>       }
>
>       snprintf(filename_in, 256, "%s.in", filename);
> @@ -666,11 +671,14 @@ static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts)
>   	    close(fd_in);
>   	if (fd_out>= 0)
>   	    close(fd_out);
> -        TFR(fd_in = fd_out = open(filename, O_RDWR | O_BINARY));
> -        if (fd_in<  0)
> -            return NULL;
> +        TFR(fd_in = fd_out = qemu_open(filename, O_RDWR | O_BINARY));
> +        if (fd_in<  0) {
> +            return -errno;
> +        }
>       }
> -    return qemu_chr_open_fd(fd_in, fd_out);
> +
> +    *_chr = qemu_chr_open_fd(fd_in, fd_out);
> +    return 0;
>   }
>
>
> @@ -761,12 +769,14 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr)
>       fd_chr_close(chr);
>   }
>
> -static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts)
> +static int qemu_chr_open_stdio(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr;
>
> -    if (stdio_nb_clients>= STDIO_MAX_CLIENTS)
> -        return NULL;
> +    if (stdio_nb_clients>= STDIO_MAX_CLIENTS) {
> +        return -EBUSY;
> +    }
> +
>       if (stdio_nb_clients == 0) {
>           old_fd0_flags = fcntl(0, F_GETFL);
>           tcgetattr (0,&oldtty);
> @@ -783,7 +793,8 @@ static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts)
>                                              display_type != DT_NOGRAPHIC);
>       qemu_chr_set_echo(chr, false);
>
> -    return chr;
> +    *_chr = chr;
> +    return 0;
>   }
>
>   #ifdef __sun__
> @@ -970,7 +981,7 @@ static void pty_chr_close(struct CharDriverState *chr)
>       qemu_chr_event(chr, CHR_EVENT_CLOSED);
>   }
>
> -static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
> +static int qemu_chr_open_pty(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr;
>       PtyCharDriver *s;
> @@ -988,7 +999,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
>       s = qemu_mallocz(sizeof(PtyCharDriver));
>
>       if (openpty(&s->fd,&slave_fd, pty_name, NULL, NULL)<  0) {
> -        return NULL;
> +        return -errno;
>       }
>
>       /* Set raw attributes on the pty. */
> @@ -1010,7 +1021,8 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
>
>       s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr);
>
> -    return chr;
> +    *_chr = chr;
> +    return 0;
>   }
>
>   static void tty_serial_init(int fd, int speed,
> @@ -1211,30 +1223,28 @@ static void qemu_chr_close_tty(CharDriverState *chr)
>       }
>   }
>
> -static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
> +static int qemu_chr_open_tty(QemuOpts *opts, CharDriverState **_chr)
>   {
>       const char *filename = qemu_opt_get(opts, "path");
>       CharDriverState *chr;
>       int fd;
>
> -    TFR(fd = open(filename, O_RDWR | O_NONBLOCK));
> +    TFR(fd = qemu_open(filename, O_RDWR | O_NONBLOCK));
>       if (fd<  0) {
> -        return NULL;
> +        return -errno;
>       }
>       tty_serial_init(fd, 115200, 'N', 8, 1);
>       chr = qemu_chr_open_fd(fd, fd);
> -    if (!chr) {
> -        close(fd);
> -        return NULL;
> -    }
>       chr->chr_ioctl = tty_serial_ioctl;
>       chr->chr_close = qemu_chr_close_tty;
> -    return chr;
> +
> +    *_chr = chr;
> +    return 0;
>   }
>   #else  /* ! __linux__&&  ! __sun__ */
> -static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
> +static int qemu_chr_open_pty(QemuOpts *opts, CharDriverState **_chr)
>   {
> -    return NULL;
> +    return -ENOTSUP;
>   }
>   #endif /* __linux__ || __sun__ */
>
> @@ -1348,7 +1358,7 @@ static void pp_close(CharDriverState *chr)
>       qemu_chr_event(chr, CHR_EVENT_CLOSED);
>   }
>
> -static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
> +static int qemu_chr_open_pp(QemuOpts *opts, CharDriverState **_chr)
>   {
>       const char *filename = qemu_opt_get(opts, "path");
>       CharDriverState *chr;
> @@ -1356,12 +1366,13 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
>       int fd;
>
>       TFR(fd = open(filename, O_RDWR));
> -    if (fd<  0)
> -        return NULL;
> +    if (fd<  0) {
> +        return -errno;
> +    }
>
>       if (ioctl(fd, PPCLAIM)<  0) {
>           close(fd);
> -        return NULL;
> +        return -errno;
>       }
>
>       drv = qemu_mallocz(sizeof(ParallelCharDriver));
> @@ -1376,7 +1387,8 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
>
>       qemu_chr_generic_open(chr);
>
> -    return chr;
> +    *_chr = chr;
> +    return 0;
>   }
>   #endif /* __linux__ */
>
> @@ -1418,21 +1430,24 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
>       return 0;
>   }
>
> -static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
> +static int qemu_chr_open_pp(QemuOpts *opts, CharDriverState **_chr)
>   {
>       const char *filename = qemu_opt_get(opts, "path");
>       CharDriverState *chr;
>       int fd;
>
> -    fd = open(filename, O_RDWR);
> -    if (fd<  0)
> -        return NULL;
> +    fd = qemu_open(filename, O_RDWR);
> +    if (fd<  0) {
> +        return -errno;
> +    }
>
>       chr = qemu_mallocz(sizeof(CharDriverState));
>       chr->opaque = (void *)(intptr_t)fd;
>       chr->chr_write = null_chr_write;
>       chr->chr_ioctl = pp_ioctl;
> -    return chr;
> +
> +    *_chr = chr;
> +    return 0;
>   }
>   #endif
>
> @@ -1638,7 +1653,7 @@ static int win_chr_poll(void *opaque)
>       return 0;
>   }
>
> -static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
> +static int qemu_chr_open_win(QemuOpts *opts, CharDriverState **_chr)
>   {
>       const char *filename = qemu_opt_get(opts, "path");
>       CharDriverState *chr;
> @@ -1653,10 +1668,12 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
>       if (win_chr_init(chr, filename)<  0) {
>           free(s);
>           free(chr);
> -        return NULL;
> +        return -EIO;
>       }
>       qemu_chr_generic_open(chr);
> -    return chr;
> +
> +    *_chr = chr;
> +    return 0;
>   }
>
>   static int win_chr_pipe_poll(void *opaque)
> @@ -1738,7 +1755,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
>   }
>
>
> -static CharDriverState *qemu_chr_open_win_pipe(QemuOpts *opts)
> +static int qemu_chr_open_win_pipe(QemuOpts *opts, CharDriverState **_chr)
>   {
>       const char *filename = qemu_opt_get(opts, "path");
>       CharDriverState *chr;
> @@ -1753,10 +1770,12 @@ static CharDriverState *qemu_chr_open_win_pipe(QemuOpts *opts)
>       if (win_chr_pipe_init(chr, filename)<  0) {
>           free(s);
>           free(chr);
> -        return NULL;
> +        return -EIO;
>       }
>       qemu_chr_generic_open(chr);
> -    return chr;
> +
> +    *_chr = chr;
> +    return 0;
>   }
>
>   static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
> @@ -1773,22 +1792,23 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
>       return chr;
>   }
>
> -static CharDriverState *qemu_chr_open_win_con(QemuOpts *opts)
> +static int qemu_chr_open_win_con(QemuOpts *opts, CharDriverState **_chr)
>   {
> -    return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE));
> +    return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE), chr);
>   }
>
> -static CharDriverState *qemu_chr_open_win_file_out(QemuOpts *opts)
> +static int qemu_chr_open_win_file_out(QemuOpts *opts, CharDriverState **_chr)
>   {
>       const char *file_out = qemu_opt_get(opts, "path");
>       HANDLE fd_out;
>
>       fd_out = CreateFile(file_out, GENERIC_WRITE, FILE_SHARE_READ, NULL,
>                           OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
> -    if (fd_out == INVALID_HANDLE_VALUE)
> -        return NULL;
> +    if (fd_out == INVALID_HANDLE_VALUE) {
> +        return -EIO;
> +    }
>
> -    return qemu_chr_open_win_file(fd_out);
> +    return qemu_chr_open_win_file(fd_out, _chr);
>   }
>   #endif /* !_WIN32 */
>
> @@ -1869,11 +1889,12 @@ static void udp_chr_close(CharDriverState *chr)
>       qemu_chr_event(chr, CHR_EVENT_CLOSED);
>   }
>
> -static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
> +static int qemu_chr_open_udp(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr = NULL;
>       NetCharDriver *s = NULL;
>       int fd = -1;
> +    int ret;
>
>       chr = qemu_mallocz(sizeof(CharDriverState));
>       s = qemu_mallocz(sizeof(NetCharDriver));
> @@ -1881,6 +1902,7 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
>       fd = inet_dgram_opts(opts);
>       if (fd<  0) {
>           fprintf(stderr, "inet_dgram_opts failed\n");
> +        ret = -errno;
>           goto return_err;
>       }
>
> @@ -1891,16 +1913,17 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
>       chr->chr_write = udp_chr_write;
>       chr->chr_update_read_handler = udp_chr_update_read_handler;
>       chr->chr_close = udp_chr_close;
> -    return chr;
> +
> +    *_chr = chr;
> +    return 0;
>
>   return_err:
> -    if (chr)
> -        free(chr);
> -    if (s)
> -        free(s);
> -    if (fd>= 0)
> +    qemu_free(chr);
> +    qemu_free(s);
> +    if (fd>= 0) {
>           closesocket(fd);
> -    return NULL;
> +    }
> +    return ret;
>   }
>
>   /***********************************************************/
> @@ -2179,7 +2202,7 @@ static void tcp_chr_close(CharDriverState *chr)
>       qemu_chr_event(chr, CHR_EVENT_CLOSED);
>   }
>
> -static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
> +static int qemu_chr_open_socket(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr = NULL;
>       TCPCharDriver *s = NULL;
> @@ -2189,6 +2212,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
>       int do_nodelay;
>       int is_unix;
>       int is_telnet;
> +    int ret;
>
>       is_listen      = qemu_opt_get_bool(opts, "server", 0);
>       is_waitconnect = qemu_opt_get_bool(opts, "wait", 1);
> @@ -2214,8 +2238,10 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
>               fd = inet_connect_opts(opts);
>           }
>       }
> -    if (fd<  0)
> +    if (fd<  0) {
> +        ret = -errno;
>           goto fail;
> +    }
>
>       if (!is_waitconnect)
>           socket_set_nonblock(fd);
> @@ -2267,14 +2293,16 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
>           tcp_chr_accept(chr);
>           socket_set_nonblock(s->listen_fd);
>       }
> -    return chr;
> +
> +    *_chr = chr;
> +    return 0;
>
>    fail:
>       if (fd>= 0)
>           closesocket(fd);
>       qemu_free(s);
>       qemu_free(chr);
> -    return NULL;
> +    return ret;
>   }
>
>   /***********************************************************/
> @@ -2467,7 +2495,7 @@ fail:
>
>   static const struct {
>       const char *name;
> -    CharDriverState *(*open)(QemuOpts *opts);
> +    int (*open)(QemuOpts *opts, CharDriverState **chr);
>   } backend_table[] = {
>       { .name = "null",      .open = qemu_chr_open_null },
>       { .name = "socket",    .open = qemu_chr_open_socket },
> @@ -2507,6 +2535,7 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
>   {
>       CharDriverState *chr;
>       int i;
> +    int ret;
>
>       if (qemu_opts_id(opts) == NULL) {
>           fprintf(stderr, "chardev: no id specified\n");
> @@ -2528,10 +2557,10 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
>           return NULL;
>       }
>
> -    chr = backend_table[i].open(opts);
> -    if (!chr) {
> -        fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
> -                qemu_opt_get(opts, "backend"));
> +    ret = backend_table[i].open(opts,&chr);
> +    if (ret<  0) {
> +        fprintf(stderr, "chardev: opening backend \"%s\" failed: %s\n",
> +                qemu_opt_get(opts, "backend"), strerror(-ret));
>           return NULL;
>       }
>
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index fa15a71..bf004c3 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -160,7 +160,7 @@ static void print_allowed_subtypes(void)
>       fprintf(stderr, "\n");
>   }
>
> -CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
> +int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr)
>   {
>       CharDriverState *chr;
>       SpiceCharDriver *s;
> @@ -172,7 +172,7 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
>       if (name == NULL) {
>           fprintf(stderr, "spice-qemu-char: missing name parameter\n");
>           print_allowed_subtypes();
> -        return NULL;
> +        return -EINVAL;
>       }
>       for(;*psubtype != NULL; ++psubtype) {
>           if (strcmp(name, *psubtype) == 0) {
> @@ -183,7 +183,7 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
>       if (subtype == NULL) {
>           fprintf(stderr, "spice-qemu-char: unsupported name\n");
>           print_allowed_subtypes();
> -        return NULL;
> +        return -EINVAL;
>       }
>
>       chr = qemu_mallocz(sizeof(CharDriverState));
> @@ -200,5 +200,6 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
>
>       qemu_chr_generic_open(chr);
>
> -    return chr;
> +    *_chr = chr;
> +    return 0;
>   }
> diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
> index 3c6f1fe..f34be69 100644
> --- a/ui/qemu-spice.h
> +++ b/ui/qemu-spice.h
> @@ -42,7 +42,7 @@ 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(QemuOpts *opts);
> +int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr);
>
>   #else  /* CONFIG_SPICE */
>