[libvirt] [PATCH v2 1/5] qemu_hotplug: Introduce and use qemuDomainDeleteDevice

[Peter Krempa]

On Thu, Mar 14, 2019 at 13:22:35 +0100, Michal Privoznik wrote:
> The aim of this function will be to fix return value of
> qemuMonitorDelDevice() in one specific case. But that is yet to
> come. Right now this is nothing but a plain substitution.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 278 +++++++++++++++-------------------------
>  1 file changed, 103 insertions(+), 175 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index f43f80668c..baa4713cf4 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -67,6 +67,44 @@ VIR_LOG_INIT("qemu.qemu_hotplug");
>  unsigned long long qemuDomainRemoveDeviceWaitTime = 1000ull * 5;
>  
>  
> +/**
> + * qemuDomainDeleteDevice:
> + * @vm: domain object
> + * @alias: device to remove
> + * @enterMonitor: whether do EnterMonitor/ExitMonitor too

I'm not persuaded about usefullnes of this argument. 

The ZPCI device which is the only code path which ever sets this to
false is never detached synchronously so vm->privateData->unplug.alias
will never contain it's alias.

This means that the event will never include the ZPCI device and thus we
don't need to actually modify the returned value for it.

In this regard we can also safely ignore code paths where
qemuDomainDetachExtensionDevice is called when hotplug of a device fails
as in that case  vm->privateData->unplug.alias is never set and also the
errors from that function are ignored.

That leaves two calls which are not ignored:

The call in qemuDomainRemoveRNGDevice happens in the 'Remove' handler
which is executed after the device which would set the unplug.alias was
already removed so it's not relevant

The interresting one is in qemuDomainDetachControllerDevice:

Ee call qemuDomainMarkDeviceForRemoval(vm, &detach->info)
and then  qemuDomainDetachExtensionDevice(vm, &detach->info)
Since qemuDomainDetachZPCIDevice prefixes the device alias with 'zpci'
it's a completely different device and thus the waiting thing does not
make sense here as it should not modify the return value for
that as it's a different device.

Note that this still hints to a probable bug in the ZPCI extension
device code. The ZPCI extension device deletion error if the device does
not exist needs to be ignored. Your second patch will help in that, but
it will require more work. It's most probable that nobody will ever
encounter that bug though given the popularity of that platform.

This means that for the ZPCI extension device does not need this
handling as basically any errors should be ignore. Since the
'enterMonitor' flag results into an utter locking mess in the upcomming
commit which is basically useless you should drop
it, make the ZPCI function ignore error if the device is missing and use
qemuMonitorDelDevice rather than qemuDomainDeleteDevice in it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190314/16d660f9/attachment-0001.sig>