* [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event
@ 2013-03-06 16:49 Michael S. Tsirkin
2013-03-06 16:52 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2013-03-06 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Anthony Liguori, Eduardo Habkost, Stefan Hajnoczi,
Markus Armbruster, Luiz Capitulino, Gerd Hoffmann, laine,
Paolo Bonzini, Andreas Färber
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>
---
Changes from v1:
- move to device_unparent
- address comments by Andreas and Eric
Andreas also suggested a more generic object-deleted event,
I'm not sure how useful that is so let's add what we already need, for
devices with an id and wait and see what's necessary for non-device
objects?
QMP/qmp-events.txt | 15 +++++++++++++++
hw/qdev.c | 7 +++++++
include/monitor/monitor.h | 1 +
monitor.c | 1 +
qapi-schema.json | 4 +++-
5 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index b2698e4..f2f115a 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -136,6 +136,21 @@ 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)
+
+{ "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..d603f4f 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;
@@ -761,6 +762,12 @@ static void device_unparent(Object *obj)
DeviceClass *dc = DEVICE_GET_CLASS(dev);
BusState *bus;
+ if (dev->id) {
+ QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id);
+ monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
+ qobject_decref(data);
+ }
+
while (dev->num_child_bus) {
bus = QLIST_FIRST(&dev->child_bus);
qbus_free(bus);
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
##
--
MST
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event
2013-03-06 16:49 [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event Michael S. Tsirkin
@ 2013-03-06 16:52 ` Paolo Bonzini
2013-03-06 16:55 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2013-03-06 16:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, Anthony Liguori, Eduardo Habkost, Stefan Hajnoczi,
Markus Armbruster, qemu-devel, Gerd Hoffmann, laine,
Luiz Capitulino, Andreas Färber
Il 06/03/2013 17:49, Michael S. Tsirkin ha scritto:
> 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>
> ---
>
> Changes from v1:
> - move to device_unparent
> - address comments by Andreas and Eric
>
> Andreas also suggested a more generic object-deleted event,
> I'm not sure how useful that is so let's add what we already need, for
> devices with an id and wait and see what's necessary for non-device
> objects?
>
> QMP/qmp-events.txt | 15 +++++++++++++++
> hw/qdev.c | 7 +++++++
> include/monitor/monitor.h | 1 +
> monitor.c | 1 +
> qapi-schema.json | 4 +++-
> 5 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index b2698e4..f2f115a 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -136,6 +136,21 @@ 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)
> +
> +{ "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..d603f4f 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;
> @@ -761,6 +762,12 @@ static void device_unparent(Object *obj)
> DeviceClass *dc = DEVICE_GET_CLASS(dev);
> BusState *bus;
>
> + if (dev->id) {
> + QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id);
> + monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
> + qobject_decref(data);
> + }
Do this at the end of device_unparent, so that parents are reported
after their children.
Paolo
> while (dev->num_child_bus) {
> bus = QLIST_FIRST(&dev->child_bus);
> qbus_free(bus);
> 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
> ##
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event
2013-03-06 16:52 ` Paolo Bonzini
@ 2013-03-06 16:55 ` Michael S. Tsirkin
2013-03-06 16:57 ` Paolo Bonzini
2013-03-06 17:03 ` Andreas Färber
0 siblings, 2 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2013-03-06 16:55 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, Anthony Liguori, Eduardo Habkost, Stefan Hajnoczi,
Markus Armbruster, qemu-devel, Gerd Hoffmann, laine,
Luiz Capitulino, Andreas Färber
On Wed, Mar 06, 2013 at 05:52:05PM +0100, Paolo Bonzini wrote:
> Il 06/03/2013 17:49, Michael S. Tsirkin ha scritto:
> > 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>
> > ---
> >
> > Changes from v1:
> > - move to device_unparent
> > - address comments by Andreas and Eric
> >
> > Andreas also suggested a more generic object-deleted event,
> > I'm not sure how useful that is so let's add what we already need, for
> > devices with an id and wait and see what's necessary for non-device
> > objects?
> >
> > QMP/qmp-events.txt | 15 +++++++++++++++
> > hw/qdev.c | 7 +++++++
> > include/monitor/monitor.h | 1 +
> > monitor.c | 1 +
> > qapi-schema.json | 4 +++-
> > 5 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> > index b2698e4..f2f115a 100644
> > --- a/QMP/qmp-events.txt
> > +++ b/QMP/qmp-events.txt
> > @@ -136,6 +136,21 @@ 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)
> > +
> > +{ "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..d603f4f 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;
> > @@ -761,6 +762,12 @@ static void device_unparent(Object *obj)
> > DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > BusState *bus;
> >
> > + if (dev->id) {
> > + QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id);
> > + monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
> > + qobject_decref(data);
> > + }
>
> Do this at the end of device_unparent, so that parents are reported
> after their children.
>
> Paolo
Hmm yes it seems cleaner, though we'd need to copy the
id as the device can go away.
Doing this after
while (dev->num_child_bus) {
bus = QLIST_FIRST(&dev->child_bus);
qbus_free(bus);
}
would be enough, isn't it?
> > while (dev->num_child_bus) {
> > bus = QLIST_FIRST(&dev->child_bus);
> > qbus_free(bus);
> > 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
> > ##
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event
2013-03-06 16:55 ` Michael S. Tsirkin
@ 2013-03-06 16:57 ` Paolo Bonzini
2013-03-06 17:03 ` Andreas Färber
1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2013-03-06 16:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, Anthony Liguori, Eduardo Habkost, Stefan Hajnoczi,
Markus Armbruster, qemu-devel, Gerd Hoffmann, laine,
Luiz Capitulino, Andreas Färber
Il 06/03/2013 17:55, Michael S. Tsirkin ha scritto:
> On Wed, Mar 06, 2013 at 05:52:05PM +0100, Paolo Bonzini wrote:
>> Il 06/03/2013 17:49, Michael S. Tsirkin ha scritto:
>>> 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>
>>> ---
>>>
>>> Changes from v1:
>>> - move to device_unparent
>>> - address comments by Andreas and Eric
>>>
>>> Andreas also suggested a more generic object-deleted event,
>>> I'm not sure how useful that is so let's add what we already need, for
>>> devices with an id and wait and see what's necessary for non-device
>>> objects?
>>>
>>> QMP/qmp-events.txt | 15 +++++++++++++++
>>> hw/qdev.c | 7 +++++++
>>> include/monitor/monitor.h | 1 +
>>> monitor.c | 1 +
>>> qapi-schema.json | 4 +++-
>>> 5 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
>>> index b2698e4..f2f115a 100644
>>> --- a/QMP/qmp-events.txt
>>> +++ b/QMP/qmp-events.txt
>>> @@ -136,6 +136,21 @@ 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)
>>> +
>>> +{ "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..d603f4f 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;
>>> @@ -761,6 +762,12 @@ static void device_unparent(Object *obj)
>>> DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>> BusState *bus;
>>>
>>> + if (dev->id) {
>>> + QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id);
>>> + monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
>>> + qobject_decref(data);
>>> + }
>>
>> Do this at the end of device_unparent, so that parents are reported
>> after their children.
>>
>> Paolo
>
> Hmm yes it seems cleaner, though we'd need to copy the
> id as the device can go away.
>
> Doing this after
> while (dev->num_child_bus) {
> bus = QLIST_FIRST(&dev->child_bus);
> qbus_free(bus);
> }
> would be enough, isn't it?
Yes, but don't worry - the device cannot go away, it is protected while
device_unparent is called (see object_unparent in qom/object.c). So put
it wherever you prefer.
Paolo
>>> while (dev->num_child_bus) {
>>> bus = QLIST_FIRST(&dev->child_bus);
>>> qbus_free(bus);
>>> 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
>>> ##
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event
2013-03-06 16:55 ` Michael S. Tsirkin
2013-03-06 16:57 ` Paolo Bonzini
@ 2013-03-06 17:03 ` Andreas Färber
1 sibling, 0 replies; 5+ messages in thread
From: Andreas Färber @ 2013-03-06 17:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, Anthony Liguori, Eduardo Habkost, Stefan Hajnoczi,
qemu-devel, Markus Armbruster, Gerd Hoffmann, laine,
Paolo Bonzini, Luiz Capitulino
Am 06.03.2013 17:55, schrieb Michael S. Tsirkin:
> On Wed, Mar 06, 2013 at 05:52:05PM +0100, Paolo Bonzini wrote:
>> Il 06/03/2013 17:49, Michael S. Tsirkin ha scritto:
>>> 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>
>>> ---
>>>
>>> Changes from v1:
>>> - move to device_unparent
>>> - address comments by Andreas and Eric
>>>
>>> Andreas also suggested a more generic object-deleted event,
>>> I'm not sure how useful that is so let's add what we already need, for
>>> devices with an id and wait and see what's necessary for non-device
>>> objects?
Fine with me, just wanted to bring that up for consideration since it
impacts not only where we call it but also what data we can provide.
>>> diff --git a/hw/qdev.c b/hw/qdev.c
>>> index 689cd54..d603f4f 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;
>>> @@ -761,6 +762,12 @@ static void device_unparent(Object *obj)
>>> DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>> BusState *bus;
>>>
>>> + if (dev->id) {
>>> + QObject *data = qobject_from_jsonf("{ 'device': %s }", dev->id);
>>> + monitor_protocol_event(QEVENT_DEVICE_DELETED, data);
>>> + qobject_decref(data);
>>> + }
>>
>> Do this at the end of device_unparent, so that parents are reported
>> after their children.
>>
>> Paolo
>
> Hmm yes it seems cleaner, though we'd need to copy the
> id as the device can go away.
>
> Doing this after
> while (dev->num_child_bus) {
> bus = QLIST_FIRST(&dev->child_bus);
> qbus_free(bus);
> }
> would be enough, isn't it?
I had implicitly suggested after dev->realized and before
dev->parent_bus. The reason being that what is inside the if
(dev->realized) {} block actually still needs to be moved into an
unrealize function, and realized = false might still do device-specific
cleanups that could consume time. I'll try to cook something up tonight,
shouldn't collide with your change.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-03-06 17:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-06 16:49 [Qemu-devel] [PATCHv2] qdev: DEVICE_DELETED event Michael S. Tsirkin
2013-03-06 16:52 ` Paolo Bonzini
2013-03-06 16:55 ` Michael S. Tsirkin
2013-03-06 16:57 ` Paolo Bonzini
2013-03-06 17:03 ` Andreas Färber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).