Re: [libvirt] [PATCH 1/5] Introduce virDomainDetachDeviceAlias API
On 05/22/2018 08:36 AM, Peter Krempa wrote:
On Mon, May 21, 2018 at 18:07:58 +0200, Michal Privoznik wrote:
> When detaching a device it can be uniquely identified by its
> alias. Instead of misusing virDomainDetachDeviceFlags which has
> the same signature introduce new function.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> include/libvirt/libvirt-domain.h | 3 +++
> src/driver-hypervisor.h | 6 ++++++
> src/libvirt-domain.c | 46 ++++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 5 +++++
> 4 files changed, 60 insertions(+)
[...]
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 2d86e48979..b32c1d3064 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -8349,6 +8349,52 @@ virDomainUpdateDeviceFlags(virDomainPtr domain,
> }
>
>
> +/**
> + * virDomainDetachDeviceAlias:
> + * @domain: pointer to domain object
> + * @alias: device alias
> + * @flags: bitwise-OR of virDomainDeviceModifyFlags
> + *
> + * Detach a virtual device from a domain, using the alias to
> + * specify device.
> + *
> + * See virDomainDetachDeviceFlags() for more details.
Since this is a new API I think that we should finally fix the broken
semantics of the old API and return a 'timeout' code rather than
success. Obviously this will make the implementation quite more
challenging, but the old semantics are really broken.
So just to make sure I understand. We would still wait for some time
(just like we are doing now with virDomainDetachDeviceFlags()), but if
the qemu event does not arrive in defined timeout, an error is reported
(whereas virDomainDetachDeviceFlags() returns success).
Well couple of points to make there:
1) I bet there will be somebody wanting us to make the timeout
configurable (bad idea if you ask me),
2) users/mgmt apps still need to implement DEVICE_DELETED event handling.
If it is so, why not make this completely asynchronous and basically
return true/false - "yes we did send request to qemu"? That way we don't
have to care about any timeouts, and this still could be a mere drop in
replacement for virDomainDetachDevice().
Michal