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 = {
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);
+ }
+ }
+
+ 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);
--
2.50.1On 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
© 2016 - 2026 Red Hat, Inc.