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

On Thu, Mar 14, 2019 at 13:22:35 +0100, Michal Privoznik wrote: 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.