diff mbox

[9/9] vhost-user: Improve -netdev/netdev_add/-net/... error reporting

Message ID 1432815695-31687-10-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster May 28, 2015, 12:21 p.m. UTC
When -netdev vhost-user fails, it first reports a specific error, then
one or more generic ones, like this:

    $ qemu-system-x86_64 -netdev vhost-user,id=foo,chardev=xxx
    qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: chardev "xxx" not found
    qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: No suitable chardev found
    qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: Device 'vhost-user' could not be initialized

With the command line, the messages go to stderr.  In HMP, they go to
the monitor.  In QMP, the last one becomes the error reply, and the
others go to stderr.

Convert net_init_vhost_user() and its helpers to Error.  This
suppresses the unwanted unspecific error messages, and makes the
specific error the QMP error reply.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net/vhost-user.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

Comments

Eric Blake May 28, 2015, 7:20 p.m. UTC | #1
On 05/28/2015 06:21 AM, Markus Armbruster wrote:
> When -netdev vhost-user fails, it first reports a specific error, then
> one or more generic ones, like this:
> 
>     $ qemu-system-x86_64 -netdev vhost-user,id=foo,chardev=xxx
>     qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: chardev "xxx" not found
>     qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: No suitable chardev found
>     qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: Device 'vhost-user' could not be initialized
> 
> With the command line, the messages go to stderr.  In HMP, they go to
> the monitor.  In QMP, the last one becomes the error reply, and the
> others go to stderr.
> 
> Convert net_init_vhost_user() and its helpers to Error.  This
> suppresses the unwanted unspecific error messages, and makes the
> specific error the QMP error reply.
> 
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  net/vhost-user.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Stefan Hajnoczi June 2, 2015, 4:32 p.m. UTC | #2
On Thu, May 28, 2015 at 02:21:35PM +0200, Markus Armbruster wrote:
> When -netdev vhost-user fails, it first reports a specific error, then
> one or more generic ones, like this:
> 
>     $ qemu-system-x86_64 -netdev vhost-user,id=foo,chardev=xxx
>     qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: chardev "xxx" not found
>     qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: No suitable chardev found
>     qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: Device 'vhost-user' could not be initialized
> 
> With the command line, the messages go to stderr.  In HMP, they go to
> the monitor.  In QMP, the last one becomes the error reply, and the
> others go to stderr.
> 
> Convert net_init_vhost_user() and its helpers to Error.  This
> suppresses the unwanted unspecific error messages, and makes the
> specific error the QMP error reply.
> 
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  net/vhost-user.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox

Patch

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 2effe29..49d340e 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -166,34 +166,34 @@  static int net_vhost_chardev_opts(void *opaque,
     } else if (strcmp(name, "server") == 0) {
         props->is_server = true;
     } else {
-        error_report("vhost-user does not support a chardev"
-                     " with the following option:\n %s = %s",
-                     name, value);
+        error_setg(errp,
+                   "vhost-user does not support a chardev with option %s=%s",
+                   name, value);
         return -1;
     }
     return 0;
 }
 
-static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *opts)
+static CharDriverState *net_vhost_parse_chardev(
+    const NetdevVhostUserOptions *opts, Error **errp)
 {
     CharDriverState *chr = qemu_chr_find(opts->chardev);
     VhostUserChardevProps props;
 
     if (chr == NULL) {
-        error_report("chardev \"%s\" not found", opts->chardev);
+        error_setg(errp, "chardev \"%s\" not found", opts->chardev);
         return NULL;
     }
 
     /* inspect chardev opts */
     memset(&props, 0, sizeof(props));
-    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props,
-                         &error_abort)) {
+    if (qemu_opt_foreach(chr->opts, net_vhost_chardev_opts, &props, errp)) {
         return NULL;
     }
 
     if (!props.is_socket || !props.is_unix) {
-        error_report("chardev \"%s\" is not a unix socket",
-                     opts->chardev);
+        error_setg(errp, "chardev \"%s\" is not a unix socket",
+                   opts->chardev);
         return NULL;
     }
 
@@ -217,7 +217,7 @@  static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
 
     if (strcmp(netdev, name) == 0 &&
         strncmp(driver, virtio_name, strlen(virtio_name)) != 0) {
-        error_report("vhost-user requires frontend driver virtio-net-*");
+        error_setg(errp, "vhost-user requires frontend driver virtio-net-*");
         return -1;
     }
 
@@ -227,25 +227,22 @@  static int net_vhost_check_net(void *opaque, QemuOpts *opts, Error **errp)
 int net_init_vhost_user(const NetClientOptions *opts, const char *name,
                         NetClientState *peer, Error **errp)
 {
-    /* FIXME error_setg(errp, ...) on failure */
     const NetdevVhostUserOptions *vhost_user_opts;
     CharDriverState *chr;
 
     assert(opts->kind == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
     vhost_user_opts = opts->vhost_user;
 
-    chr = net_vhost_parse_chardev(vhost_user_opts);
+    chr = net_vhost_parse_chardev(vhost_user_opts, errp);
     if (!chr) {
-        error_report("No suitable chardev found");
         return -1;
     }
 
     /* verify net frontend */
     if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
-                          (char *)name, &error_abort)) {
+                          (char *)name, errp)) {
         return -1;
     }
 
-
     return net_vhost_user_init(peer, "vhost_user", name, chr);
 }