src/qemu/qemu_driver.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 12 +++++++++++- tests/qemuhotplugtest.c | 2 +- 3 files changed, 47 insertions(+), 2 deletions(-)
From: Long YunJian <long.yunjian@zte.com.cn>
We detach windows disk with libvirt-python api dom.detachDeviceFlags(xmlstr,3),
but files in windows disk is opened and busy, and libvirt return success.
We found disk not detached actually. so, we close files those opened in windows,
and want to detach again. However, we failed with device not found but disk is
exist actually.
first time detach disk:
a. get vmdef from domain->newDef here:
qemuDomainDetachDeviceLiveAndConfig
virDomainObjCopyPersistentDef
virDomainObjGetPersistentDef
domain->newDef
b. delete disk config here:
qemuDomainDetachDeviceLiveAndConfig
qemuDomainDetachDeviceConfig
virDomainDiskRemoveByName
virDomainDiskRemove
c. update from vmdef to newDef
qemuDomainDetachDeviceLiveAndConfig
virDomainObjAssignDef
domain->newDef = vmdef
second time detach disk:
get vmdef from domain->newDef here:
qemuDomainDetachDeviceLiveAndConfig
virDomainObjCopyPersistentDef
virDomainObjGetPersistentDef
domain->newDef
and then, report device not found here for disk config deleted at the first time:
qemuDomainDetachDeviceLiveAndConfig
qemuDomainDetachDeviceConfig
virDomainDiskRemoveByName
to fix this problem, let detach disk again, we add disk to config if timeout,
and let it deleted in processDeviceDeletedEvent if guestos detach disk successfinnaly.
Signed-off-by: Long YunJian <long.yunjian@zte.com.cn>
---
src/qemu/qemu_driver.c | 35 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_hotplug.c | 12 +++++++++++-
tests/qemuhotplugtest.c | 2 +-
3 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d01f788aea..0ab661251c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3598,6 +3598,9 @@ processDeviceDeletedEvent(virQEMUDriver *driver,
const char *devAlias)
{
virDomainDeviceDef dev;
+ virDomainDiskDef *det_disk = NULL;
+ g_autofree char *disk_dst_name = NULL;
+ bool devicedisk = false;
VIR_DEBUG("Removing device %s from domain %p %s",
devAlias, vm, vm->def->name);
@@ -3616,8 +3619,19 @@ processDeviceDeletedEvent(virQEMUDriver *driver,
if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0)
goto endjob;
+ if (dev.type & VIR_DOMAIN_DEVICE_DISK) {
+ devicedisk = true;
+ disk_dst_name = g_strdup(dev.data.disk->dst);
+ }
+
if (qemuDomainRemoveDevice(driver, vm, &dev) < 0)
goto endjob;
+
+ if (devicedisk && disk_dst_name) {
+ if ((det_disk = virDomainDiskRemoveByName(vm->newDef, disk_dst_name))) {
+ virDomainDiskDefFree(det_disk);
+ }
+ }
}
qemuDomainSaveStatus(vm);
@@ -7491,6 +7505,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver,
g_autoptr(virDomainDeviceDef) dev_live = NULL;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
g_autoptr(virDomainDef) vmdef = NULL;
+ bool timeout = false;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -7530,6 +7545,10 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver,
if ((rc = qemuDomainDetachDeviceLive(vm, dev_live, driver, false)) < 0)
return -1;
+ if (rc == 2) {
+ timeout = true;
+ rc = 0;
+ }
if (rc == 0 && qemuDomainUpdateDeviceList(vm, VIR_ASYNC_JOB_NONE) < 0)
return -1;
@@ -7542,6 +7561,22 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver,
if (virDomainDefSave(vmdef, driver->xmlopt, cfg->configDir) < 0)
return -1;
+ /* if timeout, add device to vmdef, and delete at processDeviceDeletedEvent */
+ if (timeout && (dev_config->type & VIR_DOMAIN_DEVICE_DISK)) {
+ g_autoptr(virDomainDeviceDef) devConf = NULL;
+ unsigned int attach_parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+ VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
+
+ if (!(devConf = virDomainDeviceDefParse(xml, vmdef,
+ driver->xmlopt, priv->qemuCaps,
+ attach_parse_flags)))
+ return -1;
+ if (qemuDomainAttachDeviceConfig(vmdef, devConf, priv->qemuCaps,
+ attach_parse_flags,
+ driver->xmlopt) < 0)
+ return -1;
+ }
+
virDomainObjAssignDef(vm, &vmdef, false, NULL);
/* Event sending if persistent config has changed */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 774962b0df..98b47bfa9c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -6090,6 +6090,13 @@ qemuDomainDetachDeviceLease(virQEMUDriver *driver,
}
+/* Returns:
+ * -1 Unplug of the device failed
+ *
+ * 0 Unplug of the device success
+ *
+ * 2 removal of the device did not finish in qemuDomainRemoveDeviceWaitTime
+ */
int
qemuDomainDetachDeviceLive(virDomainObj *vm,
virDomainDeviceDef *match,
@@ -6297,7 +6304,10 @@ qemuDomainDetachDeviceLive(virDomainObj *vm,
if (async) {
ret = 0;
} else {
- if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
+ ret = qemuDomainWaitForDeviceRemoval(vm);
+ if (ret == 0)
+ ret = 2;
+ if (ret == 1)
ret = qemuDomainRemoveDevice(driver, vm, &detach);
}
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 961a7a3c64..6c1c6112d9 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -238,7 +238,7 @@ testQemuHotplug(const void *data)
case DETACH:
ret = qemuDomainDetachDeviceLive(vm, dev, &driver, false);
- if (ret == 0 || fail)
+ if (ret == 2 || ret == 0 || fail)
ret = testQemuHotplugCheckResult(vm, domain_xml,
domain_filename, fail);
break;
--
2.31.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Fri, Apr 26, 2024 at 12:29:05 +0800, long.yunjian@zte.com.cn wrote: > From: Long YunJian <long.yunjian@zte.com.cn> > > We detach windows disk with libvirt-python api dom.detachDeviceFlags(xmlstr,3), > but files in windows disk is opened and busy, and libvirt return success. > We found disk not detached actually. 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#virDomainDetachDeviceFlags > so, we close files those opened in windows, > and want to detach again. However, we failed with device not found but disk is > exist actually. 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. _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
© 2016 - 2024 Red Hat, Inc.