Qemu reply to libvirt "DeviceNotFound" and libvirt didn't clean on the
side the live configuration.

qemuMonitorDelDevice() return -2 to qemuDomainDeleteDevice() and during
this action in qemuDomainDetachDeviceLive() the remove is never call.

Ref #359

Signed-off-by: Pierre LIBEAU <pierre.libeau@corp.ovh.com>
---
 src/qemu/qemu_hotplug.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9b508dc8f0..52a14a4476 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -93,6 +93,8 @@ qemuDomainResetDeviceRemoval(virDomainObj *vm);
  *
  * Returns: 0 on success,
  *         -1 otherwise.
+ *         -2 device does not exist in qemu, but it still
+ *            exists in libvirt
  */
 static int
 qemuDomainDeleteDevice(virDomainObj *vm,
@@ -124,7 +126,6 @@ qemuDomainDeleteDevice(virDomainObj *vm,
              * domain XML is queried right after detach API the
              * device would still be there.  */
             VIR_DEBUG("Detaching of device %s failed and no event arrived", alias);
-            rc = 0;
         }
     }
 
@@ -6055,7 +6056,11 @@ qemuDomainDetachDeviceLive(virDomainObj *vm,
     if (!async)
         qemuDomainMarkDeviceForRemoval(vm, info);
 
-    if (qemuDomainDeleteDevice(vm, info->alias) < 0) {
+    int rc;
+    rc = qemuDomainDeleteDevice(vm, info->alias);
+    if (rc < 0) {
+        if (rc == -2)
+	        ret = qemuDomainRemoveDevice(driver, vm, &detach);
         if (virDomainObjIsActive(vm))
             qemuDomainRemoveAuditDevice(vm, &detach, false);
         goto cleanup;
-- 
2.37.3

On 8/31/22 14:22, Pierre LIBEAU wrote:
> Qemu reply to libvirt "DeviceNotFound" and libvirt didn't clean on the
> side the live configuration.
> 
> qemuMonitorDelDevice() return -2 to qemuDomainDeleteDevice() and during
> this action in qemuDomainDetachDeviceLive() the remove is never call.
> 
> Ref #359

We tend to put 'Resolves:' with full URL so that it's directly clickable
when viewing the git log.

> 
> Signed-off-by: Pierre LIBEAU <pierre.libeau@corp.ovh.com>
> ---
>  src/qemu/qemu_hotplug.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 9b508dc8f0..52a14a4476 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -93,6 +93,8 @@ qemuDomainResetDeviceRemoval(virDomainObj *vm);
>   *
>   * Returns: 0 on success,
>   *         -1 otherwise.
> + *         -2 device does not exist in qemu, but it still
> + *            exists in libvirt
>   */
>  static int
>  qemuDomainDeleteDevice(virDomainObj *vm,
> @@ -124,7 +126,6 @@ qemuDomainDeleteDevice(virDomainObj *vm,
>               * domain XML is queried right after detach API the
>               * device would still be there.  */
>              VIR_DEBUG("Detaching of device %s failed and no event arrived", alias);
> -            rc = 0;
>          }
>      }
>  
> @@ -6055,7 +6056,11 @@ qemuDomainDetachDeviceLive(virDomainObj *vm,
>      if (!async)
>          qemuDomainMarkDeviceForRemoval(vm, info);
>  
> -    if (qemuDomainDeleteDevice(vm, info->alias) < 0) {
> +    int rc;

We don't really allow declarations in the middle of a block.

> +    rc = qemuDomainDeleteDevice(vm, info->alias);
> +    if (rc < 0) {
> +        if (rc == -2)
> +	        ret = qemuDomainRemoveDevice(driver, vm, &detach);

Oops a TAB sneaked in.

>          if (virDomainObjIsActive(vm))
>              qemuDomainRemoveAuditDevice(vm, &detach, false);
>          goto cleanup;


Fixed up all the small nits and pushed.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Hello guys,


I will very happy if you have time to give me feedback about this patch.

It's the first time for me about libvirt project so don't hesitate to say me if I have missed something.


I have pushed this change on my side internally and it's resolved my issue.


Pierre
________________________________
De : Pierre Libeau
Envoyé : mercredi 31 août 2022 14:22:51
À : libvir-list@redhat.com
Cc : Pierre Libeau
Objet : [PATCH] Fix race condition when detaching a device

Qemu reply to libvirt "DeviceNotFound" and libvirt didn't clean on the
side the live configuration.

qemuMonitorDelDevice() return -2 to qemuDomainDeleteDevice() and during
this action in qemuDomainDetachDeviceLive() the remove is never call.

Ref #359

Signed-off-by: Pierre LIBEAU <pierre.libeau@corp.ovh.com>
---
 src/qemu/qemu_hotplug.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9b508dc8f0..52a14a4476 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -93,6 +93,8 @@ qemuDomainResetDeviceRemoval(virDomainObj *vm);
  *
  * Returns: 0 on success,
  *         -1 otherwise.
+ *         -2 device does not exist in qemu, but it still
+ *            exists in libvirt
  */
 static int
 qemuDomainDeleteDevice(virDomainObj *vm,
@@ -124,7 +126,6 @@ qemuDomainDeleteDevice(virDomainObj *vm,
              * domain XML is queried right after detach API the
              * device would still be there.  */
             VIR_DEBUG("Detaching of device %s failed and no event arrived", alias);
-            rc = 0;
         }
     }

@@ -6055,7 +6056,11 @@ qemuDomainDetachDeviceLive(virDomainObj *vm,
     if (!async)
         qemuDomainMarkDeviceForRemoval(vm, info);

-    if (qemuDomainDeleteDevice(vm, info->alias) < 0) {
+    int rc;
+    rc = qemuDomainDeleteDevice(vm, info->alias);
+    if (rc < 0) {
+        if (rc == -2)
+               ret = qemuDomainRemoveDevice(driver, vm, &detach);
         if (virDomainObjIsActive(vm))
             qemuDomainRemoveAuditDevice(vm, &detach, false);
         goto cleanup;
--
2.37.3