Re: [PATCH] fix detach disk device failed with device not found

On Fri, Apr 26, 2024 at 12:29:05 +0800, long.yunjian(a)zte.com.cn wrote: Note that this is expected and documented behaviour as device unplug requires cooperation with the guest OS: 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. https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDetachDevic... Okay, so reading the code reveals the real problem. You are using _LIVE and _CONFIG flags together, in which case the persistent definition is already removed, thus produces failure. In case your application cares about these advanced scenarios you'll need to handle the update of the presistent definition separately from the update of _CONFIG in the handler for VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED, or implement the retry without the _CONFIG flag if it fails. Reallistically due to backwards compatibility reasons we can't afford to change the behaviour of the device removal in this case. In case you'd be still wanting to get a more sane behaviour of virDomainDetachDeviceFlags which removes the persistent (_CONFIG) definition only after the _LIVE is detached this mode would require a new flag to initiate it. The new mode would then need to record which definition to remove as the actual removal of the _LIVE device can happen at any point in the future, and even can be fully ignored. This also means that the information needed to remove the _CONFIG bit would need to be recorded to the status XML to persist it across libvirtd/virtqemud restarts. Also this would need to be implemented for all devices and not just disks to be an acceptable patch as well as not actually atempt to remove the device and then return it back reconstructed from the XML the user passed as that may be incomplete.