[PATCH v2 00/13] CH: Add disk hotplug support to Cloud Hypervisor domains

This patchset adds the necessary functionality to support disk hotplugging in the CH driver. Some alias handling that can be extended to other device types is added, in order to allow detaching the devices via the Cloud Hypervisor API. Stefan Kober (13): ch: add DomainAttachDevice skeletons ch: add DomainDetachDevice skeletons ch: add ch_hotplug.{h,c} files to CH build ch: call into ch_hotplug implementation ch: pass disk alias to CHV ch: add ch_alias.{c,h} for device alias handling ch: add virCHMonitorPut function ch: use MonitorPut in MonitorPutNoContent ch: add disk attach functionality ch: assign aliases in ProcessPrepareDomain ch: Add virCHMonitorRemoveDevice function ch: implement disk device hot removal NEWS: announce disk hotplug support for ch NEWS.rst | 5 + po/POTFILES | 1 + src/ch/ch_alias.c | 59 ++++++++ src/ch/ch_alias.h | 27 ++++ src/ch/ch_driver.c | 86 ++++++++++++ src/ch/ch_hotplug.c | 333 ++++++++++++++++++++++++++++++++++++++++++++ src/ch/ch_hotplug.h | 33 +++++ src/ch/ch_monitor.c | 81 +++++++++-- src/ch/ch_monitor.h | 7 + src/ch/ch_process.c | 4 + src/ch/meson.build | 4 + 11 files changed, 630 insertions(+), 10 deletions(-) create mode 100644 src/ch/ch_alias.c create mode 100644 src/ch/ch_alias.h create mode 100644 src/ch/ch_hotplug.c create mode 100644 src/ch/ch_hotplug.h -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_driver.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index cf6874f22e..7e8f73564e 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -2344,6 +2344,40 @@ chDomainInterfaceAddresses(virDomain *dom, return ret; } +static int +chDomainAttachDeviceFlags(virDomainPtr dom, + const char *xml G_GNUC_UNUSED, + unsigned int flags) +{ + virDomainObj *vm = NULL; + int ret = -1; + + if (!(vm = virCHDomainObjFromDomain(dom))) + goto cleanup; + + if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + goto endjob; + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + +static int +chDomainAttachDevice(virDomainPtr dom, + const char *xml) +{ + return chDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); +} /* Function Tables */ static virHypervisorDriver chHypervisorDriver = { @@ -2406,6 +2440,9 @@ static virHypervisorDriver chHypervisorDriver = { .connectDomainEventRegisterAny = chConnectDomainEventRegisterAny, /* 10.10.0 */ .connectDomainEventDeregisterAny = chConnectDomainEventDeregisterAny, /* 10.10.0 */ .domainInterfaceAddresses = chDomainInterfaceAddresses, /* 11.0.0 */ + .domainAttachDevice = chDomainAttachDevice, /* 11.8.0 */ + .domainAttachDeviceFlags = chDomainAttachDeviceFlags, /* 11.8.0 */ + }; static virConnectDriver chConnectDriver = { -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_driver.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 7e8f73564e..e5e1bfd7d1 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -2379,6 +2379,40 @@ chDomainAttachDevice(virDomainPtr dom, return chDomainAttachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); } +static int +chDomainDetachDeviceFlags(virDomainPtr dom, + const char *xml G_GNUC_UNUSED, + unsigned int flags) +{ + 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; + + 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", @@ -2442,7 +2476,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 = { -- 2.50.1

The files are meant to contain all device hotplug related code. The first implementation will be live storage attach and detach. On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_hotplug.c | 35 +++++++++++++++++++++++++++++++++++ src/ch/ch_hotplug.h | 27 +++++++++++++++++++++++++++ src/ch/meson.build | 2 ++ 3 files changed, 64 insertions(+) create mode 100644 src/ch/ch_hotplug.c create mode 100644 src/ch/ch_hotplug.h diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c new file mode 100644 index 0000000000..c46628e7e9 --- /dev/null +++ b/src/ch/ch_hotplug.c @@ -0,0 +1,35 @@ +/* + * ch_hotplug.c: CH device hotplug handling + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "ch_hotplug.h" + +#define VIR_FROM_THIS VIR_FROM_CH + +int +chDomainAttachDeviceLiveAndUpdateConfig(virDomainObj *vm G_GNUC_UNUSED, + virCHDriver *driver G_GNUC_UNUSED, + const char *xml G_GNUC_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + return -1; +} diff --git a/src/ch/ch_hotplug.h b/src/ch/ch_hotplug.h new file mode 100644 index 0000000000..04915ba5de --- /dev/null +++ b/src/ch/ch_hotplug.h @@ -0,0 +1,27 @@ +/* + * ch_hotplug.h: CH device hotplug handling + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "ch_conf.h" + +int +chDomainAttachDeviceLiveAndUpdateConfig(virDomainObj *vm, + virCHDriver *driver, + const char *xml, + unsigned int flags); diff --git a/src/ch/meson.build b/src/ch/meson.build index cd20c3d065..bba7ee90ee 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -17,6 +17,8 @@ ch_driver_sources = [ 'ch_process.h', 'ch_hostdev.c', 'ch_hostdev.h', + 'ch_hotplug.c', + 'ch_hotplug.h', ] driver_source_files += files(ch_driver_sources) -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index e5e1bfd7d1..764efb87e9 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -25,6 +25,7 @@ #include "ch_conf.h" #include "ch_domain.h" #include "ch_driver.h" +#include "ch_hotplug.h" #include "ch_monitor.h" #include "ch_process.h" #include "domain_cgroup.h" @@ -2346,9 +2347,10 @@ chDomainInterfaceAddresses(virDomain *dom, static int chDomainAttachDeviceFlags(virDomainPtr dom, - const char *xml G_GNUC_UNUSED, + const char *xml, unsigned int flags) { + virCHDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; int ret = -1; @@ -2364,6 +2366,12 @@ chDomainAttachDeviceFlags(virDomainPtr dom, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; + if (chDomainAttachDeviceLiveAndUpdateConfig(vm, driver, xml, flags) < 0) { + goto endjob; + } + + ret = 0; + endjob: virDomainObjEndJob(vm); -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_monitor.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 6bf877fef3..d369236183 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -249,6 +249,11 @@ virCHMonitorBuildDiskJson(virJSONValue *disks, virDomainDiskDef *diskdef) _("Missing disk file path in domain")); return -1; } + if (!diskdef->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing disk alias")); + return -1; + } if (diskdef->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { virReportError(VIR_ERR_INVALID_ARG, _("Only virtio bus types are supported for '%1$s'"), @@ -261,6 +266,9 @@ virCHMonitorBuildDiskJson(virJSONValue *disks, virDomainDiskDef *diskdef) if (virJSONValueObjectAppendBoolean(disk, "readonly", true) < 0) return -1; } + if (virJSONValueObjectAppendString(disk, "id", diskdef->info.alias) < 0) { + return -1; + } if (virJSONValueArrayAppend(disks, &disk) < 0) return -1; -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_alias.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++ src/ch/ch_alias.h | 27 +++++++++++++++++++++ src/ch/meson.build | 2 ++ 3 files changed, 88 insertions(+) create mode 100644 src/ch/ch_alias.c create mode 100644 src/ch/ch_alias.h diff --git a/src/ch/ch_alias.c b/src/ch/ch_alias.c new file mode 100644 index 0000000000..63bcd9f212 --- /dev/null +++ b/src/ch/ch_alias.c @@ -0,0 +1,59 @@ +/* + * ch_alias.c: CH device alias handling + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "virutil.h" + +#include "ch_alias.h" + +#define VIR_FROM_THIS VIR_FROM_CH + +int chAssignDeviceDiskAlias(virDomainDiskDef *disk) +{ + const char *prefix = virDomainDiskBusTypeToString(disk->bus); + int idx = -1; + + if (disk->info.alias) { + return 0; + } + + idx = virDiskNameToIndex(disk->dst); + + if (idx < 0) { + return -1; + } + + disk->info.alias = g_strdup_printf("%s-disk%d", prefix, idx); + + return 0; +} + +int chAssignDeviceAliases(virDomainDef *def) +{ + size_t i; + + for (i = 0; i < def->ndisks; i++) { + if (chAssignDeviceDiskAlias(def->disks[i]) < 0) + return -1; + } + + /* TODO: handle other devices */ + + return 0; +} diff --git a/src/ch/ch_alias.h b/src/ch/ch_alias.h new file mode 100644 index 0000000000..81e20c27c7 --- /dev/null +++ b/src/ch/ch_alias.h @@ -0,0 +1,27 @@ +/* + * ch_alias.h: CH device alias handling + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include "domain_conf.h" + +int +chAssignDeviceDiskAlias(virDomainDiskDef *disk); + +int +chAssignDeviceAliases(virDomainDef *def); diff --git a/src/ch/meson.build b/src/ch/meson.build index bba7ee90ee..b3e9c03832 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -19,6 +19,8 @@ ch_driver_sources = [ 'ch_hostdev.h', 'ch_hotplug.c', 'ch_hotplug.h', + 'ch_alias.c', + 'ch_alias.h', ] driver_source_files += files(ch_driver_sources) -- 2.50.1

This allows users to call API endpoints that require passing data in a generic way. Previously, only virCHMonitorPutNoContent was offered. On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_monitor.c | 61 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index d369236183..63c8425b4b 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -62,6 +62,10 @@ VIR_ONCE_GLOBAL_INIT(virCHMonitor); int virCHMonitorShutdownVMM(virCHMonitor *mon); int virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint, domainLogContext *logCtxt); +int +virCHMonitorPut(virCHMonitor *mon, const char *endpoint, + const char *payload, domainLogContext *logCtxt, + virJSONValue** answer); static int virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef) @@ -866,6 +870,63 @@ curl_callback(void *contents, size_t size, size_t nmemb, void *userp) return content_size; } +int +virCHMonitorPut(virCHMonitor *mon, const char *endpoint, + const char *payload, domainLogContext *logCtxt, + virJSONValue** answer) +{ + VIR_LOCK_GUARD lock = virObjectLockGuard(mon); + g_autofree char *url = NULL; + int responseCode = 0; + struct curl_data data = {0}; + struct curl_slist *headers = NULL; + + url = g_strdup_printf("%s/%s", URL_ROOT, endpoint); + + /* reset all options of a libcurl session handle at first */ + curl_easy_reset(mon->handle); + + curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath); + curl_easy_setopt(mon->handle, CURLOPT_URL, url); + curl_easy_setopt(mon->handle, CURLOPT_UPLOAD, 1L); + curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, NULL); + curl_easy_setopt(mon->handle, CURLOPT_INFILESIZE, 0L); + + headers = curl_slist_append(headers, "Content-Type: application/json"); + + curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers); + curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION, curl_callback); + curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void *)&data); + + if (payload) { + curl_easy_setopt(mon->handle, CURLOPT_POSTFIELDS, payload); + curl_easy_setopt(mon->handle, CURLOPT_CUSTOMREQUEST, "PUT"); + headers = curl_slist_append(headers, "Accept: application/json"); + } + + responseCode = virCHMonitorCurlPerform(mon->handle); + + data.content = g_realloc(data.content, data.size + 1); + data.content[data.size] = 0; + + if (logCtxt && data.size) { + /* Do this to append a NULL char at the end of data */ + domainLogContextWrite(logCtxt, "HTTP response code from CH: %d\n", responseCode); + domainLogContextWrite(logCtxt, "Response = %s\n", data.content); + } + + curl_slist_free_all(headers); + + if (responseCode != 200 && responseCode != 204) { + return -1; + } + + if (answer) + *answer = virJSONValueFromString(data.content); + + return 0; +} + int virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint, domainLogContext *logCtxt) -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_monitor.c | 40 +--------------------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 63c8425b4b..5f3e2adbee 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -931,45 +931,7 @@ int virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint, domainLogContext *logCtxt) { - VIR_LOCK_GUARD lock = virObjectLockGuard(mon); - g_autofree char *url = NULL; - int responseCode = 0; - int ret = -1; - struct curl_data data = {0}; - struct curl_slist *headers = NULL; - - url = g_strdup_printf("%s/%s", URL_ROOT, endpoint); - - /* reset all options of a libcurl session handle at first */ - curl_easy_reset(mon->handle); - - curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath); - curl_easy_setopt(mon->handle, CURLOPT_URL, url); - curl_easy_setopt(mon->handle, CURLOPT_UPLOAD, 1L); - curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, NULL); - curl_easy_setopt(mon->handle, CURLOPT_INFILESIZE, 0L); - - headers = curl_slist_append(headers, "Accept: application/json"); - curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, headers); - curl_easy_setopt(mon->handle, CURLOPT_WRITEFUNCTION, curl_callback); - curl_easy_setopt(mon->handle, CURLOPT_WRITEDATA, (void *)&data); - - responseCode = virCHMonitorCurlPerform(mon->handle); - - if (logCtxt && data.size) { - /* Do this to append a NULL char at the end of data */ - data.content = g_realloc(data.content, data.size + 1); - data.content[data.size] = 0; - domainLogContextWrite(logCtxt, "HTTP response code from CH: %d\n", responseCode); - domainLogContextWrite(logCtxt, "Response = %s\n", data.content); - } - - if (responseCode == 200 || responseCode == 204) - ret = 0; - - curl_slist_free_all(headers); - - return ret; + return virCHMonitorPut(mon, endpoint, NULL, logCtxt, NULL); } static int -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- po/POTFILES | 1 + src/ch/ch_hotplug.c | 131 ++++++++++++++++++++++++++++++++++++++++++-- src/ch/ch_monitor.c | 17 ++++++ src/ch/ch_monitor.h | 4 ++ 4 files changed, 149 insertions(+), 4 deletions(-) diff --git a/po/POTFILES b/po/POTFILES index dc7293d0cd..50a055fb73 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -24,6 +24,7 @@ src/ch/ch_domain.c src/ch/ch_driver.c src/ch/ch_events.c src/ch/ch_hostdev.c +src/ch/ch_hotplug.c src/ch/ch_interface.c src/ch/ch_monitor.c src/ch/ch_process.c 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; } 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) { + 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); -- 2.50.1

