Paolo Bonzini <pbonz...@redhat.com> writes:
> Otherwise there is a race where the DEVICE_DELETED event has been sent but
> attempts to reuse the ID will fail.
>
> Reported-by: Michael S. Tsirkin <m...@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
Let's see whether I understand this.
> ---
> hw/core/qdev.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4ab04aa..92bd8bb 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -1203,7 +1203,6 @@ static void device_finalize(Object *obj)
> NamedGPIOList *ngl, *next;
>
> DeviceState *dev = DEVICE(obj);
> - qemu_opts_del(dev->opts);
>
> QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
> QLIST_REMOVE(ngl, node);
> @@ -1251,6 +1250,9 @@ static void device_unparent(Object *obj)
> qapi_event_send_device_deleted(!!dev->id, dev->id, path,
> &error_abort);
DEVICE_DELETED sent here.
> g_free(path);
> }
> +
> + qemu_opts_del(dev->opts);
> + dev->opts = NULL;
> }
>
> static void device_class_init(ObjectClass *class, void *data)
object_finalize_child_property() runs during unplug:
static void object_finalize_child_property(Object *obj, const char *name,
void *opaque)
{
Object *child = opaque;
if (child->class->unparent) {
(child->class->unparent)(child); <--- calls device_unparent()
}
child->parent = NULL;
object_unref(child); <--- calls device_finalize()
}
device_unparent() sends DEVICE_DELETED, but dev->opts gets only deleted
later, in device_finalize. If the client tries to reuse the ID in the
meantime, it fails.
Two remarks:
1. Wouldn't it be cleaner to delete dev-opts *before* sending
DEVICE_DELETED? Like this:
+++ b/hw/core/qdev.c
@@ -1244,6 +1244,9 @@ static void device_unparent(Object *obj)
dev->parent_bus = NULL;
}
+ qemu_opts_del(dev->opts);
+ dev->opts = NULL;
+
/* Only send event if the device had been completely realized */
if (dev->pending_deleted_event) {
gchar *path = object_get_canonical_path(OBJECT(dev));
2. If the device is a block device, then unplugging it also deletes its
backend (ugly wart we keep for backward compatibility; *not* for
blockdev-add, though). This backend also has a QemuOpts. It gets
deleted in drive_info_del(). Just like device_finalize(), it runs
within object_unref(), i.e. after DEVICE_DELETED is sent. Same race,
different ID, or am I missing something?
See also https://bugzilla.redhat.com/show_bug.cgi?id=1256044
