Re: [PATCH 5/8] ch: use monitor socket fd to send restore request

[Praveen K Paladugu <praveenkpaladugu@xxxxxxxxx>]

On 6/28/2024 2:26 AM, Purna Pavan Chandra wrote:
> This change will later help in passing new net fds to CH during
> restore operation.
Please consider rewording this commit message to contain a 'what' and a 
'why'. Something to the effect of:

Instead of curl, use low-level socket connections to restore VMs. This 
is required to pass net FDs to CH while restoring net devices.

> Signed-off-by: Purna Pavan Chandra <paekkaladevi@xxxxxxxxxxxxxxxxxxx>
> ---
>   src/ch/ch_monitor.c | 13 +++++++++++--
>   src/ch/ch_monitor.h |  2 +-
>   src/ch/ch_process.c | 39 +++++++++++++++++++++++++++++++++------
>   3 files changed, 45 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 3f2a3f81e5..b8c6080d5c 100644
> --- a/src/ch/ch_process.c
> +++ b/src/ch/ch_process.c
> @@ -903,6 +903,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 */
> @@ -917,12 +923,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 */
> @@ -933,6 +933,33 @@ 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;
> +    }
> +
> +    if (chSocketProcessHttpResponse(mon_sockfd) < 0)
> +        return -1;
> +
>       if (virCHProcessSetup(vm) < 0)
>           return -1;
>   

--
Regards,
Praveen