Paolo Bonzini <pbonz...@redhat.com> writes: > When a device is unparented (i.e. made completely hidden from management) > we want to send a DEVICE_DELETED event only if the device actually was > realized. This avoids raising DEVICE_DELETED events when device_add > fails. > > However, this does not work right for recursively-deleted > devices: the whole tree is _first_ unrealized, _then_ unparented. > Then device_unparent sees realized==false and fails to trigger > the event. The solution is simply to move have_realized into > the DeviceState struct. If device_add fails, we never set the > new field to true and DEVICE_DELETED is not sent. > > Fixes qemu-iotests testcase 067.
Suggest to add "Broken in commit 5942a19" here, to make it clear that it's a recent regression. > Reported-by: Markus Armbruster <arm...@redhat.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > hw/core/qdev.c | 5 +++-- > include/hw/qdev-core.h | 1 + > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index d1eba3c..c520415 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -848,6 +848,7 @@ static void device_set_realized(Object *obj, bool value, > Error **errp) if (value && !dev->realized) { [...] > if (dev->hotplugged && local_err == NULL) { > device_reset(dev); > } > + dev->pending_deleted_event = false; Unset on completion of unrealized -> realized transition. > } else if (!value && dev->realized) { > QLIST_FOREACH(bus, &dev->child_bus, sibling) { > object_property_set_bool(OBJECT(bus), false, "realized", > @@ -862,6 +863,7 @@ static void device_set_realized(Object *obj, bool value, > Error **errp) > if (dc->unrealize && local_err == NULL) { > dc->unrealize(dev, &local_err); > } > + dev->pending_deleted_event = true; Set on completion of realized -> unrealized transition. > } > > if (local_err != NULL) { > @@ -972,7 +974,6 @@ static void device_unparent(Object *obj) > { > DeviceState *dev = DEVICE(obj); > BusState *bus; > - bool have_realized = dev->realized; > > if (dev->realized) { > object_property_set_bool(obj, false, "realized", NULL); > @@ -988,7 +989,7 @@ static void device_unparent(Object *obj) > } > > /* Only send event if the device had been completely realized */ > - if (have_realized) { > + if (dev->pending_deleted_event) { > gchar *path = object_get_canonical_path(OBJECT(dev)); > > qapi_event_send_device_deleted(!!dev->id, dev->id, path, > &error_abort); Let's see whether I understand how this works. Please correct misunderstandings. device_unparent() runs right before device deletion, and only then. First thing it does is setting property "realized" to false. Does nothing if the device has never been completely realized. dev->pending_deleted_event remains in its initial state false. DEVICE_DELETED not sent. Good. Else, the device was completely realized at some time. If it is currently realized, we get a transition to unrealized right now, setting dev->pending_deleted_event. Else, the last transition must have been realized -> unrealized, setting dev->pending_deleted_event. Since it gets unset only on unrealized -> realized, it's still set. Therefore, dev->pending_deleted_event is set if and only if the device has been completely realized. > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 9221cfc..0799ff2 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -156,6 +156,7 @@ struct DeviceState { > > const char *id; > bool realized; > + bool pending_deleted_event; > QemuOpts *opts; > int hotplugged; > BusState *parent_bus; Reviewed-by: Markus Armbruster <arm...@redhat.com> (Tested, too, but my r-by subsumes that here)