12:59 a.m.
This series enables console support in the cloud-hypervisor driver.
Cloud-hypervisor only supports a single console or serial device at a
time, hence the checks to ensure the domain configuration is only
passing one or the other.
Changes since v2:
* Squashed an additional patch.
Changes since v1:
* Added missing patch to add the virChrdev device
* Added handling for multiple curl WRITEFUNCTION call backs
* Added missing free for data.content
* Removed redundant console configuration check
* Improved handling of pty JSON data to make use of g_autoptr
* Squashed two patches
William Douglas (8):
ch_domain: Add virChrdevs for console support
ch_monitor: Make unused function static
ch_monitor: Update virCHMonitorGet to handle accept a response
ch_monitor: Use virCHMonitorGet to access cloud-hypervisor API
ch_monitor: Add pty json builder function
ch_process: Handle enabled console devices
ch_domain: Allow controller and chr devices
ch_driver: Add handler for console API
src/ch/ch_domain.c | 11 ++++-
src/ch/ch_domain.h | 3 ++
src/ch/ch_driver.c | 78 +++++++++++++++++++++++++++++
src/ch/ch_monitor.c | 118 ++++++++++++++++++++++++++++++++++++++++++--
src/ch/ch_monitor.h | 3 ++
src/ch/ch_process.c | 67 +++++++++++++++++++++++++
6 files changed, 274 insertions(+), 6 deletions(-)
--
2.31.1
12:59 a.m.
New subject: [PATCH 1/8] ch_domain: Add virChrdevs for console support
Add and initialize a virChrdevs to the _virCHDomainObjPrivate
structure in order to eventually track the consoles in use by a domain.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_domain.c | 7 +++++++
src/ch/ch_domain.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index 780a46ba00..a6b87e28e5 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -22,6 +22,7 @@
#include "ch_domain.h"
#include "viralloc.h"
+#include "virchrdev.h"
#include "virlog.h"
#include "virtime.h"
@@ -146,6 +147,11 @@ virCHDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED)
return NULL;
}
+ if (!(priv->chrdevs = virChrdevAlloc())) {
+ g_free(priv);
+ return NULL;
+ }
+
return priv;
}
@@ -154,6 +160,7 @@ virCHDomainObjPrivateFree(void *data)
{
virCHDomainObjPrivate *priv = data;
+ virChrdevFree(priv->chrdevs);
virCHDomainObjFreeJob(priv);
g_free(priv);
}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index b4e0d4c212..61b34b0467 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -22,6 +22,7 @@
#include "ch_conf.h"
#include "ch_monitor.h"
+#include "virchrdev.h"
/* Give up waiting for mutex after 30 seconds */
#define CH_JOB_WAIT_TIME (1000ull * 30)
@@ -52,6 +53,8 @@ struct _virCHDomainObjPrivate {
struct virCHDomainJobObj job;
virCHMonitor *monitor;
+
+ virChrdevs *chrdevs;
};
extern virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks;
--
2.31.1
7:32 p.m.
New subject: [PATCH 1/8] ch_domain: Add virChrdevs for console support
On Thu, Aug 26, 2021 at 02:59:15PM -0700, William Douglas wrote:
Add and initialize a virChrdevs to the _virCHDomainObjPrivate
structure in order to eventually track the consoles in use by a domain.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_domain.c | 7 +++++++
src/ch/ch_domain.h | 3 +++
2 files changed, 10 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:59 a.m.
New subject: [PATCH 2/8] ch_monitor: Make unused function static
The virCHMonitorGet function wasn't in use and was declared as
non-static (though not in a header file). This function isn't going to
be used outside of the monitor though so remove the initial
declaration and define the function to be static.
Future work should adjust this function to allow reading VM state
needed for PTY enablement.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 0f8752d1ed..b4bc10bfcf 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -54,7 +54,6 @@ VIR_ONCE_GLOBAL_INIT(virCHMonitor);
int virCHMonitorShutdownVMM(virCHMonitor *mon);
int virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint);
-int virCHMonitorGet(virCHMonitor *mon, const char *endpoint);
static int
virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef)
@@ -612,7 +611,7 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint)
return ret;
}
-int
+static int
virCHMonitorGet(virCHMonitor *mon, const char *endpoint)
{
g_autofree char *url = NULL;
--
2.31.1
7:33 p.m.
New subject: [PATCH 2/8] ch_monitor: Make unused function static
On Thu, Aug 26, 2021 at 02:59:16PM -0700, William Douglas wrote:
The virCHMonitorGet function wasn't in use and was declared as
non-static (though not in a header file). This function isn't going to
be used outside of the monitor though so remove the initial
declaration and define the function to be static.
Future work should adjust this function to allow reading VM state
needed for PTY enablement.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:59 a.m.
New subject: [PATCH 3/8] ch_monitor: Update virCHMonitorGet to handle accept a response
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(a)intel.com>
---
src/ch/ch_monitor.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index b4bc10bfcf..ee7e4683e3 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -611,12 +611,36 @@ 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;
+
+ if (content_size == 0)
+ return content_size;
+
+ data->content = g_realloc(data->content, data->size + content_size);
+
+ memcpy(&(data->content[data->size]), contents, content_size);
+ 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 = {0};
url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
@@ -628,12 +652,28 @@ 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) {
+ data.content = g_realloc(data.content, data.size + 1);
+ data.content[data.size] = 0;
+ *response = virJSONValueFromString(data.content);
+ }
+ }
+
+ g_free(data.content);
return ret;
}
--
2.31.1
7:30 p.m.
New subject: [PATCH 3/8] ch_monitor: Update virCHMonitorGet to handle accept a response
On Thu, Aug 26, 2021 at 02:59:17PM -0700, 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(a)intel.com>
---
src/ch/ch_monitor.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index b4bc10bfcf..ee7e4683e3 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -611,12 +611,36 @@ 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;
+
+ if (content_size == 0)
+ return content_size;
+
+ data->content = g_realloc(data->content, data->size + content_size);
+
+ memcpy(&(data->content[data->size]), contents, content_size);
+ 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 = {0};
url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
@@ -628,12 +652,28 @@ 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);
This bit feels dodgy to me. mon->handle has its lifetime tied to
the virCHMonitor object, but '&data' is allocated on the stack.
IOW, this pointer is persistent, but it goes out of scope.
I guess you're relying on the fact that calls to this method are
serialized, and we re-write the bad pointer on every call, but
if 'response' is NULL on the next call we're not going to do that
re-write.
Can all these options be set unconditionally when the mon->handle
is first created ?
+ }
+
responseCode = virCHMonitorCurlPerform(mon->handle);
virObjectUnlock(mon);
- if (responseCode == 200 || responseCode == 204)
+ if (responseCode == 200 || responseCode == 204) {
ret = 0;
+ if (response) {
+ data.content = g_realloc(data.content, data.size + 1);
+ data.content[data.size] = 0;
+ *response = virJSONValueFromString(data.content);
+ }
+ }
+
+ g_free(data.content);
return ret;
}
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:08 a.m.
New subject: [PATCH 3/8] ch_monitor: Update virCHMonitorGet to handle accept a response
On Fri, 2021-08-27 at 17:30 +0100, Daniel P. Berrangé wrote:
On Thu, Aug 26, 2021 at 02:59:17PM -0700, 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(a)intel.com>
> ---
> src/ch/ch_monitor.c | 44
> ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index b4bc10bfcf..ee7e4683e3 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -611,12 +611,36 @@ 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;
> +
> + if (content_size == 0)
> + return content_size;
> +
> + data->content = g_realloc(data->content, data->size +
> content_size);
> +
> + memcpy(&(data->content[data->size]), contents, content_size);
> + 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 = {0};
>
> url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
>
> @@ -628,12 +652,28 @@ 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);
This bit feels dodgy to me. mon->handle has its lifetime tied to
the virCHMonitor object, but '&data' is allocated on the stack.
IOW, this pointer is persistent, but it goes out of scope.
I guess you're relying on the fact that calls to this method are
serialized, and we re-write the bad pointer on every call, but
if 'response' is NULL on the next call we're not going to do that
re-write.
Can all these options be set unconditionally when the mon->handle
is first created ?
They could be but the use of curl_easy_reset (which is just outside the
context of this patch I'm noticing, sorry) will cause anything we
initialize the handle with on creation to be reset anyway.
All the calls using the handle lock the mon and then do a
curl_easy_reset on the handle prior to use.
I still understand the dislike for having the handle with potentially
bad pointers in it though. Another option would just be having the
handle exist on the stack though we then lose the live connections but
that wouldn't be an issue for this use case.
Thoughts?
> + }
> +
> responseCode = virCHMonitorCurlPerform(mon->handle);
>
> virObjectUnlock(mon);
>
> - if (responseCode == 200 || responseCode == 204)
> + if (responseCode == 200 || responseCode == 204) {
> ret = 0;
> + if (response) {
> + data.content = g_realloc(data.content, data.size + 1);
> + data.content[data.size] = 0;
> + *response = virJSONValueFromString(data.content);
> + }
> + }
> +
> + g_free(data.content);
>
> return ret;
> }
Regards,
Daniel
11:45 a.m.
New subject: [PATCH 3/8] ch_monitor: Update virCHMonitorGet to handle accept a response
On Tue, Aug 31, 2021 at 09:08:21PM +0000, Douglas, William wrote:
On Fri, 2021-08-27 at 17:30 +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 26, 2021 at 02:59:17PM -0700, 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(a)intel.com>
> > ---
> > src/ch/ch_monitor.c | 44
> > ++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 42 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> > index b4bc10bfcf..ee7e4683e3 100644
> > --- a/src/ch/ch_monitor.c
> > +++ b/src/ch/ch_monitor.c
> > @@ -611,12 +611,36 @@ 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;
> > +
> > + if (content_size == 0)
> > + return content_size;
> > +
> > + data->content = g_realloc(data->content, data->size +
> > content_size);
> > +
> > + memcpy(&(data->content[data->size]), contents, content_size);
> > + 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 = {0};
> >
> > url = g_strdup_printf("%s/%s", URL_ROOT, endpoint);
> >
> > @@ -628,12 +652,28 @@ 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);
>
> This bit feels dodgy to me. mon->handle has its lifetime tied to
> the virCHMonitor object, but '&data' is allocated on the stack.
> IOW, this pointer is persistent, but it goes out of scope.
>
> I guess you're relying on the fact that calls to this method are
> serialized, and we re-write the bad pointer on every call, but
> if 'response' is NULL on the next call we're not going to do that
> re-write.
>
> Can all these options be set unconditionally when the mon->handle
> is first created ?
They could be but the use of curl_easy_reset (which is just outside the
context of this patch I'm noticing, sorry) will cause anything we
initialize the handle with on creation to be reset anyway.
All the calls using the handle lock the mon and then do a
curl_easy_reset on the handle prior to use.
I still understand the dislike for having the handle with potentially
bad pointers in it though. Another option would just be having the
handle exist on the stack though we then lose the live connections but
that wouldn't be an issue for this use case.
Thoughts?
I'd be happy if we just cleared the bad point at the end of this
method, or even just added another call to easy_reset at end of
this method.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:59 a.m.
New subject: [PATCH 4/8] ch_monitor: Use virCHMonitorGet to access cloud-hypervisor API
Now that virCHMonitorGet is capable of handling data returned by the
cloud-hypervisor API, make use of this via virCHMonitorGetInfo to call
into the vm.info endpoint.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 15 +++++++++++++++
src/ch/ch_monitor.h | 3 +++
2 files changed, 18 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index ee7e4683e3..28d1c213cc 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -751,3 +751,18 @@ virCHMonitorResumeVM(virCHMonitor *mon)
{
return virCHMonitorPutNoContent(mon, URL_VM_RESUME);
}
+
+/**
+ * virCHMonitorGetInfo:
+ * @mon: Pointer to the monitor
+ * @info: Get VM info
+ *
+ * Retrive the VM info and store in @info
+ *
+ * Returns 0 on success.
+ */
+int
+virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue **info)
+{
+ return virCHMonitorGet(mon, URL_VM_INFO, info);
+}
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index e717e11cbc..e39b4eb8b2 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -23,6 +23,7 @@
#include <curl/curl.h>
#include "virobject.h"
+#include "virjson.h"
#include "domain_conf.h"
#define URL_ROOT "http://localhost/api/v1"
@@ -34,6 +35,7 @@
#define URL_VM_REBOOT "vm.reboot"
#define URL_VM_Suspend "vm.pause"
#define URL_VM_RESUME "vm.resume"
+#define URL_VM_INFO "vm.info"
typedef struct _virCHMonitor virCHMonitor;
@@ -58,3 +60,4 @@ int virCHMonitorShutdownVM(virCHMonitor *mon);
int virCHMonitorRebootVM(virCHMonitor *mon);
int virCHMonitorSuspendVM(virCHMonitor *mon);
int virCHMonitorResumeVM(virCHMonitor *mon);
+int virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue **info);
--
2.31.1
7:34 p.m.
New subject: [PATCH 4/8] ch_monitor: Use virCHMonitorGet to access cloud-hypervisor API
On Thu, Aug 26, 2021 at 02:59:18PM -0700, William Douglas wrote:
Now that virCHMonitorGet is capable of handling data returned by the
cloud-hypervisor API, make use of this via virCHMonitorGetInfo to call
into the vm.info endpoint.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 15 +++++++++++++++
src/ch/ch_monitor.h | 3 +++
2 files changed, 18 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:59 a.m.
New subject: [PATCH 5/8] ch_monitor: Add pty json builder function
Add function to build the the json structure to configure a PTY in
cloud-hypervisor. The configuration only supports a single serial or
console device.
The devices themselves still aren't allowed in configurations yet
though.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 28d1c213cc..1ff956b61e 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -89,6 +89,59 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef)
return -1;
}
+static int
+virCHMonitorBuildPTYJson(virJSONValue *content, virDomainDef *vmdef)
+{
+ virJSONValue *ptys = virJSONValueNewObject();
+
+ if ((vmdef->nconsoles &&
+ vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY)
+ && (vmdef->nserials &&
+ vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single console or serial can be configured for this
domain"));
+ return -1;
+ } else if (vmdef->nconsoles > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single console can be configured for this
domain"));
+ return -1;
+ } else if (vmdef->nserials > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single serial can be configured for this
domain"));
+ return -1;
+ }
+
+ if (vmdef->nconsoles) {
+ g_autoptr(virJSONValue) pty = virJSONValueNewObject();
+ if (vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(pty, "mode", "Pty")
< 0)
+ return -1;
+ if (virJSONValueObjectAppend(content, "console", &pty) < 0)
+ return -1;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Console can only be enabled for a PTY"));
+ return -1;
+ }
+ }
+
+ if (vmdef->nserials) {
+ g_autoptr(virJSONValue) pty = virJSONValueNewObject();
+ if (vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(ptys, "mode", "Pty")
< 0)
+ return -1;
+ if (virJSONValueObjectAppend(content, "serial", &pty) < 0)
+ return -1;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Serial can only be enabled for a PTY"));
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static int
virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef)
{
@@ -370,6 +423,9 @@ virCHMonitorBuildVMJson(virDomainDef *vmdef, char **jsonstr)
goto cleanup;
}
+ if (virCHMonitorBuildPTYJson(content, vmdef) < 0)
+ goto cleanup;
+
if (virCHMonitorBuildCPUJson(content, vmdef) < 0)
goto cleanup;
--
2.31.1
7:38 p.m.
New subject: [PATCH 5/8] ch_monitor: Add pty json builder function
On Thu, Aug 26, 2021 at 02:59:19PM -0700, William Douglas wrote:
Add function to build the the json structure to configure a PTY in
cloud-hypervisor. The configuration only supports a single serial or
console device.
The devices themselves still aren't allowed in configurations yet
though.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 28d1c213cc..1ff956b61e 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -89,6 +89,59 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef)
return -1;
}
+static int
+virCHMonitorBuildPTYJson(virJSONValue *content, virDomainDef *vmdef)
+{
+ virJSONValue *ptys = virJSONValueNewObject();
+
+ if ((vmdef->nconsoles &&
+ vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY)
+ && (vmdef->nserials &&
+ vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single console or serial can be configured for
this domain"));
+ return -1;
+ } else if (vmdef->nconsoles > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single console can be configured for this
domain"));
+ return -1;
+ } else if (vmdef->nserials > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single serial can be configured for this
domain"));
+ return -1;
+ }
IIRC, we'd recommend that these checks go into the 'deviceValidateCallback'
impl that you have in ch_domain.c, so that they get run at time the user
loads the XML into libvirt, rather than when the Vm is started.
The code below can still safely assume the checks have been done by the
validate function.
+
+ if (vmdef->nconsoles) {
+ g_autoptr(virJSONValue) pty = virJSONValueNewObject();
+ if (vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(pty, "mode", "Pty")
< 0)
+ return -1;
+ if (virJSONValueObjectAppend(content, "console", &pty) <
0)
+ return -1;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Console can only be enabled for a PTY"));
+ return -1;
+ }
The check for type != PTY ought to be in the above validation block
too, so this code then just assumes PTY.
+ }
+
+ if (vmdef->nserials) {
+ g_autoptr(virJSONValue) pty = virJSONValueNewObject();
+ if (vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(ptys, "mode", "Pty")
< 0)
+ return -1;
+ if (virJSONValueObjectAppend(content, "serial", &pty) < 0)
+ return -1;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Serial can only be enabled for a PTY"));
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
static int
virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef)
{
@@ -370,6 +423,9 @@ virCHMonitorBuildVMJson(virDomainDef *vmdef, char **jsonstr)
goto cleanup;
}
+ if (virCHMonitorBuildPTYJson(content, vmdef) < 0)
+ goto cleanup;
+
if (virCHMonitorBuildCPUJson(content, vmdef) < 0)
goto cleanup;
--
2.31.1
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1:35 a.m.
New subject: [PATCH 5/8] ch_monitor: Add pty json builder function
On Fri, 2021-08-27 at 17:38 +0100, Daniel P. Berrangé wrote:
On Thu, Aug 26, 2021 at 02:59:19PM -0700, William Douglas wrote:
> Add function to build the the json structure to configure a PTY in
> cloud-hypervisor. The configuration only supports a single serial
> or
> console device.
>
> The devices themselves still aren't allowed in configurations yet
> though.
>
> Signed-off-by: William Douglas <william.douglas(a)intel.com>
> ---
> src/ch/ch_monitor.c | 56
> +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index 28d1c213cc..1ff956b61e 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -89,6 +89,59 @@ virCHMonitorBuildCPUJson(virJSONValue *content,
> virDomainDef *vmdef)
> return -1;
> }
>
> +static int
> +virCHMonitorBuildPTYJson(virJSONValue *content, virDomainDef
> *vmdef)
> +{
> + virJSONValue *ptys = virJSONValueNewObject();
> +
> + if ((vmdef->nconsoles &&
> + vmdef->consoles[0]->source->type ==
> VIR_DOMAIN_CHR_TYPE_PTY)
> + && (vmdef->nserials &&
> + vmdef->serials[0]->source->type ==
> VIR_DOMAIN_CHR_TYPE_PTY)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Only a single console or serial can be
> configured for this domain"));
> + return -1;
> + } else if (vmdef->nconsoles > 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Only a single console can be configured
> for this domain"));
> + return -1;
> + } else if (vmdef->nserials > 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Only a single serial can be configured
> for this domain"));
> + return -1;
> + }
IIRC, we'd recommend that these checks go into the
'deviceValidateCallback'
impl that you have in ch_domain.c, so that they get run at time the
user
loads the XML into libvirt, rather than when the Vm is started.
Looking a little at the qemu code I was curious if this should be in
the domainValidateCallback instead?
The code below can still safely assume the checks have been done by
the
validate function.
> +
> + if (vmdef->nconsoles) {
> + g_autoptr(virJSONValue) pty = virJSONValueNewObject();
> + if (vmdef->consoles[0]->source->type ==
> VIR_DOMAIN_CHR_TYPE_PTY) {
> + if (virJSONValueObjectAppendString(pty, "mode",
"Pty")
> < 0)
> + return -1;
> + if (virJSONValueObjectAppend(content, "console", &pty)
> < 0)
> + return -1;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Console can only be enabled for a
> PTY"));
> + return -1;
> + }
The check for type != PTY ought to be in the above validation block
too, so this code then just assumes PTY.
> + }
> +
> + if (vmdef->nserials) {
> + g_autoptr(virJSONValue) pty = virJSONValueNewObject();
> + if (vmdef->serials[0]->source->type ==
> VIR_DOMAIN_CHR_TYPE_PTY) {
> + if (virJSONValueObjectAppendString(ptys, "mode",
> "Pty") < 0)
> + return -1;
> + if (virJSONValueObjectAppend(content, "serial", &pty)
> < 0)
> + return -1;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Serial can only be enabled for a
> PTY"));
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int
> virCHMonitorBuildKernelRelatedJson(virJSONValue *content,
> virDomainDef *vmdef)
> {
> @@ -370,6 +423,9 @@ virCHMonitorBuildVMJson(virDomainDef *vmdef,
> char **jsonstr)
> goto cleanup;
> }
>
> + if (virCHMonitorBuildPTYJson(content, vmdef) < 0)
> + goto cleanup;
> +
> if (virCHMonitorBuildCPUJson(content, vmdef) < 0)
> goto cleanup;
>
> --
> 2.31.1
>
Regards,
Daniel
12:59 a.m.
New subject: [PATCH 6/8] ch_process: Handle enabled console devices
Add functionality to allow libvirt console to connect to the
cloud-hypervisor created PTY associated with a VM by updating the
domain with console path information. This has to be run after the VM
is created by cloud-hypervisor.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_process.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 93b1f7f97e..f6ae1677fd 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -28,6 +28,7 @@
#include "ch_process.h"
#include "viralloc.h"
#include "virerror.h"
+#include "virjson.h"
#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_CH
@@ -52,6 +53,69 @@ virCHProcessConnectMonitor(virCHDriver *driver,
return monitor;
}
+static void
+virCHProcessUpdateConsoleDevice(virDomainObj *vm,
+ virJSONValue *config,
+ const char *device)
+{
+ const char *path;
+ virDomainChrDef *chr = NULL;
+ virJSONValue *dev, *file;
+
+ if (!config)
+ return;
+
+ dev = virJSONValueObjectGet(config, device);
+ if (!dev)
+ return;
+
+ file = virJSONValueObjectGet(dev, "file");
+ if (!file)
+ return;
+
+ path = virJSONValueGetString(file);
+ if (!path)
+ return;
+
+ if (STREQ(device, "console")) {
+ chr = vm->def->consoles[0];
+ } else if (STREQ(device, "serial")) {
+ chr = vm->def->serials[0];
+ }
+
+ if (chr && chr->source)
+ chr->source->data.file.path = g_strdup(path);
+}
+
+static void
+virCHProcessUpdateConsole(virDomainObj *vm,
+ virJSONValue *info)
+{
+ virJSONValue *config;
+
+ config = virJSONValueObjectGet(info, "config");
+ if (!config)
+ return;
+
+ virCHProcessUpdateConsoleDevice(vm, config, "console");
+ virCHProcessUpdateConsoleDevice(vm, config, "serial");
+}
+
+static int
+virCHProcessUpdateInfo(virDomainObj *vm)
+{
+ virJSONValue *info;
+ virCHDomainObjPrivate *priv = vm->privateData;
+ if (virCHMonitorGetInfo(priv->monitor, &info) < 0)
+ return -1;
+
+ virCHProcessUpdateConsole(vm, info);
+
+ virJSONValueFree(info);
+
+ return 0;
+}
+
/**
* virCHProcessStart:
* @driver: pointer to driver structure
@@ -92,6 +156,9 @@ int virCHProcessStart(virCHDriver *driver,
vm->pid = priv->monitor->pid;
vm->def->id = vm->pid;
+
+ virCHProcessUpdateInfo(vm);
+
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason);
return 0;
--
2.31.1
7:40 p.m.
New subject: [PATCH 6/8] ch_process: Handle enabled console devices
On Thu, Aug 26, 2021 at 02:59:20PM -0700, William Douglas wrote:
Add functionality to allow libvirt console to connect to the
cloud-hypervisor created PTY associated with a VM by updating the
domain with console path information. This has to be run after the VM
is created by cloud-hypervisor.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_process.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 93b1f7f97e..f6ae1677fd 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -28,6 +28,7 @@
#include "ch_process.h"
#include "viralloc.h"
#include "virerror.h"
+#include "virjson.h"
#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_CH
@@ -52,6 +53,69 @@ virCHProcessConnectMonitor(virCHDriver *driver,
return monitor;
}
+static void
+virCHProcessUpdateConsoleDevice(virDomainObj *vm,
+ virJSONValue *config,
+ const char *device)
+{
+ const char *path;
+ virDomainChrDef *chr = NULL;
+ virJSONValue *dev, *file;
+
+ if (!config)
+ return;
+
+ dev = virJSONValueObjectGet(config, device);
+ if (!dev)
+ return;
+
+ file = virJSONValueObjectGet(dev, "file");
+ if (!file)
+ return;
+
+ path = virJSONValueGetString(file);
+ if (!path)
+ return;
+
+ if (STREQ(device, "console")) {
+ chr = vm->def->consoles[0];
+ } else if (STREQ(device, "serial")) {
+ chr = vm->def->serials[0];
+ }
+
+ if (chr && chr->source)
+ chr->source->data.file.path = g_strdup(path);
+}
All the error paths here are silent. I wonder if we shouldn't be
reporting proper errors if the data in the JSON reply doesn't
match the config we passed into cloudhypervisor when creating
the VM
+
+static void
+virCHProcessUpdateConsole(virDomainObj *vm,
+ virJSONValue *info)
+{
+ virJSONValue *config;
+
+ config = virJSONValueObjectGet(info, "config");
+ if (!config)
+ return;
+
+ virCHProcessUpdateConsoleDevice(vm, config, "console");
+ virCHProcessUpdateConsoleDevice(vm, config, "serial");
+}
+
+static int
+virCHProcessUpdateInfo(virDomainObj *vm)
+{
+ virJSONValue *info;
+ virCHDomainObjPrivate *priv = vm->privateData;
+ if (virCHMonitorGetInfo(priv->monitor, &info) < 0)
+ return -1;
+
+ virCHProcessUpdateConsole(vm, info);
+
+ virJSONValueFree(info);
+
+ return 0;
+}
+
/**
* virCHProcessStart:
* @driver: pointer to driver structure
@@ -92,6 +156,9 @@ int virCHProcessStart(virCHDriver *driver,
vm->pid = priv->monitor->pid;
vm->def->id = vm->pid;
+
+ virCHProcessUpdateInfo(vm);
+
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason);
return 0;
--
2.31.1
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:59 a.m.
New subject: [PATCH 7/8] ch_domain: Allow controller and chr devices
With the console and serial device handling fully functional, allow
the required device types to be specified in the domain
configuration.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index a6b87e28e5..89a8ad8f45 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -215,6 +215,8 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
case VIR_DOMAIN_DEVICE_NET:
case VIR_DOMAIN_DEVICE_MEMORY:
case VIR_DOMAIN_DEVICE_VSOCK:
+ case VIR_DOMAIN_DEVICE_CONTROLLER:
+ case VIR_DOMAIN_DEVICE_CHR:
break;
case VIR_DOMAIN_DEVICE_LEASE:
@@ -224,12 +226,10 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_HOSTDEV:
case VIR_DOMAIN_DEVICE_WATCHDOG:
- case VIR_DOMAIN_DEVICE_CONTROLLER:
case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_SMARTCARD:
- case VIR_DOMAIN_DEVICE_CHR:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
case VIR_DOMAIN_DEVICE_RNG:
--
2.31.1
7:41 p.m.
New subject: [PATCH 7/8] ch_domain: Allow controller and chr devices
On Thu, Aug 26, 2021 at 02:59:21PM -0700, William Douglas wrote:
With the console and serial device handling fully functional, allow
the required device types to be specified in the domain
configuration.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index a6b87e28e5..89a8ad8f45 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -215,6 +215,8 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
case VIR_DOMAIN_DEVICE_NET:
case VIR_DOMAIN_DEVICE_MEMORY:
case VIR_DOMAIN_DEVICE_VSOCK:
+ case VIR_DOMAIN_DEVICE_CONTROLLER:
+ case VIR_DOMAIN_DEVICE_CHR:
break;
case VIR_DOMAIN_DEVICE_LEASE:
@@ -224,12 +226,10 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
case VIR_DOMAIN_DEVICE_VIDEO:
case VIR_DOMAIN_DEVICE_HOSTDEV:
case VIR_DOMAIN_DEVICE_WATCHDOG:
- case VIR_DOMAIN_DEVICE_CONTROLLER:
case VIR_DOMAIN_DEVICE_GRAPHICS:
case VIR_DOMAIN_DEVICE_HUB:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_SMARTCARD:
- case VIR_DOMAIN_DEVICE_CHR:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
case VIR_DOMAIN_DEVICE_RNG:
This method is in fact where the validation ought to go that was mentioned
in an earlier patch.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:59 a.m.
New subject: [PATCH 8/8] ch_driver: Add handler for console API
Enable the handler function to find and open the console character
device that will be used by the console API.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_driver.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index ac958d73a8..a11357f83a 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -30,6 +30,7 @@
#include "viraccessapicheck.h"
#include "viralloc.h"
#include "virbuffer.h"
+#include "virchrdev.h"
#include "vircommand.h"
#include "virerror.h"
#include "virfile.h"
@@ -816,6 +817,82 @@ static int chDomainGetInfo(virDomainPtr dom,
return ret;
}
+static int
+chDomainOpenConsole(virDomainPtr dom,
+ const char *dev_name,
+ virStreamPtr st,
+ unsigned int flags)
+{
+ virDomainObj *vm = NULL;
+ int ret = -1;
+ size_t i;
+ virDomainChrDef *chr = NULL;
+ virCHDomainObjPrivate *priv;
+
+ virCheckFlags(VIR_DOMAIN_CONSOLE_SAFE | VIR_DOMAIN_CONSOLE_FORCE, -1);
+
+ if (!(vm = chDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (dev_name) {
+ for (i = 0; !chr && i < vm->def->nconsoles; i++) {
+ if (vm->def->consoles[i]->info.alias &&
+ STREQ(dev_name, vm->def->consoles[i]->info.alias))
+ chr = vm->def->consoles[i];
+ }
+ for (i = 0; !chr && i < vm->def->nserials; i++) {
+ if (STREQ(dev_name, vm->def->serials[i]->info.alias))
+ chr = vm->def->serials[i];
+ }
+ for (i = 0; !chr && i < vm->def->nparallels; i++) {
+ if (STREQ(dev_name, vm->def->parallels[i]->info.alias))
+ chr = vm->def->parallels[i];
+ }
+ } else {
+ if (vm->def->nconsoles &&
+ vm->def->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY)
+ chr = vm->def->consoles[0];
+ else if (vm->def->nserials &&
+ vm->def->serials[0]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY)
+ chr = vm->def->serials[0];
+ }
+
+ if (!chr) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find character device
%s"),
+ NULLSTR(dev_name));
+ goto cleanup;
+ }
+
+ if (chr->source->type != VIR_DOMAIN_CHR_TYPE_PTY) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("character device %s is not using a PTY"),
+ dev_name ? dev_name : NULLSTR(chr->info.alias));
+ goto cleanup;
+ }
+
+ /* handle mutually exclusive access to console devices */
+ ret = virChrdevOpen(priv->chrdevs, chr->source, st,
+ (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0);
+
+ if (ret == 1) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Active console session exists for this domain"));
+ ret = -1;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
static int chStateCleanup(void)
{
if (ch_driver == NULL)
@@ -913,6 +990,7 @@ static virHypervisorDriver chHypervisorDriver = {
.domainGetXMLDesc = chDomainGetXMLDesc, /* 7.5.0 */
.domainGetInfo = chDomainGetInfo, /* 7.5.0 */
.domainIsActive = chDomainIsActive, /* 7.5.0 */
+ .domainOpenConsole = chDomainOpenConsole, /* 7.6.0 */
.nodeGetInfo = chNodeGetInfo, /* 7.5.0 */
};
--
2.31.1
7:45 p.m.
New subject: [PATCH 8/8] ch_driver: Add handler for console API
On Thu, Aug 26, 2021 at 02:59:22PM -0700, William Douglas wrote:
Enable the handler function to find and open the console character
device that will be used by the console API.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_driver.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index ac958d73a8..a11357f83a 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -30,6 +30,7 @@
#include "viraccessapicheck.h"
#include "viralloc.h"
#include "virbuffer.h"
+#include "virchrdev.h"
#include "vircommand.h"
#include "virerror.h"
#include "virfile.h"
@@ -816,6 +817,82 @@ static int chDomainGetInfo(virDomainPtr dom,
return ret;
}
+static int
+chDomainOpenConsole(virDomainPtr dom,
+ const char *dev_name,
+ virStreamPtr st,
+ unsigned int flags)
+{
+ virDomainObj *vm = NULL;
+ int ret = -1;
+ size_t i;
+ virDomainChrDef *chr = NULL;
+ virCHDomainObjPrivate *priv;
+
+ virCheckFlags(VIR_DOMAIN_CONSOLE_SAFE | VIR_DOMAIN_CONSOLE_FORCE, -1);
+
+ if (!(vm = chDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (virDomainObjCheckActive(vm) < 0)
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (dev_name) {
+ for (i = 0; !chr && i < vm->def->nconsoles; i++) {
+ if (vm->def->consoles[i]->info.alias &&
+ STREQ(dev_name, vm->def->consoles[i]->info.alias))
+ chr = vm->def->consoles[i];
+ }
+ for (i = 0; !chr && i < vm->def->nserials; i++) {
+ if (STREQ(dev_name, vm->def->serials[i]->info.alias))
+ chr = vm->def->serials[i];
+ }
+ for (i = 0; !chr && i < vm->def->nparallels; i++) {
+ if (STREQ(dev_name, vm->def->parallels[i]->info.alias))
+ chr = vm->def->parallels[i];
+ }
You can drop the parallels loop since your driver doesn't
support that IIUC.
+ } else {
+ if (vm->def->nconsoles &&
+ vm->def->consoles[0]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY)
+ chr = vm->def->consoles[0];
+ else if (vm->def->nserials &&
+ vm->def->serials[0]->source->type ==
VIR_DOMAIN_CHR_TYPE_PTY)
+ chr = vm->def->serials[0];
+ }
+
+ if (!chr) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find character device
%s"),
+ NULLSTR(dev_name));
+ goto cleanup;
+ }
+
+ if (chr->source->type != VIR_DOMAIN_CHR_TYPE_PTY) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("character device %s is not using a PTY"),
+ dev_name ? dev_name : NULLSTR(chr->info.alias));
+ goto cleanup;
+ }
+
+ /* handle mutually exclusive access to console devices */
+ ret = virChrdevOpen(priv->chrdevs, chr->source, st,
+ (flags & VIR_DOMAIN_CONSOLE_FORCE) != 0);
+
+ if (ret == 1) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Active console session exists for this domain"));
+ ret = -1;
+ }
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
static int chStateCleanup(void)
{
if (ch_driver == NULL)
@@ -913,6 +990,7 @@ static virHypervisorDriver chHypervisorDriver = {
.domainGetXMLDesc = chDomainGetXMLDesc, /* 7.5.0 */
.domainGetInfo = chDomainGetInfo, /* 7.5.0 */
.domainIsActive = chDomainIsActive, /* 7.5.0 */
+ .domainOpenConsole = chDomainOpenConsole, /* 7.6.0 */
.nodeGetInfo = chNodeGetInfo, /* 7.5.0 */
};
--
2.31.1
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1287
days inactive
1293
days old
19 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Douglas, William -
William Douglas