On Thu, Jul 25, 2024 at 12:04:47PM -0500, Praveen K Paladugu wrote:
Cloud-hypervisor now supports restoring with new net fds. Ref: https://github.com/cloud-hypervisor/cloud-hypervisor/pull/6402 So, pass new tap fds via SCM_RIGHTS to CH's restore api.
Signed-off-by: Purna Pavan Chandra <paekkaladevi@linux.microsoft.com> --- src/ch/ch_capabilities.c | 6 +++ src/ch/ch_capabilities.h | 1 + src/ch/ch_driver.c | 29 +++++++----- src/ch/ch_monitor.c | 21 ++++++++- src/ch/ch_monitor.h | 2 +- src/ch/ch_process.c | 98 +++++++++++++++++++++++++++++++++------- 6 files changed, 126 insertions(+), 31 deletions(-)
diff --git a/src/ch/ch_capabilities.c b/src/ch/ch_capabilities.c index 5941851500..f3765a8095 100644 --- a/src/ch/ch_capabilities.c +++ b/src/ch/ch_capabilities.c @@ -65,6 +65,12 @@ virCHCapsInitCHVersionCaps(int version) if (version >= 36000000) virCHCapsSet(chCaps, CH_SOCKET_BACKEND_SERIAL_PORT); + /* Starting v40, Cloud-Hypervisor supports restore with new net fds. + * This is required to be able to restore a guest with network config define. + * https://github.com/cloud-hypervisor/cloud-hypervisor/releases/tag/v40.0 */ + if (version >= 40000000) + virCHCapsSet(chCaps, CH_RESTORE_WITH_NEW_TAPFDS); + return g_steal_pointer(&chCaps); } diff --git a/src/ch/ch_capabilities.h b/src/ch/ch_capabilities.h index 03932511f6..e5efb14b68 100644 --- a/src/ch/ch_capabilities.h +++ b/src/ch/ch_capabilities.h @@ -28,6 +28,7 @@ typedef enum { CH_SERIAL_CONSOLE_IN_PARALLEL, /* Serial and Console ports can work in parallel */ CH_MULTIFD_IN_ADDNET, /* Cloud-hypervisor can accept multiple FDs in add-net api */ CH_SOCKET_BACKEND_SERIAL_PORT, /* Support Unix socket as a backend for a serial port */ + CH_RESTORE_WITH_NEW_TAPFDS, /* Cloud-hypervisor support for restore with new net fds */ CH_CAPS_LAST /* this must always be the last item */ } virCHCapsFlags; diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index fbeac60825..24aed471c5 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -680,22 +680,24 @@ chDomainDestroy(virDomainPtr dom) } static int -chDomainSaveAdditionalValidation(virDomainDef *vmdef) +chDomainSaveRestoreAdditionalValidation(virCHDriver *driver, virDomainDef *vmdef) { - /* - SAVE and RESTORE are functional only without any networking and - device passthrough configuration - */ - if (vmdef->nnets > 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot save domain with network interfaces")); - return -1; - } + /* SAVE and RESTORE are functional only without any host device + * passthrough configuration */ if (vmdef->nhostdevs > 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot save domain with host devices")); + _("cannot save/restore domain with host devices")); return -1; } + + if (vmdef->nnets > 0) { + if (!virBitmapIsBitSet(driver->chCaps, CH_RESTORE_WITH_NEW_TAPFDS)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot save/restore domain with network devices")); + return -1; + } + } + return 0; } @@ -728,7 +730,7 @@ chDoDomainSave(virCHDriver *driver, VIR_AUTOCLOSE fd = -1; int ret = -1; - if (chDomainSaveAdditionalValidation(vm->def) < 0) + if (chDomainSaveRestoreAdditionalValidation(driver, vm->def) < 0) goto end; domainState = virDomainObjGetState(vm, NULL); @@ -1087,6 +1089,9 @@ chDomainRestoreFlags(virConnectPtr conn, if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) goto cleanup; + if (chDomainSaveRestoreAdditionalValidation(driver, def) < 0) + goto cleanup; + if (!(vm = virDomainObjListAdd(driver->domains, &def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 8d8be1f46e..3af5e7f2d1 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -978,14 +978,31 @@ virCHMonitorSaveVM(virCHMonitor *mon, const char *to) } int -virCHMonitorBuildRestoreJson(const char *from, char **jsonstr) +virCHMonitorBuildRestoreJson(virDomainDef *vmdef, const char *from, char **jsonstr) { + size_t i; g_autoptr(virJSONValue) restore_json = virJSONValueNewObject(); - g_autofree char *path_url = g_strdup_printf("file://%s", from); + if (virJSONValueObjectAppendString(restore_json, "source_url", path_url)) return -1; + /* Pass the netconfig needed to restore with new netfds */ + if (vmdef->nnets) { + g_autoptr(virJSONValue) nets = virJSONValueNewArray(); + for (i = 0; i < vmdef->nnets; i++) { + g_autoptr(virJSONValue) net_json = virJSONValueNewObject(); + g_autofree char *id = g_strdup_printf("%s_%ld", CH_NET_ID_PREFIX, i); + if (virJSONValueObjectAppendString(net_json, "id", id) < 0) + return -1; + if (virJSONValueObjectAppendNumberInt(net_json, "num_fds", vmdef->nets[i]->driver.virtio.queues)) + return -1; + virJSONValueArrayAppend(nets, &net_json); + } + if (virJSONValueObjectAppend(restore_json, "net_fds", &nets)) + return -1; + } + if (!(*jsonstr = virJSONValueToString(restore_json, false))) return -1; diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h index 375b7a49a4..02a6685d58 100644 --- a/src/ch/ch_monitor.h +++ b/src/ch/ch_monitor.h @@ -127,4 +127,4 @@ int virCHMonitorGetIOThreads(virCHMonitor *mon, virDomainIOThreadInfo ***iothreads); int virCHMonitorBuildNetJson(virDomainNetDef *netdef, int netindex, char **jsonstr); -int virCHMonitorBuildRestoreJson(const char *from, char **jsonstr); +int virCHMonitorBuildRestoreJson(virDomainDef *vmdef, const char *from, char **jsonstr); diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 0ac0a5c9a2..540bf6801d 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -29,6 +29,7 @@ #include "ch_process.h" #include "domain_cgroup.h" #include "domain_interface.h" +#include "viralloc.h" #include "virerror.h" #include "virfile.h" #include "virjson.h" @@ -716,6 +717,56 @@ chProcessAddNetworkDevices(virCHDriver *driver, return 0; } +/** + * virCHRestoreCreateNetworkDevices: + * @driver: pointer to driver structure + * @vmdef: pointer to domain definition + * @vmtapfds: returned array of FDs of guest interfaces + * @nvmtapfds: returned number of network indexes + * @nicindexes: returned array of network indexes + * @nnicindexes: returned number of network indexes + * + * Create network devices for the domain. This function is called during + * domain restore. + * + * Returns 0 on success or -1 in case of error +*/ +static int +virCHRestoreCreateNetworkDevices(virCHDriver *driver, + virDomainDef *vmdef, + int **vmtapfds, + size_t *nvmtapfds, + int **nicindexes, + size_t *nnicindexes) +{ + size_t i, j; + size_t tapfd_len; + size_t index_vmtapfds; Instead of duplicating these steps from chProcessAddNetworkDevices,
On 7/4/2024 6:13 AM, Purna Pavan Chandra wrote: please consider refactoring them into a different to be used here and in chProcessAddNetworkDevices.
chProcessAddNetworkDevices is already refactored and the common code is moved to new functions. Since the action performed inside the loop are different here, this can not be refactored further.
+ for (i = 0; i < vmdef->nnets; i++) { + g_autofree int *tapfds = NULL; + 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; + } + tapfds = g_new0(int, tapfd_len); + memset(tapfds, -1, (tapfd_len) * sizeof(int)); + + /* Connect Guest interfaces */ + if (virCHConnetNetworkInterfaces(driver, vmdef, vmdef->nets[i], tapfds, + nicindexes, nnicindexes) < 0) + return -1; + + index_vmtapfds = *nvmtapfds; + VIR_EXPAND_N(*vmtapfds, *nvmtapfds, tapfd_len); + for (j = 0; j < tapfd_len; j++) { + VIR_APPEND_ELEMENT_INPLACE(*vmtapfds, index_vmtapfds, tapfds[j]); + } + } + return 0; +} + /** * virCHProcessStartValidate: * @driver: pointer to driver structure @@ -917,7 +968,12 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from g_autofree char *payload = NULL; g_autofree char *response = NULL; VIR_AUTOCLOSE mon_sockfd = -1; + g_autofree int *tapfds = NULL; + g_autofree int *nicindexes = NULL; size_t payload_len; + size_t ntapfds = 0; + size_t nnicindexes = 0; + int ret = -1; if (!priv->monitor) { /* Get the first monitor connection if not already */ @@ -932,17 +988,7 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from vm->def->id = vm->pid; priv->machineName = virCHDomainGetMachineName(vm); - /* Pass 0, NULL as restore only works without networking support */ - if (virDomainCgroupSetupCgroup("ch", vm, - 0, NULL, /* nnicindexes, nicindexes */ - &priv->cgroup, - cfg->cgroupControllers, - 0, /*maxThreadsPerProc*/ - priv->driver->privileged, - priv->machineName) < 0) - return -1; - - if (virCHMonitorBuildRestoreJson(from, &payload) < 0) { + if (virCHMonitorBuildRestoreJson(vm->def, from, &payload) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to restore domain")); return -1; @@ -960,20 +1006,40 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from if ((mon_sockfd = chMonitorSocketConnect(priv->monitor)) < 0) return -1; - if (virSocketSendMsgWithFDs(mon_sockfd, payload, payload_len, NULL, 0) < 0) { + if (virCHRestoreCreateNetworkDevices(driver, vm->def, &tapfds, &ntapfds, &nicindexes, &nnicindexes) < 0) + goto cleanup; + + if (virDomainCgroupSetupCgroup("ch", vm, + nnicindexes, nicindexes, + &priv->cgroup, + cfg->cgroupControllers, + 0, /*maxThreadsPerProc*/ + priv->driver->privileged, + priv->machineName) < 0) + goto cleanup; + + /* Bring up netdevs before restoring vm */ + if (virDomainInterfaceStartDevices(vm->def) < 0) + goto cleanup; + + if (virSocketSendMsgWithFDs(mon_sockfd, payload, payload_len, tapfds, ntapfds) < 0) { virReportSystemError(errno, "%s", _("Failed to send restore request to CH")); - return -1; + goto cleanup; }
After sending the FDs, they have to closed on Libvirt side, even if the send was successful. Please check chProcessAddNetworkDevices for details on it.
This is taken care in the cleanup section.
/* Restore is a syncronous operationin CH. so, pass false to wait until there's a reponse */ if (chSocketProcessHttpResponse(mon_sockfd, false) < 0) - return -1; + goto cleanup; if (virCHProcessSetup(vm) < 0) - return -1; + goto cleanup; virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_FROM_SNAPSHOT); + ret = 0; - return 0; + cleanup: + if (tapfds) + chCloseFDs(tapfds, ntapfds); + return ret; }
-- Regards, Praveen