This is required to have unique device aliases for devices throughout the domain lifecycle. On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_process.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index cd2e88af1e..550bfefae1 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -24,6 +24,7 @@ #include <fcntl.h> #include <poll.h> +#include "ch_alias.h" #include "ch_domain.h" #include "ch_monitor.h" #include "ch_process.h" @@ -889,6 +890,9 @@ virCHProcessPrepareDomain(virDomainObj *vm) if (virCHProcessPrepareDomainHostdevs(vm) < 0) return -1; + if (chAssignDeviceAliases(vm->def) < 0) + return -1; + return 0; } -- 2.50.1

The function calls the respective CH API to remove a device of any type from a VM. On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_monitor.c | 13 +++++++++++++ src/ch/ch_monitor.h | 3 +++ 2 files changed, 16 insertions(+) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 8968d84a71..ee629f61fb 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -596,6 +596,19 @@ virCHMonitorBuildKeyValueStringJson(char **jsonstr, return 0; } +int virCHMonitorRemoveDevice(virCHMonitor *mon, + const char* device_id) +{ + g_autofree char *payload = NULL; + + if (virCHMonitorBuildKeyValueStringJson(&payload, "id", device_id) != 0) + return -1; + + VIR_DEBUG("Remove device %s", device_id); + + return virCHMonitorPut(mon, URL_VM_REMOVE_DEVICE, payload, NULL, NULL); +} + static int chMonitorCreateSocket(const char *socket_path) { diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h index 8338059c7c..e8fa393e54 100644 --- a/src/ch/ch_monitor.h +++ b/src/ch/ch_monitor.h @@ -41,6 +41,7 @@ #define URL_VM_SAVE "vm.snapshot" #define URL_VM_RESTORE "vm.restore" #define URL_VM_ADD_DISK "vm.add-disk" +#define URL_VM_REMOVE_DEVICE "vm.remove-device" #define VIRCH_THREAD_NAME_LEN 16 @@ -141,6 +142,8 @@ virCHMonitorBuildNetJson(virDomainNetDef *netdef, char **jsonstr); int virCHMonitorAddDisk(virCHMonitor* mon, virDomainDiskDef *diskdef); +int virCHMonitorRemoveDevice(virCHMonitor *mon, + const char* device_id); int virCHMonitorBuildRestoreJson(virDomainDef *vmdef, const char *from, -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_driver.c | 8 +- src/ch/ch_hotplug.c | 175 ++++++++++++++++++++++++++++++++++++++++++++ src/ch/ch_hotplug.h | 6 ++ src/ch/ch_monitor.c | 2 +- 4 files changed, 189 insertions(+), 2 deletions(-) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 764efb87e9..760fccba82 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -2389,9 +2389,10 @@ chDomainAttachDevice(virDomainPtr dom, static int chDomainDetachDeviceFlags(virDomainPtr dom, - const char *xml G_GNUC_UNUSED, + const char *xml, unsigned int flags) { + virCHDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; int ret = -1; @@ -2407,6 +2408,11 @@ chDomainDetachDeviceFlags(virDomainPtr dom, if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) goto endjob; + if (chDomainDetachDeviceLiveAndUpdateConfig(driver, vm, xml, flags) < 0) + goto endjob; + + ret = 0; + endjob: virDomainObjEndJob(vm); 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); diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index ee629f61fb..2da71e962b 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -604,7 +604,7 @@ int virCHMonitorRemoveDevice(virCHMonitor *mon, if (virCHMonitorBuildKeyValueStringJson(&payload, "id", device_id) != 0) return -1; - VIR_DEBUG("Remove device %s", device_id); + VIR_DEBUG("Remove device with id: %s", device_id); return virCHMonitorPut(mon, URL_VM_REMOVE_DEVICE, payload, NULL, NULL); } -- 2.50.1

On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- NEWS.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 9577be0213..9b64489954 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,11 @@ v11.8.0 (unreleased) * **New features** + * ch: Disk hotplug Support + + Users can now attach and detach disks of Cloud Hypervisor domains at + runtime. + * **Improvements** * **Bug fixes** -- 2.50.1

I think I missed most of Michal's comments on the v1 patchset. I will incorporate those.
participants (2)
-
Stefan Kober
-
stefan.kober@cyberus-technology.de