[PATCH 02/10] ch_monitor: Update virCHMonitorGet to handle accept a response

William Douglas posted 10 patches 4 years, 7 months ago
There is a newer version of this series
[PATCH 02/10] ch_monitor: Update virCHMonitorGet to handle accept a response
Posted by William Douglas 4 years, 7 months ago
The virCHMonitorGet function needed to be able to return data from the
hypervisor. This functionality is needed in order for the driver to
support PTY enablement and getting details about the VM state.

Signed-off-by: William Douglas <william.douglas@intel.com>
---
 src/ch/ch_monitor.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index f76bc2b423..93a5f8fdb4 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -612,12 +612,33 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint)
     return ret;
 }
 
+struct curl_data {
+    char *content;
+    size_t size;
+};
+
+static size_t
+curl_callback(void *contents, size_t size, size_t nmemb, void *userp)
+{
+    size_t content_size = size * nmemb;
+    struct curl_data *data = (struct curl_data *)userp;
+
+    data->content = g_malloc0(content_size + 1);
+    memcpy(data->content, contents, content_size);
+    data->content[content_size] = 0;
+    data->size = content_size;
+
+    return content_size;
+}
+
 static int
-virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
+virCHMonitorGet(virCHMonitor *mon, const char *endpoint, virJSONValue **response)
 {
     g_autofree char *url = NULL;
     int responseCode = 0;
     int ret = -1;
+    struct curl_slist *headers = NULL;
+    struct curl_data data;
 
     url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
 
@@ -629,12 +650,24 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
     curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath);
     curl_easy_setopt(mon->handle, CURLOPT_URL, url);
 
+    if (response) {
+        headers = curl_slist_append(headers, "Accept: application/json");
+        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);
+    }
+
     responseCode = virCHMonitorCurlPerform(mon->handle);
 
     virObjectUnlock(mon);
 
-    if (responseCode == 200 || responseCode == 204)
+    if (responseCode == 200 || responseCode == 204) {
         ret = 0;
+        if (response) {
+            *response = virJSONValueFromString(data.content);
+        }
+    }
 
     return ret;
 }
-- 
2.31.1

Re: [PATCH 02/10] ch_monitor: Update virCHMonitorGet to handle accept a response
Posted by Michal Prívozník 4 years, 6 months ago
On 6/30/21 1:05 AM, William Douglas wrote:
> The virCHMonitorGet function needed to be able to return data from the
> hypervisor. This functionality is needed in order for the driver to
> support PTY enablement and getting details about the VM state.
> 
> Signed-off-by: William Douglas <william.douglas@intel.com>
> ---
>  src/ch/ch_monitor.c | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index f76bc2b423..93a5f8fdb4 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -612,12 +612,33 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint)
>      return ret;
>  }
>  
> +struct curl_data {
> +    char *content;
> +    size_t size;
> +};
> +
> +static size_t
> +curl_callback(void *contents, size_t size, size_t nmemb, void *userp)
> +{
> +    size_t content_size = size * nmemb;
> +    struct curl_data *data = (struct curl_data *)userp;
> +
> +    data->content = g_malloc0(content_size + 1);

We prefer g_new(char, ...), but ...

> +    memcpy(data->content, contents, content_size);
> +    data->content[content_size] = 0;
> +    data->size = content_size;
> +
> +    return content_size;

.. are we sure that the incoming data is split into small enough hunks
that this function is called just once? I mean, the other WRITEFUNCTION
callbacks we have in our code are aware of this and pick up where the
previous run dropped.

> +}
> +
>  static int
> -virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
> +virCHMonitorGet(virCHMonitor *mon, const char *endpoint, virJSONValue **response)
>  {
>      g_autofree char *url = NULL;
>      int responseCode = 0;
>      int ret = -1;
> +    struct curl_slist *headers = NULL;
> +    struct curl_data data;
>  
>      url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
>  
> @@ -629,12 +650,24 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
>      curl_easy_setopt(mon->handle, CURLOPT_UNIX_SOCKET_PATH, mon->socketpath);
>      curl_easy_setopt(mon->handle, CURLOPT_URL, url);
>  
> +    if (response) {
> +        headers = curl_slist_append(headers, "Accept: application/json");
> +        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);
> +    }
> +
>      responseCode = virCHMonitorCurlPerform(mon->handle);
>  
>      virObjectUnlock(mon);
>  
> -    if (responseCode == 200 || responseCode == 204)
> +    if (responseCode == 200 || responseCode == 204) {
>          ret = 0;
> +        if (response) {
> +            *response = virJSONValueFromString(data.content);

data.content is not eaten by virJSONValueFromString(). You need to free
it explicitly.

> +        }
> +    }
>  
>      return ret;
>  }
> 

Michal

Re: [PATCH 02/10] ch_monitor: Update virCHMonitorGet to handle accept a response
Posted by Douglas, William 4 years, 6 months ago
On Mon, 2021-07-12 at 14:05 +0200, Michal Prívozník wrote:
> On 6/30/21 1:05 AM, William Douglas wrote:
> 
> > +    memcpy(data->content, contents, content_size);
> > +    data->content[content_size] = 0;
> > +    data->size = content_size;
> > +
> > +    return content_size;
> 
> .. are we sure that the incoming data is split into small enough
> hunks
> that this function is called just once? I mean, the other
> WRITEFUNCTION
> callbacks we have in our code are aware of this and pick up where the
> previous run dropped.

It should always be within 16K but somebody could have a differently
configured curl regardless. Will fix.

> > +            *response = virJSONValueFromString(data.content);
> 
> data.content is not eaten by virJSONValueFromString(). You need to
> free
> it explicitly.

Oops, thanks.