The response message from CH for vm.add-net api will be more helpful in debugging. Hence, log the message instead of just response code. Signed-off-by: Purna Pavan Chandra <paekkaladevi(a)linux.microsoft.com&gt; --- src/ch/ch_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 48190a1273..6f99fec900 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -665,7 +665,7 @@ chProcessAddNetworkDevices(virCHDriver *driver, } if (http_res != 204 && http_res != 200) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unexpected response from CH: %1$d"), http_res); + _("Unexpected response from CH: %1$s"), response); return -1; } } -- 2.34.1

Move monitor socket connection, response handling and closing FDs code into new functions in preparation for adding restore support for net devices. Signed-off-by: Purna Pavan Chandra <paekkaladevi(a)linux.microsoft.com&gt; --- src/ch/ch_process.c | 121 +++++++++++++++++++++++++++++--------------- 1 file changed, 80 insertions(+), 41 deletions(-) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 5907a6e81f..1943c4381b 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -490,6 +490,47 @@ virCHProcessSetup(virDomainObj *vm) } +/** + * chMonitorSocketConnect: + * @mon: pointer to monitor object + * + * Connects to the monitor socket. Caller is responsible for closing the socketfd + * + * Returns socket fd on success, -1 on error + */ +static int +chMonitorSocketConnect(virCHMonitor *mon) +{ + struct sockaddr_un server_addr; + int sock; + + sock = socket(AF_UNIX, SOCK_STREAM, 0); + if (sock < 0) { + virReportSystemError(errno, "%s", _("Failed to open a UNIX socket")); + return -1; + } + + memset(&server_addr, 0, sizeof(server_addr)); + server_addr.sun_family = AF_UNIX; + if (virStrcpyStatic(server_addr.sun_path, mon->socketpath) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("UNIX socket path '%1$s' too long"), mon->socketpath); + goto error; + } + + if (connect(sock, (struct sockaddr *)&server_addr, + sizeof(server_addr)) == -1) { + virReportSystemError(errno, "%s", _("Failed to connect to mon socket")); + goto error; + } + + return sock; + error: + VIR_FORCE_CLOSE(sock); + return -1; +} + + #define PKT_TIMEOUT_MS 500 /* ms */ static char * @@ -532,6 +573,42 @@ chSocketRecv(int sock) #undef PKT_TIMEOUT_MS +static int +chSocketProcessHttpResponse(int sock) +{ + g_autofree char *response = NULL; + int http_res; + + response = chSocketRecv(sock); + if (response == NULL) { + return -1; + } + + /* Parse the HTTP response code */ + if (sscanf(response, "HTTP/1.%*d %d", &http_res) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to parse HTTP response code")); + return -1; + } + if (http_res != 204 && http_res != 200) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected response from CH: %1$s"), response); + return -1; + } + + return 0; +} + +static int +chCloseFDs(int *fds, size_t nfds) +{ + size_t i; + for (i = 0; i < nfds; i++) { + VIR_FORCE_CLOSE(fds[i]); + } + return 0; +} + /** * chProcessAddNetworkDevices: * @driver: pointer to ch driver object @@ -554,7 +631,6 @@ chProcessAddNetworkDevices(virCHDriver *driver, { size_t i; VIR_AUTOCLOSE mon_sockfd = -1; - struct sockaddr_un server_addr; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) http_headers = VIR_BUFFER_INITIALIZER; @@ -564,25 +640,8 @@ chProcessAddNetworkDevices(virCHDriver *driver, return -1; } - mon_sockfd = socket(AF_UNIX, SOCK_STREAM, 0); - if (mon_sockfd < 0) { - virReportSystemError(errno, "%s", _("Failed to open a UNIX socket")); - return -1; - } - - memset(&server_addr, 0, sizeof(server_addr)); - server_addr.sun_family = AF_UNIX; - if (virStrcpyStatic(server_addr.sun_path, mon->socketpath) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("UNIX socket path '%1$s' too long"), mon->socketpath); - return -1; - } - - if (connect(mon_sockfd, (struct sockaddr *)&server_addr, - sizeof(server_addr)) == -1) { - virReportSystemError(errno, "%s", _("Failed to connect to mon socket")); + if ((mon_sockfd = chMonitorSocketConnect(mon)) < 0) return -1; - } virBufferAddLit(&http_headers, "PUT /api/v1/vm.add-net HTTP/1.1\r\n"); virBufferAddLit(&http_headers, "Host: localhost\r\n"); @@ -592,11 +651,9 @@ chProcessAddNetworkDevices(virCHDriver *driver, g_autofree int *tapfds = NULL; g_autofree char *payload = NULL; g_autofree char *response = NULL; - size_t j; size_t tapfd_len; size_t payload_len; int saved_errno; - int http_res; int rc; if (vmdef->nets[i]->driver.virtio.queues == 0) { @@ -640,9 +697,7 @@ chProcessAddNetworkDevices(virCHDriver *driver, saved_errno = errno; /* Close sent tap fds in Libvirt, as they have been dup()ed in CH */ - for (j = 0; j < tapfd_len; j++) { - VIR_FORCE_CLOSE(tapfds[j]); - } + chCloseFDs(tapfds, tapfd_len); if (rc < 0) { virReportSystemError(saved_errno, "%s", @@ -650,24 +705,8 @@ chProcessAddNetworkDevices(virCHDriver *driver, return -1; } - /* Process the response from CH */ - response = chSocketRecv(mon_sockfd); - if (response == NULL) { + if (chSocketProcessHttpResponse(mon_sockfd) < 0) return -1; - } - - /* Parse the HTTP response code */ - rc = sscanf(response, "HTTP/1.%*d %d", &http_res); - if (rc != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to parse HTTP response code")); - return -1; - } - if (http_res != 204 && http_res != 200) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unexpected response from CH: %1$s"), response); - return -1; - } } return 0; -- 2.34.1

