diff mbox

[RFC,48/48] monitor: convert do_device_add() to QObject

Message ID 1267034160-3517-49-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Feb. 24, 2010, 5:56 p.m. UTC 
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/qdev.c       |   29 +++++++++++++++++++++--------
 hw/qdev.h       |    2 +-
 qemu-monitor.hx |    3 ++-
 3 files changed, 24 insertions(+), 10 deletions(-)

Comments

Luiz Capitulino Feb. 26, 2010, 7:47 p.m. UTC | #1 
On Wed, 24 Feb 2010 18:56:00 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/qdev.c       |   29 +++++++++++++++++++++--------
>  hw/qdev.h       |    2 +-
>  qemu-monitor.hx |    3 ++-
>  3 files changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index bcd989c..a631492 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -586,7 +586,9 @@ static BusState *qbus_find(const char *path)
>          dev = qbus_find_dev(bus, elem);
>          if (!dev) {
>              qemu_error_new(QERR_DEVICE_NOT_FOUND, elem);
> -            qbus_list_dev(bus);
> +            if (!in_qmp_mon()) {
> +                qbus_list_dev(bus);
> +            }
>              return NULL;
>          }
>  
> @@ -605,7 +607,9 @@ static BusState *qbus_find(const char *path)
>                  return QLIST_FIRST(&dev->child_bus);
>              default:
>                  qemu_error_new(QERR_DEVICE_MULTIPLE_BUSSES, elem);
> -                qbus_list_bus(dev);
> +                if (!in_qmp_mon()) {
> +                    qbus_list_bus(dev);
> +                }
>                  return NULL;
>              }
>          }
> @@ -619,7 +623,9 @@ static BusState *qbus_find(const char *path)
>          bus = qbus_find_bus(dev, elem);
>          if (!bus) {
>              qemu_error_new(QERR_BUS_NOT_FOUND, elem);
> -            qbus_list_bus(dev);
> +            if (!in_qmp_mon()) {
> +                qbus_list_bus(dev);
> +            }
>              return NULL;
>          }
>      }
> @@ -762,16 +768,23 @@ void do_info_qdm(Monitor *mon)
>      }
>  }
>  
> -void do_device_add(Monitor *mon, const QDict *qdict)
> +int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
>  {
>      QemuOpts *opts;
>  
>      opts = qemu_opts_from_qdict(&qemu_device_opts, qdict);
> -    if (opts) {
> -        if (qdev_device_help(opts) || qdev_device_add(opts) == NULL) {
> -            qemu_opts_del(opts);
> -        }
> +    if (!opts) {
> +        return -1;
>      }
> +    if (!in_qmp_mon() && qdev_device_help(opts)) {
> +        qemu_opts_del(opts);
> +        return 0;
> +    }
> +    if (!qdev_device_add(opts)) {
> +        qemu_opts_del(opts);
> +        return -1;
> +    }
> +    return 0;
>  }

 We need at least basic documentation of the accepted parameters and
an example, just like all others QMP-enabled handlers.

>  
>  void do_device_del(Monitor *mon, const QDict *qdict)
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 0eb45b0..c86a59d 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -171,7 +171,7 @@ void qbus_free(BusState *bus);
>  
>  void do_info_qtree(Monitor *mon);
>  void do_info_qdm(Monitor *mon);
> -void do_device_add(Monitor *mon, const QDict *qdict);
> +int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  void do_device_del(Monitor *mon, const QDict *qdict);
>  
>  /*** qdev-properties.c ***/
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 641a8ef..e67ab20 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -573,7 +573,8 @@ ETEXI
>          .args_type  = "device:O",
>          .params     = "driver[,prop=value],[,...]",
>          .help       = "add device, like -device on the command line",
> -        .mhandler.cmd = do_device_add,
> +        .user_print = monitor_user_noop,
> +        .mhandler.cmd_new = do_device_add,
>      },
>  
>  STEXI
Markus Armbruster March 1, 2010, 9:25 a.m. UTC | #2 
Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Wed, 24 Feb 2010 18:56:00 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/qdev.c       |   29 +++++++++++++++++++++--------
>>  hw/qdev.h       |    2 +-
>>  qemu-monitor.hx |    3 ++-
>>  3 files changed, 24 insertions(+), 10 deletions(-)
>> 
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index bcd989c..a631492 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -586,7 +586,9 @@ static BusState *qbus_find(const char *path)
>>          dev = qbus_find_dev(bus, elem);
>>          if (!dev) {
>>              qemu_error_new(QERR_DEVICE_NOT_FOUND, elem);
>> -            qbus_list_dev(bus);
>> +            if (!in_qmp_mon()) {
>> +                qbus_list_dev(bus);
>> +            }
>>              return NULL;
>>          }
>>  
>> @@ -605,7 +607,9 @@ static BusState *qbus_find(const char *path)
>>                  return QLIST_FIRST(&dev->child_bus);
>>              default:
>>                  qemu_error_new(QERR_DEVICE_MULTIPLE_BUSSES, elem);
>> -                qbus_list_bus(dev);
>> +                if (!in_qmp_mon()) {
>> +                    qbus_list_bus(dev);
>> +                }
>>                  return NULL;
>>              }
>>          }
>> @@ -619,7 +623,9 @@ static BusState *qbus_find(const char *path)
>>          bus = qbus_find_bus(dev, elem);
>>          if (!bus) {
>>              qemu_error_new(QERR_BUS_NOT_FOUND, elem);
>> -            qbus_list_bus(dev);
>> +            if (!in_qmp_mon()) {
>> +                qbus_list_bus(dev);
>> +            }
>>              return NULL;
>>          }
>>      }
>> @@ -762,16 +768,23 @@ void do_info_qdm(Monitor *mon)
>>      }
>>  }
>>  
>> -void do_device_add(Monitor *mon, const QDict *qdict)
>> +int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>  {
>>      QemuOpts *opts;
>>  
>>      opts = qemu_opts_from_qdict(&qemu_device_opts, qdict);
>> -    if (opts) {
>> -        if (qdev_device_help(opts) || qdev_device_add(opts) == NULL) {
>> -            qemu_opts_del(opts);
>> -        }
>> +    if (!opts) {
>> +        return -1;
>>      }
>> +    if (!in_qmp_mon() && qdev_device_help(opts)) {
>> +        qemu_opts_del(opts);
>> +        return 0;
>> +    }
>> +    if (!qdev_device_add(opts)) {
>> +        qemu_opts_del(opts);
>> +        return -1;
>> +    }
>> +    return 0;
>>  }
>
>  We need at least basic documentation of the accepted parameters and
> an example, just like all others QMP-enabled handlers.

Fair enough.

[...]
diff mbox

Patch

diff --git a/hw/qdev.c b/hw/qdev.c
index bcd989c..a631492 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -586,7 +586,9 @@  static BusState *qbus_find(const char *path)
         dev = qbus_find_dev(bus, elem);
         if (!dev) {
             qemu_error_new(QERR_DEVICE_NOT_FOUND, elem);
-            qbus_list_dev(bus);
+            if (!in_qmp_mon()) {
+                qbus_list_dev(bus);
+            }
             return NULL;
         }
 
@@ -605,7 +607,9 @@  static BusState *qbus_find(const char *path)
                 return QLIST_FIRST(&dev->child_bus);
             default:
                 qemu_error_new(QERR_DEVICE_MULTIPLE_BUSSES, elem);
-                qbus_list_bus(dev);
+                if (!in_qmp_mon()) {
+                    qbus_list_bus(dev);
+                }
                 return NULL;
             }
         }
