[libvirt] [PATCH v2 10/12] qemu_hotplug: Allow asynchronous detach

[Ján Tomko]

On Thu, May 24, 2018 at 01:13:37PM +0200, Michal Privoznik wrote:
>The virDomainDetachDeviceAlias API is designed so that it only
>sends detach request to qemu. It's user's responsibility to wait

s/user/the user/

>for DEVICE_DELETED event, not libvirt's. Add @async flag to
>qemuDomainDetach*Device() functions so that caller can chose if
>detach is semi-synchronous (old virDomainDetachDeviceFlags()) or
>fully asynchronous (new virDomainDetachDeviceFlags()).
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/qemu/qemu_driver.c  |  32 +++----
> src/qemu/qemu_hotplug.c | 231 ++++++++++++++++++++++++++++++++----------------
> src/qemu/qemu_hotplug.h |  33 ++++---
> tests/qemuhotplugtest.c |  13 +--
> 4 files changed, 203 insertions(+), 106 deletions(-)
>
>@@ -4769,18 +4771,24 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
>     if (qemuDomainObjExitMonitor(driver, vm) < 0)
>         goto cleanup;
>
>-    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
>-        ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
>+    if (async) {
>+        ret = 0;
>+    } else {
>+        if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
>+            ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
>+    }

I'm sure the ret assignments could be reduced here, but that's out of
scope of this series...

Also, most of the code is executed if (!async). It might be nicer to
come up with a variable that can be used in a positive condition,
but it would be less precise, because only the new async API is
reasonably defined - even if (wait) does not seem to outweigh
the loss of clarity.

>
>  cleanup:
>-    qemuDomainResetDeviceRemoval(vm);
>+    if (!async)
>+        qemuDomainResetDeviceRemoval(vm);
>     return ret;
> }
>
> static int
> qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
>                            virDomainObjPtr vm,
>-                           virDomainDiskDefPtr detach)
>+                           virDomainDiskDefPtr detach,
>+                           bool async)
> {
>     int ret = -1;
>     qemuDomainObjPrivatePtr priv = vm->privateData;
>@@ -5336,7 +5375,8 @@ qemuDomainDetachWatchdog(virQEMUDriverPtr driver,
>         return -1;
>     }
>
>-    qemuDomainMarkDeviceForRemoval(vm, &watchdog->info);
>+    if (!async)
>+        qemuDomainMarkDeviceForRemoval(vm, &watchdog->info);
>
>     qemuDomainObjEnterMonitor(driver, vm);
>     if (qemuMonitorDelDevice(priv->mon, watchdog->info.alias) < 0) {

Missing condition around qemuDomainWaitForDeviceRemoval here in
qemuDomainDetachWatchdog. It should only be called if (!async).

With that fixed:

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180526/7f0a2661/attachment-0001.sig>