Instead of curl, use low-level socket connections to make restore api request to CH. This will enable passing new net FDs to CH while restoring domains with network configuration. Signed-off-by: Purna Pavan Chandra <paekkaladevi(a)linux.microsoft.com&gt; --- src/ch/ch_monitor.c | 13 +++++++++++-- src/ch/ch_monitor.h | 2 +- src/ch/ch_process.c | 40 ++++++++++++++++++++++++++++++++++------ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 6af5b29d82..0875887fd8 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -991,9 +991,18 @@ virCHMonitorSaveVM(virCHMonitor *mon, const char *to) } int -virCHMonitorRestoreVM(virCHMonitor *mon, const char *from) +virCHMonitorBuildRestoreJson(const char *from, char **jsonstr) { - return virCHMonitorSaveRestoreVM(mon, from, false); + 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; + + if (!(*jsonstr = virJSONValueToString(restore_json, false))) + return -1; + + return 0; } /** diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h index bac80b5fcb..375b7a49a4 100644 --- a/src/ch/ch_monitor.h +++ b/src/ch/ch_monitor.h @@ -115,7 +115,6 @@ int virCHMonitorRebootVM(virCHMonitor *mon); int virCHMonitorSuspendVM(virCHMonitor *mon); int virCHMonitorResumeVM(virCHMonitor *mon); int virCHMonitorSaveVM(virCHMonitor *mon, const char *to); -int virCHMonitorRestoreVM(virCHMonitor *mon, const char *from); int virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue **info); void virCHMonitorCPUInfoFree(virCHMonitorCPUInfo *cpus); @@ -128,3 +127,4 @@ int virCHMonitorGetIOThreads(virCHMonitor *mon, virDomainIOThreadInfo ***iothreads); int virCHMonitorBuildNetJson(virDomainNetDef *netdef, int netindex, char **jsonstr); +int virCHMonitorBuildRestoreJson(const char *from, char **jsonstr); diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 851ab7545c..0ac0a5c9a2 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -912,6 +912,12 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from { virCHDomainObjPrivate *priv = vm->privateData; g_autoptr(virCHDriverConfig) cfg = virCHDriverGetConfig(priv->driver); + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) http_headers = VIR_BUFFER_INITIALIZER; + g_autofree char *payload = NULL; + g_autofree char *response = NULL; + VIR_AUTOCLOSE mon_sockfd = -1; + size_t payload_len; if (!priv->monitor) { /* Get the first monitor connection if not already */ @@ -926,12 +932,6 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from vm->def->id = vm->pid; priv->machineName = virCHDomainGetMachineName(vm); - if (virCHMonitorRestoreVM(priv->monitor, from) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to restore domain")); - return -1; - } - /* Pass 0, NULL as restore only works without networking support */ if (virDomainCgroupSetupCgroup("ch", vm, 0, NULL, /* nnicindexes, nicindexes */ @@ -942,6 +942,34 @@ virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from priv->machineName) < 0) return -1; + if (virCHMonitorBuildRestoreJson(from, &payload) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to restore domain")); + return -1; + } + + virBufferAddLit(&http_headers, "PUT /api/v1/vm.restore HTTP/1.1\r\n"); + virBufferAddLit(&http_headers, "Host: localhost\r\n"); + virBufferAddLit(&http_headers, "Content-Type: application/json\r\n"); + virBufferAsprintf(&buf, "%s", virBufferCurrentContent(&http_headers)); + virBufferAsprintf(&buf, "Content-Length: %ld\r\n\r\n", strlen(payload)); + virBufferAsprintf(&buf, "%s", payload); + payload_len = virBufferUse(&buf); + payload = virBufferContentAndReset(&buf); + + if ((mon_sockfd = chMonitorSocketConnect(priv->monitor)) < 0) + return -1; + + if (virSocketSendMsgWithFDs(mon_sockfd, payload, payload_len, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("Failed to send restore request to CH")); + return -1; + } + + /* Restore is a syncronous operationin CH. so, pass false to wait until there's a reponse */ + if (chSocketProcessHttpResponse(mon_sockfd, false) < 0) + return -1; + if (virCHProcessSetup(vm) < 0) return -1; -- 2.34.1

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(a)linux.microsoft.com&gt; --- 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; + 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; } /* 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; } -- 2.34.1