diff mbox

[2/2] qapi: convert device_del

Message ID 1333040202-11225-3-git-send-email-lcapitulino@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino March 29, 2012, 4:56 p.m. UTC
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hmp-commands.hx   |    3 +--
 hmp.c             |    9 +++++++++
 hmp.h             |    1 +
 hw/qdev-monitor.c |   18 +++++-------------
 qapi-schema.json  |   20 ++++++++++++++++++++
 qmp-commands.hx   |    5 +----
 6 files changed, 37 insertions(+), 19 deletions(-)

Comments

Eric Blake March 29, 2012, 5:55 p.m. UTC | #1
On 03/29/2012 10:56 AM, Luiz Capitulino wrote:
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  hmp-commands.hx   |    3 +--
>  hmp.c             |    9 +++++++++
>  hmp.h             |    1 +
>  hw/qdev-monitor.c |   18 +++++-------------
>  qapi-schema.json  |   20 ++++++++++++++++++++
>  qmp-commands.hx   |    5 +----
>  6 files changed, 37 insertions(+), 19 deletions(-)

> +# @device_del:
> +#
> +# Remove a device from a guest
> +#
> +# @id: the name of the device
> +#
> +# Returns: Nothing on success
> +#          If @id is not a valid device, DeviceNotFound
> +#          If the device does not support unplug, BusNoHotplug
> +#
> +# Notes: When this command completes, the device may not be removed from the
> +#        guest.  Hot removal is an operation that requires guest cooperation.
> +#        This command merely requests that the guest begin the hot removal
> +#        process.

Nothing against your patch itself, but is there any way we could enhance
things in future patches to actually notify the management app when the
guest has cooperated and the devices is actually removed?  A new event
would be helpful, as well as a way to detect whether the new event
exists and should be waited for.
Luiz Capitulino March 30, 2012, 1:10 p.m. UTC | #2
On Thu, 29 Mar 2012 11:55:31 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 03/29/2012 10:56 AM, Luiz Capitulino wrote:
> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hmp-commands.hx   |    3 +--
> >  hmp.c             |    9 +++++++++
> >  hmp.h             |    1 +
> >  hw/qdev-monitor.c |   18 +++++-------------
> >  qapi-schema.json  |   20 ++++++++++++++++++++
> >  qmp-commands.hx   |    5 +----
> >  6 files changed, 37 insertions(+), 19 deletions(-)
> 
> > +# @device_del:
> > +#
> > +# Remove a device from a guest
> > +#
> > +# @id: the name of the device
> > +#
> > +# Returns: Nothing on success
> > +#          If @id is not a valid device, DeviceNotFound
> > +#          If the device does not support unplug, BusNoHotplug
> > +#
> > +# Notes: When this command completes, the device may not be removed from the
> > +#        guest.  Hot removal is an operation that requires guest cooperation.
> > +#        This command merely requests that the guest begin the hot removal
> > +#        process.
> 
> Nothing against your patch itself, but is there any way we could enhance
> things in future patches to actually notify the management app when the
> guest has cooperated and the devices is actually removed?  A new event
> would be helpful, as well as a way to detect whether the new event
> exists and should be waited for.

To allow to query for the existence of the event, we need to add the
'event' type to the qapi and allow clients to query our schema. Maybe we
could try it for 1.2.

The 'device removed' event could be added now, but I think it's better to
wait for the QOM refactoring because there might be better ways to do it.
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index bd35a3e..a6f5a84 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -622,8 +622,7 @@  ETEXI
         .args_type  = "id:s",
         .params     = "device",
         .help       = "remove device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_device_del,
+        .mhandler.cmd = hmp_device_del,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 9cf2d13..f3e5163 100644
--- a/hmp.c
+++ b/hmp.c
@@ -934,3 +934,12 @@  void hmp_migrate(Monitor *mon, const QDict *qdict)
         qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock));
     }
 }
+
+void hmp_device_del(Monitor *mon, const QDict *qdict)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    Error *err = NULL;
+
+    qmp_device_del(id, &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 8807853..443b812 100644
--- a/hmp.h
+++ b/hmp.h
@@ -60,5 +60,6 @@  void hmp_block_stream(Monitor *mon, const QDict *qdict);
 void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
+void hmp_device_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index e952238..7e3c925 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -19,6 +19,7 @@ 
 
 #include "qdev.h"
 #include "monitor.h"
+#include "qmp-commands.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -574,26 +575,17 @@  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
-int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_device_del(const char *id, Error **errp)
 {
-    const char *id = qdict_get_str(qdict, "id");
-    Error *local_err = NULL;
     DeviceState *dev;
 
     dev = qdev_find_recursive(sysbus_get_default(), id);
     if (NULL == dev) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, id);
-        return -1;
-    }
-
-    qdev_unplug(dev, &local_err);
-    if (error_is_set(&local_err)) {
-        qerror_report_err(local_err);
-        error_free(local_err);
-        return -1;
+        error_set(errp, QERR_DEVICE_NOT_FOUND, id);
+        return;
     }
 
-    return 0;
+    qdev_unplug(dev, errp);
 }
 
 void qdev_machine_init(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index 0d11d6e..ace55f3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1701,3 +1701,23 @@ 
 # Since: 1.1
 ##
 { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
+
+##
+# @device_del:
+#
+# Remove a device from a guest
+#
+# @id: the name of the device
+#
+# Returns: Nothing on success
+#          If @id is not a valid device, DeviceNotFound
+#          If the device does not support unplug, BusNoHotplug
+#
+# Notes: When this command completes, the device may not be removed from the
+#        guest.  Hot removal is an operation that requires guest cooperation.
+#        This command merely requests that the guest begin the hot removal
+#        process.
+#
+# Since: 0.14.0
+##
+{ 'command': 'device_del', 'data': {'id': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c626ba8..c878b54 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -314,10 +314,7 @@  EQMP
     {
         .name       = "device_del",
         .args_type  = "id:s",
-        .params     = "device",
-        .help       = "remove device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_device_del,
+        .mhandler.cmd_new = qmp_marshal_input_device_del,
     },
 
 SQMP