[libvirt] [PATCH] qemu: hotplug: fail when net device detach times out

[Daniel P. Berrangé]

On Tue, Mar 13, 2018 at 02:07:08PM +0000, Jonathan Davies wrote:
> qemuDomainDetachNetDevice should not treat a timeout in a call to
> qemuDomainWaitForDeviceRemoval as success. Instead, it should treat it
> as failure -- this is the intention behind having a timeout.

Actually this is intentional current behaviour. See the API docs for
virDomainDetachDeviceFlags:

 * Beware that depending on the hypervisor and device type, detaching a device
 * from a running domain may be asynchronous. That is, calling
 * virDomainDetachDeviceFlags may just request device removal while the device
 * is actually removed later (in cooperation with a guest OS). Previously,
 * this fact was ignored and the device could have been removed from domain
 * configuration before it was actually removed by the hypervisor causing
 * various failures on subsequent operations. To check whether the device was
 * successfully removed, either recheck domain configuration using
 * virDomainGetXMLDesc() or add a handler for the VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED
 * event. In case the device is already gone when virDomainDetachDeviceFlags
 * returns, the event is delivered before this API call ends. To help existing
 * clients work better in most cases, this API will try to transform an
 * asynchronous device removal that finishes shortly after the request into
 * a synchronous removal. In other words, this API may wait a bit for the
 * removal to complete in case it was not synchronous.



> 
> If qemuDomainWaitForDeviceRemoval times out, it returns 0. In this
> instance, if qemuDomainDetachNetDevice returns 0, virsh detach-interface
> reports:
> 
>   Interface detached successfully

If anything needs changing it is this message from virsh.

Virsh should not assume return value of 0 means the device has been
detached. Virsh should do what the API docs suggest and check the
XML config to see if the asynchronous detach requets has completed
or not. IOW, it should report either

 Interface detached successfully

or

 Interface detach request still pending



> with a zero exit status.
> 
> But with a negative return value from qemuDomainDetachNetDevice, virsh
> detach-interface reports:
> 
>   error: Failed to detach interface
>   error: An error occurred, but the cause is unknown
> 
> with a non-zero exit status, which is more appropriate.
> 
> Signed-off-by: Jonathan Davies <jonathan.davies at nutanix.com>
> ---
>  src/qemu/qemu_hotplug.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4be0f546c..d9a2f2d4d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -5082,8 +5082,13 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
>      if (qemuDomainObjExitMonitor(driver, vm) < 0)
>          goto cleanup;
>  
> -    if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
> +    ret = qemuDomainWaitForDeviceRemoval(vm);
> +    if (ret == 1) {
>          ret = qemuDomainRemoveNetDevice(driver, vm, detach);
> +    } else if (ret == 0) {
> +        VIR_WARN("Detach of device %s timed out; treating as a failure", detach->ifname);
> +        ret = -1;
> +    }
>  
>   cleanup:
>      qemuDomainResetDeviceRemoval(vm);
> -- 
> 2.14.1
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|