@@ -619,7 +623,9 @@  static BusState *qbus_find(const char *path)
         bus = qbus_find_bus(dev, elem);
         if (!bus) {
             qemu_error_new(QERR_BUS_NOT_FOUND, elem);
-            qbus_list_bus(dev);
+            if (!in_qmp_mon()) {
+                qbus_list_bus(dev);
+            }
             return NULL;
         }
     }
@@ -762,16 +768,23 @@  void do_info_qdm(Monitor *mon)
     }
 }
 
-void do_device_add(Monitor *mon, const QDict *qdict)
+int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     QemuOpts *opts;
 
     opts = qemu_opts_from_qdict(&qemu_device_opts, qdict);
-    if (opts) {
-        if (qdev_device_help(opts) || qdev_device_add(opts) == NULL) {
-            qemu_opts_del(opts);
-        }
+    if (!opts) {
+        return -1;
     }
+    if (!in_qmp_mon() && qdev_device_help(opts)) {
+        qemu_opts_del(opts);
+        return 0;
+    }
+    if (!qdev_device_add(opts)) {
+        qemu_opts_del(opts);
+        return -1;
+    }
+    return 0;
 }
 
 void do_device_del(Monitor *mon, const QDict *qdict)
diff --git a/hw/qdev.h b/hw/qdev.h
index 0eb45b0..c86a59d 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -171,7 +171,7 @@  void qbus_free(BusState *bus);
 
 void do_info_qtree(Monitor *mon);
 void do_info_qdm(Monitor *mon);
-void do_device_add(Monitor *mon, const QDict *qdict);
+int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 void do_device_del(Monitor *mon, const QDict *qdict);
 
 /*** qdev-properties.c ***/
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 641a8ef..e67ab20 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -573,7 +573,8 @@  ETEXI
         .args_type  = "device:O",
         .params     = "driver[,prop=value],[,...]",
         .help       = "add device, like -device on the command line",
-        .mhandler.cmd = do_device_add,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_device_add,
     },
 
 STEXI