diff mbox series

[v2,2/3] Revert "qdev: Free QemuOpts when the QOM path goes away"

Message ID 20171009170607.4155-3-mdroth@linux.vnet.ibm.com
State New
Headers show
Series qdev/vfio: defer DEVICE_DEL to avoid races with libvirt | expand

Commit Message

Michael Roth Oct. 9, 2017, 5:06 p.m. UTC 
This reverts commit abed886ec60cf239a03515cf0b30fb11fa964c44.

This patch originally addressed an issue where a DEVICE_DELETED
event could be emitted (in device_unparent()) before a Device's
QemuOpts were cleaned up (in device_finalize()), leading to a
"duplicate ID" error if management attempted to immediately add
a device with the same ID in response to the DEVICE_DELETED event.

An alternative will be implemented in a subsequent patch where we
defer the DEVICE_DELETED event until device_finalize(), which would
also prevent the race, so we revert the original fix in preparation.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Eric Auger <eric.auger@redhat.com>
---
 hw/core/qdev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

David Gibson Oct. 10, 2017, 1:42 a.m. UTC | #1 
On Mon, Oct 09, 2017 at 12:06:06PM -0500, Michael Roth wrote:
> This reverts commit abed886ec60cf239a03515cf0b30fb11fa964c44.
> 
> This patch originally addressed an issue where a DEVICE_DELETED
> event could be emitted (in device_unparent()) before a Device's
> QemuOpts were cleaned up (in device_finalize()), leading to a
> "duplicate ID" error if management attempted to immediately add
> a device with the same ID in response to the DEVICE_DELETED event.
> 
> An alternative will be implemented in a subsequent patch where we
> defer the DEVICE_DELETED event until device_finalize(), which would
> also prevent the race, so we revert the original fix in preparation.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Tested-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/core/qdev.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 0542e1879f..f7c66d9bd0 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -1068,6 +1068,7 @@ 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);
> @@ -1117,9 +1118,6 @@ static void device_unparent(Object *obj)
>          g_free(dev->canonical_path);
>          dev->canonical_path = NULL;
>      }
> -
> -    qemu_opts_del(dev->opts);
> -    dev->opts = NULL;
>  }
>  
>  static void device_class_init(ObjectClass *class, void *data)
diff mbox series

Patch

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0542e1879f..f7c66d9bd0 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1068,6 +1068,7 @@  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);
@@ -1117,9 +1118,6 @@  static void device_unparent(Object *obj)
         g_free(dev->canonical_path);
         dev->canonical_path = NULL;
     }
-
-    qemu_opts_del(dev->opts);
-    dev->opts = NULL;
 }
 
 static void device_class_init(ObjectClass *class, void *data)