[PATCH 0/5] CH: Add network interface hotplug support
This patch set adds support for attaching and detaching network interfaces to Cloud Hypervisor domains at runtime. Stefan Kober (5): ch: pass virCHDriver to chDomainAttachDeviceLice ch: add net device alias assignment ch: implement network device hot attach ch: implement network device hot detach NEWS: ch: announce network hotplug feature NEWS.rst | 5 ++ src/ch/ch_alias.c | 47 +++++++++++++ src/ch/ch_alias.h | 3 + src/ch/ch_hotplug.c | 83 ++++++++++++++++++++-- src/ch/ch_monitor.c | 5 +- src/ch/ch_monitor.h | 1 - src/ch/ch_process.c | 163 +++++++++++++++++++++++++------------------- src/ch/ch_process.h | 8 +++ 8 files changed, 234 insertions(+), 81 deletions(-) -- 2.51.0
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c index b953fe4c6c..902eab839b 100644 --- a/src/ch/ch_hotplug.c +++ b/src/ch/ch_hotplug.c @@ -18,9 +18,11 @@ #include <config.h> -#include "ch_hotplug.h" #include "ch_alias.h" #include "ch_domain.h" +#include "ch_hotplug.h" +#include "ch_process.h" + #include "domain_event.h" #include "domain_validate.h" #include "virlog.h" -- 2.51.0
On 10/6/25 17:18, Stefan Kober wrote:
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c index b953fe4c6c..902eab839b 100644 --- a/src/ch/ch_hotplug.c +++ b/src/ch/ch_hotplug.c @@ -18,9 +18,11 @@
#include <config.h>
-#include "ch_hotplug.h" #include "ch_alias.h" #include "ch_domain.h" +#include "ch_hotplug.h" +#include "ch_process.h" + #include "domain_event.h" #include "domain_validate.h" #include "virlog.h"
This doesn't pass anything. Michal
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_alias.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ src/ch/ch_alias.h | 3 +++ 2 files changed, 50 insertions(+) diff --git a/src/ch/ch_alias.c b/src/ch/ch_alias.c index bdf8452a3c..dad297b20c 100644 --- a/src/ch/ch_alias.c +++ b/src/ch/ch_alias.c @@ -18,11 +18,13 @@ #include <config.h> +#include "virstring.h" #include "virutil.h" #include "ch_alias.h" #define VIR_FROM_THIS VIR_FROM_CH +#define CH_NET_ID_PREFIX "net" int chAssignDeviceDiskAlias(virDomainDiskDef *disk) { @@ -47,6 +49,47 @@ int chAssignDeviceDiskAlias(virDomainDiskDef *disk) return 0; } +/** + * Extract the index number of some device alias + */ +static +int chDomainDeviceAliasIndex(const virDomainDeviceInfo *info, + const char *prefix) +{ + int idx; + + if (!info->alias) + return -1; + if (!STRPREFIX(info->alias, prefix)) + return -1; + + if (virStrToLong_i(info->alias + strlen(prefix), NULL, 10, &idx) < 0) + return -1; + + return idx; +} + +void chAssignDeviceNetAlias(virDomainDef *def, virDomainNetDef *net) +{ + size_t idx = 0; + size_t i; + + if (net->info.alias) { + return; + } + + for (i = 0; i < def->nnets; i++) { + int thisidx; + + if ((thisidx = chDomainDeviceAliasIndex(&def->nets[i]->info, CH_NET_ID_PREFIX)) < 0) + continue; + if (thisidx >= idx) + idx = thisidx + 1; + } + + net->info.alias = g_strdup_printf("%s%lu", CH_NET_ID_PREFIX, idx); +} + int chAssignDeviceAliases(virDomainDef *def) { size_t i; @@ -56,6 +99,10 @@ int chAssignDeviceAliases(virDomainDef *def) return -1; } + for (i = 0; i < def->nnets; i++) { + chAssignDeviceNetAlias(def, def->nets[i]); + } + /* TODO: handle other devices */ return 0; diff --git a/src/ch/ch_alias.h b/src/ch/ch_alias.h index 81e20c27c7..b9a12fac11 100644 --- a/src/ch/ch_alias.h +++ b/src/ch/ch_alias.h @@ -23,5 +23,8 @@ int chAssignDeviceDiskAlias(virDomainDiskDef *disk); +void +chAssignDeviceNetAlias(virDomainDef *def, virDomainNetDef *net); + int chAssignDeviceAliases(virDomainDef *def); -- 2.51.0
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_hotplug.c | 10 +++ src/ch/ch_monitor.c | 5 +- src/ch/ch_monitor.h | 1 - src/ch/ch_process.c | 163 +++++++++++++++++++++++++------------------- src/ch/ch_process.h | 8 +++ 5 files changed, 111 insertions(+), 76 deletions(-) diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c index 902eab839b..25058e08f3 100644 --- a/src/ch/ch_hotplug.c +++ b/src/ch/ch_hotplug.c @@ -75,6 +75,16 @@ chDomainAttachDeviceLive(virCHDriver *driver, break; case VIR_DOMAIN_DEVICE_NET: + if (chProcessAddNetworkDevice(driver, mon, vm->def, dev->data.net, + NULL, NULL) < 0) { + break; + } + + virDomainNetInsert(vm->def, dev->data.net); + + dev->data.net = NULL; + ret = 0; + break; case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 65dc89cfd1..46b31acc76 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -379,7 +379,6 @@ virCHMonitorBuildRngJson(virJSONValue *content, virDomainDef *vmdef) /** * virCHMonitorBuildNetJson: * @net: pointer to a guest network definition - * @netindex: index of the guest network definition * @jsonstr: returned network json * * Build net json to send to CH @@ -387,15 +386,13 @@ virCHMonitorBuildRngJson(virJSONValue *content, virDomainDef *vmdef) */ int virCHMonitorBuildNetJson(virDomainNetDef *net, - int netindex, char **jsonstr) { char macaddr[VIR_MAC_STRING_BUFLEN]; g_autoptr(virJSONValue) net_json = virJSONValueNewObject(); virDomainNetType actualType = virDomainNetGetActualType(net); - g_autofree char *id = g_strdup_printf("%s_%d", CH_NET_ID_PREFIX, netindex); - if (virJSONValueObjectAppendString(net_json, "id", id) < 0) + if (virJSONValueObjectAppendString(net_json, "id", net->info.alias) < 0) return -1; if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET && diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h index e3fde06f88..a462630be0 100644 --- a/src/ch/ch_monitor.h +++ b/src/ch/ch_monitor.h @@ -138,7 +138,6 @@ int virCHMonitorGetIOThreads(virCHMonitor *mon, virDomainIOThreadInfo ***iothreads); int virCHMonitorBuildNetJson(virDomainNetDef *netdef, - int netindex, char **jsonstr); int virCHMonitorAddDisk(virCHMonitor* mon, diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 54b21b0baf..a1f30f09e1 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -624,30 +624,24 @@ chCloseFDs(int *fds, size_t nfds) return 0; } -/** - * chProcessAddNetworkDevices: - * @driver: pointer to ch driver object - * @mon: pointer to the monitor object - * @vmdef: pointer to domain definition - * @nicindexes: returned array of FDs of guest interfaces - * @nnicindexes: returned number of network indexes - * - * Send tap fds to CH process via AddNet api. Capture the network indexes of - * guest interfaces in nicindexes. - * - * Returns 0 on success, -1 on error. - */ -static int -chProcessAddNetworkDevices(virCHDriver *driver, - virCHMonitor *mon, - virDomainDef *vmdef, - int **nicindexes, - size_t *nnicindexes) +int +chProcessAddNetworkDevice(virCHDriver *driver, + virCHMonitor *mon, + virDomainDef *vmdef, + virDomainNetDef *net, + int **nicindexes, + size_t *nnicindexes) { - size_t i; VIR_AUTOCLOSE mon_sockfd = -1; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) http_headers = VIR_BUFFER_INITIALIZER; + g_autofree int *tapfds = NULL; + g_autofree char *payload = NULL; + g_autofree char *response = NULL; + size_t tapfd_len; + size_t payload_len; + int saved_errno; + int rc; if (!virBitmapIsBitSet(driver->chCaps, CH_MULTIFD_IN_ADDNET)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -655,73 +649,100 @@ chProcessAddNetworkDevices(virCHDriver *driver, return -1; } - if ((mon_sockfd = chMonitorSocketConnect(mon)) < 0) + if ((mon_sockfd = chMonitorSocketConnect(mon)) < 0) { + VIR_WARN("chProcessAddNetworkDevices failed"); return -1; + } virBufferAddLit(&http_headers, "PUT /api/v1/vm.add-net HTTP/1.1\r\n"); virBufferAddLit(&http_headers, "Host: localhost\r\n"); virBufferAddLit(&http_headers, "Content-Type: application/json\r\n"); - for (i = 0; i < vmdef->nnets; i++) { - g_autofree int *tapfds = NULL; - g_autofree char *payload = NULL; - g_autofree char *response = NULL; - size_t tapfd_len; - size_t payload_len; - int saved_errno; - int rc; - - if (vmdef->nets[i]->driver.virtio.queues == 0) { - /* "queues" here refers to queue pairs. When 0, initialize - * queue pairs to 1. - */ - vmdef->nets[i]->driver.virtio.queues = 1; - } - tapfd_len = vmdef->nets[i]->driver.virtio.queues; - if (virCHDomainValidateActualNetDef(vmdef->nets[i]) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("net definition failed validation")); - return -1; - } + if (net->driver.virtio.queues == 0) { + /* "queues" here refers to queue pairs. When 0, initialize + * queue pairs to 1. + */ + net->driver.virtio.queues = 1; + } + tapfd_len = net->driver.virtio.queues; - tapfds = g_new0(int, tapfd_len); - memset(tapfds, -1, (tapfd_len) * sizeof(int)); + if (virCHDomainValidateActualNetDef(net) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("net definition failed validation")); + return -1; + } - /* Connect Guest interfaces */ - if (virCHConnetNetworkInterfaces(driver, vmdef, vmdef->nets[i], tapfds, - nicindexes, nnicindexes) < 0) - return -1; + tapfds = g_new0(int, tapfd_len); + memset(tapfds, -1, (tapfd_len) * sizeof(int)); - if (virCHMonitorBuildNetJson(vmdef->nets[i], i, &payload) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to build net json")); - return -1; - } + /* Connect Guest interfaces */ + if (virCHConnetNetworkInterfaces(driver, vmdef, net, tapfds, + nicindexes, nnicindexes) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to connect network interfaces")); + return -1; + } - VIR_DEBUG("payload sent with net-add request to CH = %s", payload); + chAssignDeviceNetAlias(vmdef, net); + if (virCHMonitorBuildNetJson(net, &payload) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to build net json")); + return -1; + } - virBufferAsprintf(&buf, "%s", virBufferCurrentContent(&http_headers)); - virBufferAsprintf(&buf, "Content-Length: %zu\r\n\r\n", strlen(payload)); - virBufferAsprintf(&buf, "%s", payload); - payload_len = virBufferUse(&buf); - payload = virBufferContentAndReset(&buf); + virBufferAsprintf(&buf, "%s", virBufferCurrentContent(&http_headers)); + virBufferAsprintf(&buf, "Content-Length: %zu\r\n\r\n", strlen(payload)); + virBufferAsprintf(&buf, "%s", payload); + payload_len = virBufferUse(&buf); + payload = virBufferContentAndReset(&buf); - rc = virSocketSendMsgWithFDs(mon_sockfd, payload, payload_len, - tapfds, tapfd_len); - saved_errno = errno; + rc = virSocketSendMsgWithFDs(mon_sockfd, payload, payload_len, + tapfds, tapfd_len); + saved_errno = errno; - /* Close sent tap fds in Libvirt, as they have been dup()ed in CH */ - chCloseFDs(tapfds, tapfd_len); + /* Close sent tap fds in Libvirt, as they have been dup()ed in CH */ + chCloseFDs(tapfds, tapfd_len); - if (rc < 0) { - virReportSystemError(saved_errno, "%s", - _("Failed to send net-add request to CH")); - return -1; - } + if (rc < 0) { + virReportSystemError(saved_errno, "%s", + _("Failed to send net-add request to CH")); + return -1; + } - if (chSocketProcessHttpResponse(mon_sockfd, true) < 0) - return -1; + if (chSocketProcessHttpResponse(mon_sockfd, true) < 0) + return -1; + + return 0; +} + +/** + * chProcessAddNetworkDevices: + * @driver: pointer to ch driver object + * @mon: pointer to the monitor object + * @vmdef: pointer to domain definition + * @nicindexes: returned array of FDs of guest interfaces + * @nnicindexes: returned number of network indexes + * + * Send tap fds to CH process via AddNet api. Capture the network indexes of + * guest interfaces in nicindexes. + * + * Returns 0 on success, -1 on error. + */ +static int +chProcessAddNetworkDevices(virCHDriver *driver, + virCHMonitor *mon, + virDomainDef *vmdef, + int **nicindexes, + size_t *nnicindexes) +{ + size_t i = 0; + + for (i = 0; i < vmdef->nnets; i++) { + if (chProcessAddNetworkDevice(driver, mon, vmdef, vmdef->nets[i], + nicindexes, nnicindexes) < 0) { + return -1; + } } return 0; diff --git a/src/ch/ch_process.h b/src/ch/ch_process.h index a22790bb5c..e9b1f559d9 100644 --- a/src/ch/ch_process.h +++ b/src/ch/ch_process.h @@ -41,3 +41,11 @@ int virCHProcessStartRestore(virCHDriver *driver, const char *from); int virCHProcessUpdateInfo(virDomainObj *vm); + +int +chProcessAddNetworkDevice(virCHDriver *driver, + virCHMonitor *mon, + virDomainDef *vmdef, + virDomainNetDef *net, + int **nicindexes, + size_t *nnicindexes); -- 2.51.0
On 10/6/25 17:18, Stefan Kober wrote:
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_hotplug.c | 10 +++ src/ch/ch_monitor.c | 5 +- src/ch/ch_monitor.h | 1 - src/ch/ch_process.c | 163 +++++++++++++++++++++++++------------------- src/ch/ch_process.h | 8 +++ 5 files changed, 111 insertions(+), 76 deletions(-)
diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c index 902eab839b..25058e08f3 100644 --- a/src/ch/ch_hotplug.c +++ b/src/ch/ch_hotplug.c @@ -75,6 +75,16 @@ chDomainAttachDeviceLive(virCHDriver *driver, break;
case VIR_DOMAIN_DEVICE_NET: + if (chProcessAddNetworkDevice(driver, mon, vm->def, dev->data.net, + NULL, NULL) < 0) { + break; + } + + virDomainNetInsert(vm->def, dev->data.net); +
Here, we need: alias = dev->data.net->info.alias; so that the code later in this function emits an event that the device was attached.
+ dev->data.net = NULL; + ret = 0; + break; case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT:
Michal
On-behalf-of: SAP stefan.kober@sap.com Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> --- src/ch/ch_hotplug.c | 69 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/src/ch/ch_hotplug.c b/src/ch/ch_hotplug.c index 25058e08f3..480acf93a6 100644 --- a/src/ch/ch_hotplug.c +++ b/src/ch/ch_hotplug.c @@ -24,6 +24,7 @@ #include "ch_process.h" #include "domain_event.h" +#include "domain_interface.h" #include "domain_validate.h" #include "virlog.h" @@ -220,6 +221,50 @@ chDomainFindDisk(virDomainObj *vm, } +static int +chFindNetID(virDomainDef *def, const char *dst) +{ + size_t i; + + for (i = 0; i < def->nnets; i++) { + if (STREQ(def->nets[i]->ifname, dst)) + return i; + } + + return -1; +} + +/** + * chDomainFindNet + * + * Helper function to find a network device definition of a domain. + * + * Searches through the network devices of a domain by comparing to 'match' and + * returns any match via the 'detach' out parameter. + */ +static int +chDomainFindNet(virDomainObj *vm, + virDomainNetDef *match, + virDomainNetDef **detach) +{ + int idx; + + if (!match->ifname) { + virReportError(VIR_ERR_DEVICE_MISSING, "%s", + _("no interface name specified")); + return -1; + } + if ((idx = chFindNetID(vm->def, match->ifname)) < 0) { + virReportError(VIR_ERR_DEVICE_MISSING, + _("net %1$s not found"), match->ifname); + return -1; + } + *detach = vm->def->nets[idx]; + + return 0; +} + + static int chDomainRemoveDevice(virDomainObj *vm, virDomainDeviceDef *device) @@ -239,9 +284,19 @@ chDomainRemoveDevice(virDomainObj *vm, } } break; + case VIR_DOMAIN_DEVICE_NET: + virDomainInterfaceStopDevice(device->data.net); + virDomainInterfaceDeleteDevice(vm->def, device->data.net, false, NULL); + for (i = 0; i < vm->def->nnets; i++) { + if (vm->def->nets[i] == device->data.net) { + virDomainNetRemove(vm->def, i); + g_clear_pointer(&device->data.net, virDomainNetDefFree); + break; + } + } + 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: @@ -296,9 +351,14 @@ chDomainDetachDeviceLive(virCHDriver *driver, return -1; } break; + case VIR_DOMAIN_DEVICE_NET: + if (chDomainFindNet(vm, match->data.net, + &detach.data.net) < 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: @@ -360,8 +420,9 @@ chDomainDetachDeviceLive(virCHDriver *driver, alias = g_strdup(info->alias); if (virCHMonitorRemoveDevice(priv->monitor, info->alias) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Invalid response from CH. Disk removal failed.")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid response from CH. Device removal failed for device %1$s."), + info->alias); return -1; } -- 2.51.0
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 006b2ccf6a..fa33721763 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,11 @@ v11.9.0 (unreleased) * **New features** + * ch: Network hotplug Support + + Users can now attach and detach network interfaces of Cloud Hypervisor + domains at runtime. + * **Improvements** * qemu: Improvements to USB controller model selection -- 2.51.0
On 10/6/25 17:18, Stefan Kober wrote:
This patch set adds support for attaching and detaching network interfaces to Cloud Hypervisor domains at runtime.
Stefan Kober (5): ch: pass virCHDriver to chDomainAttachDeviceLice ch: add net device alias assignment ch: implement network device hot attach ch: implement network device hot detach NEWS: ch: announce network hotplug feature
NEWS.rst | 5 ++ src/ch/ch_alias.c | 47 +++++++++++++ src/ch/ch_alias.h | 3 + src/ch/ch_hotplug.c | 83 ++++++++++++++++++++-- src/ch/ch_monitor.c | 5 +- src/ch/ch_monitor.h | 1 - src/ch/ch_process.c | 163 +++++++++++++++++++++++++------------------- src/ch/ch_process.h | 8 +++ 8 files changed, 234 insertions(+), 81 deletions(-)
Just so that you know, I've seen these and did some initial review. They are mostly okay, but I think we'll need some preparation work. I mean, having functions talking to the CH socket in ch_process.c seems odd. Alternatively, I can merge these (with very little changes - formatting mostly) and then post cleanup patches that move things around. What do you prefer? Michal
Alternatively, I can merge these (with very little changes - formatting mostly) and then post cleanup patches that move things around. What do you prefer?
I am fine if you make small adjustment and put any larger changes on top. Hopefully, doing so can save us some back and forth and I will take a look at the end result to get a better feeling for how to structure things in the first place for next time. Thanks for taking care :)
On 10/6/25 17:18, Stefan Kober wrote:
This patch set adds support for attaching and detaching network interfaces to Cloud Hypervisor domains at runtime.
Stefan Kober (5): ch: pass virCHDriver to chDomainAttachDeviceLice ch: add net device alias assignment ch: implement network device hot attach ch: implement network device hot detach NEWS: ch: announce network hotplug feature
NEWS.rst | 5 ++ src/ch/ch_alias.c | 47 +++++++++++++ src/ch/ch_alias.h | 3 + src/ch/ch_hotplug.c | 83 ++++++++++++++++++++-- src/ch/ch_monitor.c | 5 +- src/ch/ch_monitor.h | 1 - src/ch/ch_process.c | 163 +++++++++++++++++++++++++------------------- src/ch/ch_process.h | 8 +++ 8 files changed, 234 insertions(+), 81 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. As discussed earlier, I'll post some cleanups later. Michal
participants (3)
-
Michal Prívozník -
Stefan Kober -
stefan.kober＠cyberus-technology.de