qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] qemu-char: Print strerror message on failure


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] qemu-char: Print strerror message on failure
Date: Sat, 23 Jul 2011 11:53:06 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110516 Lightning/1.0b2 Thunderbird/3.1.10

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<address@hidden>

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 */





reply via email to

[Prev in Thread] Current Thread [Next in Thread]