Patchwork [09/10] qdev: Use QError for 'device not found' error

login
register
mail settings
Submitter Luiz Capitulino
Date Nov. 17, 2009, 7:43 p.m.
Message ID <1258487037-24950-10-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/38689/
State New
Headers show

Comments

Luiz Capitulino - Nov. 17, 2009, 7:43 p.m.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/qdev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Markus Armbruster - Nov. 18, 2009, 3:17 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  hw/qdev.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index d19d531..875ca50 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -29,6 +29,7 @@
>  #include "qdev.h"
>  #include "sysemu.h"
>  #include "monitor.h"
> +#include "qerror.h"
>  
>  static int qdev_hotplug = 0;
>  
> @@ -176,8 +177,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      /* find driver */
>      info = qdev_find_info(NULL, driver);
>      if (!info) {
> -        qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
> -                   driver);
> +        qemu_error_new(QERR_DEVICE_NOT_FOUND, driver);
>          return NULL;
>      }
>      if (info->no_user) {

Not obvious from this patch, but we lose the "Try -device '?' for a
list" hint here.  In PATCH 7/10:

+#define QERR_DEVICE_NOT_FOUND \
+        "{ 'class': 'DeviceNotFound', 'data': { 'name': %s } }"
+

and

+    {
+        .error_fmt   = QERR_DEVICE_NOT_FOUND,
+        .desc        = "device \"%(name)\" not found",
+    },

Not a deal-breaker for me.
Luiz Capitulino - Nov. 18, 2009, 5:32 p.m.
On Wed, 18 Nov 2009 16:17:02 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hw/qdev.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/qdev.c b/hw/qdev.c
> > index d19d531..875ca50 100644
> > --- a/hw/qdev.c
> > +++ b/hw/qdev.c
> > @@ -29,6 +29,7 @@
> >  #include "qdev.h"
> >  #include "sysemu.h"
> >  #include "monitor.h"
> > +#include "qerror.h"
> >  
> >  static int qdev_hotplug = 0;
> >  
> > @@ -176,8 +177,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> >      /* find driver */
> >      info = qdev_find_info(NULL, driver);
> >      if (!info) {
> > -        qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
> > -                   driver);
> > +        qemu_error_new(QERR_DEVICE_NOT_FOUND, driver);
> >          return NULL;
> >      }
> >      if (info->no_user) {
> 
> Not obvious from this patch, but we lose the "Try -device '?' for a
> list" hint here. 

 Yes, this happens because this is a generic error and '-device' is
qdev command-line specific.
Amit Shah - Nov. 20, 2009, 7:23 a.m.
On (Wed) Nov 18 2009 [16:17:02], Markus Armbruster wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > @@ -176,8 +177,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> >      /* find driver */
> >      info = qdev_find_info(NULL, driver);
> >      if (!info) {
> > -        qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
> > -                   driver);
> > +        qemu_error_new(QERR_DEVICE_NOT_FOUND, driver);
> >          return NULL;
> >      }
> >      if (info->no_user) {
> 
> Not obvious from this patch, but we lose the "Try -device '?' for a
> list" hint here.  In PATCH 7/10:

BTW that hint isn't always appropriate as it's printed on the monitor
when doing 'device_add' as well.

		Amit

Patch

diff --git a/hw/qdev.c b/hw/qdev.c
index d19d531..875ca50 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,7 @@ 
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "qerror.h"
 
 static int qdev_hotplug = 0;
 
@@ -176,8 +177,7 @@  DeviceState *qdev_device_add(QemuOpts *opts)
     /* find driver */
     info = qdev_find_info(NULL, driver);
     if (!info) {
-        qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
-                   driver);
+        qemu_error_new(QERR_DEVICE_NOT_FOUND, driver);
         return NULL;
     }
     if (info->no_user) {