On 8/28/25 14:54, 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 | 2 +- src/ch/ch_hotplug.c | 131 ++++++++++++++++++++++++++++++++++++++++++-- src/ch/ch_monitor.c | 17 ++++++ src/ch/ch_monitor.h | 4 ++ 4 files changed, 149 insertions(+), 5 deletions(-)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 39f9d934c0..0484201c88 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -2366,7 +2366,7 @@ chDomainAttachDeviceFlags(virDomainPtr dom, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob;
- if (chDomainAttachDeviceLiveAndConfig(vm, driver, xml, flags) < 0) { + if (chDomainAttachDeviceLiveAndUpdateConfig(vm, driver, xml, flags) < 0) {
This should have been squashed to one of previous patches that introduced the incorrect function call.
goto endjob; }
diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c index c46628e7e9..524355b93c 100644 --- a/src/ch/ch_hotplug.c +++ b/src/ch/ch_hotplug.c @@ -18,18 +18,141 @@
#include <config.h>
+#include "ch_alias.h" +#include "ch_domain.h" #include "ch_hotplug.h"
+#include "domain_event.h" +#include "domain_validate.h" +#include "virlog.h" + #define VIR_FROM_THIS VIR_FROM_CH
+VIR_LOG_INIT("ch.ch_hotplug"); + +static int +chDomainAddDisk(virCHMonitor *mon, virDomainObj *vm, virDomainDiskDef *disk) +{ + if (chAssignDeviceDiskAlias(disk) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Assigning disk alias failed")); + return -1; + } + + if (virCHMonitorAddDisk(mon, disk) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Adding disk to domain failed")); + return -1; + } + + virDomainDiskInsert(vm->def, disk); + + return 0; +} + +static int +chDomainAttachDeviceLive(virDomainObj *vm, + virDomainDeviceDef *dev) +{ + int ret = -1; + virCHDomainObjPrivate *priv = vm->privateData; + virCHMonitor *mon = priv->monitor; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: { + if (chDomainAddDisk(mon, vm, dev->data.disk) < 0) { + break; + } + + dev->data.disk = NULL; + ret = 0; + break; + } + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_VSOCK: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_AUDIO: + case VIR_DOMAIN_DEVICE_CRYPTO: + case VIR_DOMAIN_DEVICE_PSTORE: + case VIR_DOMAIN_DEVICE_LAST: + default: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("live attach of device '%1$s' is not supported"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + int -chDomainAttachDeviceLiveAndUpdateConfig(virDomainObj *vm G_GNUC_UNUSED, - virCHDriver *driver G_GNUC_UNUSED, - const char *xml G_GNUC_UNUSED, +chDomainAttachDeviceLiveAndUpdateConfig(virDomainObj *vm, + virCHDriver *driver, + const char *xml, unsigned int flags) { + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; + g_autoptr(virDomainDeviceDef) devLive = NULL; + g_autoptr(virDomainDef) vmdef = NULL; + g_autoptr(virCHDriverConfig) cfg = NULL; + g_autoptr(virDomainDeviceDef) devConf = NULL; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1);
- return -1; + cfg = virCHDriverGetConfig(driver); + + 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 (!(devLive = virDomainDeviceDefParse(xml, vm->def, + driver->xmlopt, NULL, + parse_flags))) { + return -1; + } + + if (virDomainDeviceValidateAliasForHotplug(vm, devLive, + VIR_DOMAIN_AFFECT_LIVE) < 0) + return -1; + + if (virDomainDefCompatibleDevice(vm->def, devLive, NULL, + VIR_DOMAIN_DEVICE_ACTION_ATTACH, + true) < 0) { + return -1; + } + + if (chDomainAttachDeviceLive(vm, devLive) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to add device")); + return -1; + } + } + + return 0; }
I'd break this patch into two. The first one adds ch_monitor and other helper code, the other then implements the public API (rendering the patch that added empty skeleton useless).
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 5f3e2adbee..8968d84a71 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -314,6 +314,23 @@ virCHMonitorBuildDisksJson(virJSONValue *content, virDomainDef *vmdef) return 0; }
+int +virCHMonitorAddDisk(virCHMonitor *monitor, + virDomainDiskDef *diskdef) +{ + g_autofree char *payload = NULL; + g_autoptr(virJSONValue) disks = virJSONValueNewArray(); + g_autoptr(virJSONValue) response = NULL; + + if (virCHMonitorBuildDiskJson(disks, diskdef) < 0) {
I wonder whether it would make sense to make virCHMonitorBuildDiskJson() return a virJSONValue of just one disk instead of appending it into an array. Appending could be then moved into the for() loop inside of virCHMonitorBuildDisksJson(). This way we could avoid creating an array here.
+ return -1; + } + + payload = virJSONValueToString(virJSONValueArrayGet(disks, 0), false); + + return virCHMonitorPut(monitor, URL_VM_ADD_DISK, payload, NULL, NULL); +} + static int virCHMonitorBuildRngJson(virJSONValue *content, virDomainDef *vmdef) { diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h index ffac9e938e..8338059c7c 100644 --- a/src/ch/ch_monitor.h +++ b/src/ch/ch_monitor.h @@ -40,6 +40,7 @@ #define URL_VM_INFO "vm.info" #define URL_VM_SAVE "vm.snapshot" #define URL_VM_RESTORE "vm.restore" +#define URL_VM_ADD_DISK "vm.add-disk"
#define VIRCH_THREAD_NAME_LEN 16
@@ -138,6 +139,9 @@ int virCHMonitorBuildNetJson(virDomainNetDef *netdef, int netindex, char **jsonstr); +int +virCHMonitorAddDisk(virCHMonitor* mon, virDomainDiskDef *diskdef); + int virCHMonitorBuildRestoreJson(virDomainDef *vmdef, const char *from, char **jsonstr);
Michal