[PATCH 07/13] ch: add virCHMonitorPut function

Stefan Kober posted 13 patches 1 week, 3 days ago
There is a newer version of this series
[PATCH 07/13] ch: add virCHMonitorPut function
Posted by Stefan Kober 1 week, 3 days ago
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
Re: [PATCH 07/13] ch: add virCHMonitorPut function
Posted by Michal Prívozník via Devel 6 days, 10 hours ago
On 8/28/25 14:54, Stefan Kober wrote:
> 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);

One argument per line please. Oh, and we like pointers to be declared as:

   virJSONValue **answer

instead of

   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");

I believe here you want "Accept: application/json" put inside 'headers',
because if 'payload' is NULL, we are not sending anything, thus
'Content-Type' header field makes no sense.

> +
> +    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");

And here you want the 'Content-type: application/json' header field.

Now, this got me thinking, since we set here 'application/json' type
regardless of the actual 'payload' data type, would it make sense to
turn 'payload' from 'const char *' to 'virJSONValue *'?

This branch would then call virJSONValueToString().

> +    }
> +
> +    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);

I think data.content needs to be freed after this, no matter what.
Otherwise it'll leak.

> +
> +    return 0;
> +}
> +
>  int
>  virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint,
>                           domainLogContext *logCtxt)

Michal
Re: [PATCH 07/13] ch: add virCHMonitorPut function
Posted by stefan.kober@cyberus-technology.de 3 days, 16 hours ago
Very good catches!

I have seen there are builds enabling the address sanitizers. Is there any good workflow to find leaks in a structured way? Can I enable address sanitizer in a libvirt build and just run some tests on that to see if anything leaks or are there things preventing that?