| Message ID | 60194bbd0a9d9259beb9a14e19c1706bd9a07e97.1363196545.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
"Michael S. Tsirkin" <mst@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@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. > + > +{ "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? > } > > 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"?
On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" <mst@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@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. > > > + > > +{ "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? > > > } > > > > 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"? Just this. Try this: rmmod acpiphp in guest, then: device_del system_reset and see the device disappear even though it was not acked by guest.
On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" <mst@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@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"?
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote: >> "Michael S. Tsirkin" <mst@redhat.com> writes: [...] >> > 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"? > > Just this. Try this: rmmod acpiphp in guest, then: > > device_del > system_reset > > and see the device disappear even though it was not acked by guest. Cool, I didn't know that. Just to make sure: does this automatic removal completion send DEVICE_DELETED events?
On Thu, Mar 14, 2013 at 01:13:54PM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote: > >> "Michael S. Tsirkin" <mst@redhat.com> writes: > [...] > >> > 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"? > > > > Just this. Try this: rmmod acpiphp in guest, then: > > > > device_del > > system_reset > > > > and see the device disappear even though it was not acked by guest. > > Cool, I didn't know that. > > Just to make sure: does this automatic removal completion send > DEVICE_DELETED events? With my patch, it does.
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Thu, Mar 14, 2013 at 09:06:15AM +0100, Markus Armbruster wrote: >> "Michael S. Tsirkin" <mst@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@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? Donning my downstream hat for a minute. Upstream after this series is fully applied: * event always has data * data always has member path * data may have member device Downstream after backport of just 1/3: * event may have data * data always has member device Alternative downstream: event always has data, data * event always has data * data may have member device Smaller difference to upstream. Would libvirt care? >> > + >> > +{ "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? It doesn't matter upstream, because the anomaly created by 1/3 gets removed by 3/3. It matters downstream if 1/3 gets backported without 3/3. I will not withhold my upstream ACK because of such downstream concerns. [...]
On 03/14/2013 06:22 AM, Michael S. Tsirkin wrote: >>> Just this. Try this: rmmod acpiphp in guest, then: >>> >>> device_del >>> system_reset >>> >>> and see the device disappear even though it was not acked by guest. >> >> Cool, I didn't know that. >> >> Just to make sure: does this automatic removal completion send >> DEVICE_DELETED events? > > With my patch, it does. Good. That is the best behavior, if libvirt is going to start relying on the event as a means to avoid polling on all but libvirtd restarts.
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) + +{ "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); + } } 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 ##
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@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(-)