On 9/4/25 14:10, Stefan Kober wrote:
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_driver.c | 42 +++++++++++ src/ch/ch_hotplug.c | 175 ++++++++++++++++++++++++++++++++++++++++++++ src/ch/ch_hotplug.h | 6 ++ 3 files changed, 223 insertions(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 4f4783efb1..760fccba82 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -2387,6 +2387,46 @@ chDomainAttachDevice(virDomainPtr dom, return chDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); }
+static int +chDomainDetachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) +{ + virCHDriver *driver = dom->conn->privateData; + virDomainObj *vm = NULL; + int ret = -1; + + if (!(vm = virCHDomainObjFromDomain(dom))) + goto cleanup; + + if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + goto endjob; + + if (chDomainDetachDeviceLiveAndUpdateConfig(driver, vm, xml, flags) < 0) + goto endjob; + + ret = 0; + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int chDomainDetachDevice(virDomainPtr dom, const char *xml) +{ + return chDomainDetachDeviceFlags(dom, xml, + VIR_DOMAIN_AFFECT_LIVE); +} + /* Function Tables */ static virHypervisorDriver chHypervisorDriver = { .name = "CH", @@ -2450,6 +2490,8 @@ static virHypervisorDriver chHypervisorDriver = { .domainInterfaceAddresses = chDomainInterfaceAddresses, /* 11.0.0 */ .domainAttachDevice = chDomainAttachDevice, /* 11.8.0 */ .domainAttachDeviceFlags = chDomainAttachDeviceFlags, /* 11.8.0 */ + .domainDetachDevice = chDomainDetachDevice, /* 11.8.0 */ + .domainDetachDeviceFlags = chDomainDetachDeviceFlags, /* 11.8.0 */ };
static virConnectDriver chConnectDriver = {
Again, until here the patch is fine and what's below should be moved into a separate patch.
diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c index 524355b93c..95fe1f0f6f 100644 --- a/src/ch/ch_hotplug.c +++ b/src/ch/ch_hotplug.c @@ -156,3 +156,178 @@ chDomainAttachDeviceLiveAndUpdateConfig(virDomainObj *vm,
return 0; } + +static int +chFindDiskId(virDomainDef *def, const char *dst) +{ + size_t i; + + for (i = 0; i < def->ndisks; i++) { + if (STREQ(def->disks[i]->dst, dst)) + return i; + } + + return -1; +} + + +/** + * chDomainFindDisk + * + * Helper function to find a disk device definition of a domain. + * + * Searches through the disk devices of a domain by comparing to 'match' and + * returns any match via the 'detach' out parameter. + */ +static int +chDomainFindDisk(virDomainObj *vm, + virDomainDiskDef *match, + virDomainDiskDef **detach) +{ + virDomainDiskDef *disk; + int idx; + + if ((idx = chFindDiskId(vm->def, match->dst)) < 0) { + virReportError(VIR_ERR_DEVICE_MISSING, + _("disk %1$s not found"), match->dst); + return -1; + } + *detach = disk = vm->def->disks[idx]; + + return 0; +} + +static int +chDomainDetachDeviceLive(virDomainObj *vm, + virDomainDeviceDef *match) +{ + virDomainDeviceDef detach = { .type = match->type }; + virDomainDeviceInfo *info = NULL; + virCHDomainObjPrivate *priv = vm->privateData; + int idx = 0; + + switch (match->type) { + case VIR_DOMAIN_DEVICE_DISK: + if (chDomainFindDisk(vm, match->data.disk, + &detach.data.disk) < 0) { + return -1; + } + break; + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_VSOCK: + case VIR_DOMAIN_DEVICE_AUDIO: + case VIR_DOMAIN_DEVICE_CRYPTO: + case VIR_DOMAIN_DEVICE_PSTORE: + case VIR_DOMAIN_DEVICE_LAST: + case VIR_DOMAIN_DEVICE_NONE: + default: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("live detach of device '%1$s' is not supported"), + virDomainDeviceTypeToString(match->type)); + return -1; + } + + /* "detach" now points to the actual device we want to detach */ + + if (!(info = virDomainDeviceGetInfo(&detach))) { + /* + * This should never happen, since all of the device types in + * the switch cases that end with a "break" instead of a + * return have a virDeviceInfo in them. + */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("device of type '%1$s' has no device info"), + virDomainDeviceTypeToString(detach.type)); + return -1; + } + + /* Make generic validation checks common to all device types */ + + if (!info->alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot detach %1$s device with no alias"), + virDomainDeviceTypeToString(detach.type)); + return -1; + } + + if (virCHMonitorRemoveDevice(priv->monitor, info->alias) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid response from CH. Disk removal failed.")); + return -1; + } + + if (match->type == VIR_DOMAIN_DEVICE_DISK) { + idx = chFindDiskId(vm->def, match->data.disk->dst); + if (idx >= 0) { + virDomainDiskRemove(vm->def, idx); + } + }
Now, this little if() bothered me for a while. I think we need some function, like chDomainDeviceRemove() with proper switch() over all device types.
+ + return 0; +} + +int +chDomainDetachDeviceLiveAndUpdateConfig(virCHDriver *driver, + virDomainObj *vm, + const char *xml, + unsigned int flags) +{ + g_autoptr(virCHDriverConfig) cfg = NULL; + g_autoptr(virDomainDeviceDef) dev_config = NULL; + g_autoptr(virDomainDeviceDef) dev_live = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; + g_autoptr(virDomainDef) vmdef = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + cfg = virCHDriverGetConfig(driver); + + if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && + !(flags & VIR_DOMAIN_AFFECT_LIVE)) + parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Persistent domain state changes are not supported")); + return -1; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!(dev_live = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt, + NULL, parse_flags))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not parse domain definition")); + return -1; + } + + if (chDomainDetachDeviceLive(vm, dev_live) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could detach device")); + return -1; + } + } + + return 0; +} diff --git a/src/ch/ch_hotplug.h b/src/ch/ch_hotplug.h index 04915ba5de..4a9b9b3b3e 100644 --- a/src/ch/ch_hotplug.h +++ b/src/ch/ch_hotplug.h @@ -25,3 +25,9 @@ chDomainAttachDeviceLiveAndUpdateConfig(virDomainObj *vm, virCHDriver *driver, const char *xml, unsigned int flags); + +int +chDomainDetachDeviceLiveAndUpdateConfig(virCHDriver *driver, + virDomainObj *vm, + const char *xml, + unsigned int flags);
Michal