3:53 a.m.
On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote:
"Michael S. Tsirkin" <mst(a)redhat.com> writes:
> libvirt has a long-standing bug: when removing the device,
> it can request removal but does not know when the
> removal completes. Add an event so we can fix this in a robust way.
>
> Signed-off-by: Michael S. Tsirkin <mst(a)redhat.com>
> ---
> QMP/qmp-events.txt | 16 ++++++++++++++++
> hw/qdev.c | 12 ++++++++++++
> include/monitor/monitor.h | 1 +
> monitor.c | 1 +
> qapi-schema.json | 4 +++-
> 5 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index b2698e4..24cf3e8 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -136,6 +136,22 @@ Example:
> Note: The "ready to complete" status is always reset by a
BLOCK_JOB_ERROR
> event.
>
> +DEVICE_DELETED
> +-----------------
> +
> +Emitted whenever the device removal completion is acknowledged
> +by the guest.
> +At this point, it's safe to reuse the specified device ID.
> +Device removal can be initiated by the guest or by HMP/QMP commands.
> +
> +Data:
> +
> +- "device": device name (json-string, optional)
If there are no members present, do we get an event without member
"data", or do we get one with an empty member?
{ "event": "DEVICE_DELETED",
"timestamp": { "seconds": 1265044230, "microseconds":
450486 } }
{ "event": "DEVICE_DELETED",
"data": { },
"timestamp": { "seconds": 1265044230, "microseconds":
450486 } }
There's no precedence, as this is the first event with data where all
data members are optional.
We get one without data. We currently have events with data and some
members, and events without data, but not events with an empty data,
I didn't see a need to invent one with an empty data. You?
> +
> +{ "event": "DEVICE_DELETED",
> + "data": { "device": "virtio-net-pci-0" },
> + "timestamp": { "seconds": 1265044230,
"microseconds": 450486 } }
> +
> DEVICE_TRAY_MOVED
> -----------------
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 689cd54..bebc44d 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -29,6 +29,7 @@
> #include "sysemu/sysemu.h"
> #include "qapi/error.h"
> #include "qapi/visitor.h"
> +#include "qapi/qmp/qjson.h"
>
> int qdev_hotplug = 0;
> static bool qdev_hot_added = false;
> @@ -760,6 +761,7 @@ static void device_unparent(Object *obj)
> DeviceState *dev = DEVICE(obj);
> DeviceClass *dc = DEVICE_GET_CLASS(dev);
> BusState *bus;
> + QObject *event_data;
>
> while (dev->num_child_bus) {
> bus = QLIST_FIRST(&dev->child_bus);
> @@ -778,6 +780,16 @@ static void device_unparent(Object *obj)
> object_unref(OBJECT(dev->parent_bus));
> dev->parent_bus = NULL;
> }
> +
> + if (dev->id) {
> + event_data = qobject_from_jsonf("{ 'device': %s }",
dev->id);
> + } else {
> + event_data = NULL;
> + }
> + monitor_protocol_event(QEVENT_DEVICE_DELETED, event_data);
> + if (event_data) {
> + qobject_decref(event_data);
> + }
You make this unconditional in 3/3. Actually, unconditional should work
just fine even here. No need to respin just for that.
Answering my doc question: we get an event without member "data".
Is that what we want?
Does it matter?
> }
>
> static void device_class_init(ObjectClass *class, void *data)
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 87fb49c..b868760 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -39,6 +39,7 @@ typedef enum MonitorEvent {
> QEVENT_BLOCK_JOB_CANCELLED,
> QEVENT_BLOCK_JOB_ERROR,
> QEVENT_BLOCK_JOB_READY,
> + QEVENT_DEVICE_DELETED,
> QEVENT_DEVICE_TRAY_MOVED,
> QEVENT_SUSPEND,
> QEVENT_SUSPEND_DISK,
> diff --git a/monitor.c b/monitor.c
> index 32a6e74..2a5e7b6 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -457,6 +457,7 @@ static const char *monitor_event_names[] = {
> [QEVENT_BLOCK_JOB_CANCELLED] = "BLOCK_JOB_CANCELLED",
> [QEVENT_BLOCK_JOB_ERROR] = "BLOCK_JOB_ERROR",
> [QEVENT_BLOCK_JOB_READY] = "BLOCK_JOB_READY",
> + [QEVENT_DEVICE_DELETED] = "DEVICE_DELETED",
> [QEVENT_DEVICE_TRAY_MOVED] = "DEVICE_TRAY_MOVED",
> [QEVENT_SUSPEND] = "SUSPEND",
> [QEVENT_SUSPEND_DISK] = "SUSPEND_DISK",
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 28b070f..bb361e1 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2354,7 +2354,9 @@
> # 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.
> +# process. Completion of the device removal process is signaled with a
> +# DEVICE_DELETED event. Guest reset will automatically complete removal
> +# for all devices.
> #
> # Since: 0.14.0
> ##
What do you mean by "Guest reset will automatically complete removal for
all devices"?