6:05 p.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.
I wasn't sure if the controller device should have some additonal
validation handling so I just did the minimal to get the console open
functioning.
William Douglas (10):
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_monitor: Make use of the PTY json builder
ch_process: Handle enabled console devices
ch_process: Update the domain with console path information
ch_domain: Allow controller and chr devices
ch_driver: Add handler for console API
ch_driver: Turn on the domainOpenConsole API
src/ch/ch_domain.c | 4 +-
src/ch/ch_driver.c | 78 +++++++++++++++++++++++++++++
src/ch/ch_monitor.c | 117 ++++++++++++++++++++++++++++++++++++++++++--
src/ch/ch_monitor.h | 3 ++
src/ch/ch_process.c | 67 +++++++++++++++++++++++++
5 files changed, 263 insertions(+), 6 deletions(-)
--
2.31.1
6:05 p.m.
New subject: [PATCH 01/10] 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 1648d05017..f76bc2b423 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)
@@ -613,7 +612,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
6:05 p.m.
New subject: [PATCH 02/10] 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 | 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
7:05 a.m.
New subject: [PATCH 02/10] ch_monitor: Update virCHMonitorGet to handle accept a response
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(a)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
5:19 p.m.
New subject: [PATCH 02/10] ch_monitor: Update virCHMonitorGet to handle accept a response
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.
6:05 p.m.
New subject: [PATCH 03/10] 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 93a5f8fdb4..5e7d6e3189 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -745,3 +745,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
6:05 p.m.
New subject: [PATCH 04/10] 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.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 5e7d6e3189..d4289b75ce 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -89,6 +89,65 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef)
return -1;
}
+static int
+virCHMonitorBuildPTYJson(virJSONValue *content, virDomainDef *vmdef)
+{
+ virJSONValue *ptyc = virJSONValueNewObject();
+ virJSONValue *ptys = virJSONValueNewObject();
+
+ if (vmdef->nconsoles || vmdef->nserials) {
+ 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"));
+ goto cleanup;
+ } else if (vmdef->nconsoles > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single console can be configured for this
domain"));
+ goto cleanup;
+ } else if (vmdef->nserials > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single serial can be configured for this
domain"));
+ goto cleanup;
+ }
+ }
+
+ if (vmdef->nconsoles) {
+ if (vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(ptyc, "mode", "Pty")
< 0)
+ goto cleanup;
+ if (virJSONValueObjectAppend(content, "console", &ptyc) <
0)
+ goto cleanup;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Console can only be enabled for a PTY"));
+ goto cleanup;
+ }
+ }
+
+ if (vmdef->nserials) {
+ if (vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(ptys, "mode", "Pty")
< 0)
+ goto cleanup;
+ if (virJSONValueObjectAppend(content, "serial", &ptys) < 0)
+ goto cleanup;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Serial can only be enabled for a PTY"));
+ goto cleanup;
+ }
+ }
+
+ return 0;
+
+ cleanup:
+ virJSONValueFree(ptyc);
+ virJSONValueFree(ptys);
+ return -1;
+}
+
static int
virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef)
{
--
2.31.1
7:05 a.m.
New subject: [PATCH 04/10] ch_monitor: Add pty json builder function
On 6/30/21 1:05 AM, 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.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 5e7d6e3189..d4289b75ce 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -89,6 +89,65 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef)
return -1;
}
+static int
+virCHMonitorBuildPTYJson(virJSONValue *content, virDomainDef *vmdef)
+{
+ virJSONValue *ptyc = virJSONValueNewObject();
+ virJSONValue *ptys = virJSONValueNewObject();
+
+ if (vmdef->nconsoles || vmdef->nserials) {
This ^^^ looks redundant since you explicitly check for problematic
configuration below.
+ 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"));
+ goto cleanup;
+ } else if (vmdef->nconsoles > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single console can be configured for this
domain"));
+ goto cleanup;
+ } else if (vmdef->nserials > 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Only a single serial can be configured for this
domain"));
+ goto cleanup;
+ }
+ }
+
+ if (vmdef->nconsoles) {
+ if (vmdef->consoles[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(ptyc, "mode", "Pty")
< 0)
+ goto cleanup;
+ if (virJSONValueObjectAppend(content, "console", &ptyc) <
0)
+ goto cleanup;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Console can only be enabled for a PTY"));
+ goto cleanup;
+ }
+ }
+
+ if (vmdef->nserials) {
+ if (vmdef->serials[0]->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+ if (virJSONValueObjectAppendString(ptys, "mode", "Pty")
< 0)
+ goto cleanup;
+ if (virJSONValueObjectAppend(content, "serial", &ptys) <
0)
+ goto cleanup;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Serial can only be enabled for a PTY"));
+ goto cleanup;
+ }
+ }
+
+ return 0;
+
+ cleanup:
+ virJSONValueFree(ptyc);
+ virJSONValueFree(ptys);
You can avoid these explicit calls to virJSONValueFree() if you declare
those variables as g_autoptr() like this:
g_autoptr(virJSONValue) ptyc = NULL;
g_autoptr(virJSONValue) pyts = NULL;
and you can even do that in the inner most blocks where the vars are used.
+ return -1;
Then, this return -1 can be at all those places where 'goto cleanup' is.
+}
+
static int
virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef)
{
Michal
6:05 p.m.
New subject: [PATCH 05/10] ch_monitor: Make use of the PTY json builder
Call into the PTY json builder for configured serial and console
devices though the devices themselves still aren't allowed in
configuration.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index d4289b75ce..2ade6967fb 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -429,6 +429,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:05 a.m.
New subject: [PATCH 05/10] ch_monitor: Make use of the PTY json builder
On 6/30/21 1:05 AM, William Douglas wrote:
Call into the PTY json builder for configured serial and console
devices though the devices themselves still aren't allowed in
configuration.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_monitor.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index d4289b75ce..2ade6967fb 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -429,6 +429,9 @@ virCHMonitorBuildVMJson(virDomainDef *vmdef, char **jsonstr)
goto cleanup;
}
+ if (virCHMonitorBuildPTYJson(content, vmdef) < 0)
+ goto cleanup;
+
if (virCHMonitorBuildCPUJson(content, vmdef) < 0)
goto cleanup;
This can be merged with the previous patch.
Michal
6:05 p.m.
New subject: [PATCH 06/10] ch_process: Handle enabled console devices
Add functionality to allow libvirt console to connect to the
cloud-hypervisor created PTY associated with a VM. This will need to
be run once the VM is created by cloud-hypervisor.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_process.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 93b1f7f97e..90344f14ea 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
--
2.31.1
6:05 p.m.
New subject: [PATCH 07/10] ch_process: Update the domain with console path information
Add call to update the domain with console path information from
cloud-hypervisor as part of the last stages of initializing the
domain.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_process.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 90344f14ea..f6ae1677fd 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -156,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
6:05 p.m.
New subject: [PATCH 08/10] 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
6:05 p.m.
New subject: [PATCH 09/10] ch_driver: Add handler for console API
Add 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 | 77 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index ac958d73a8..b0b8e19120 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)
--
2.31.1
7:05 a.m.
New subject: [PATCH 09/10] ch_driver: Add handler for console API
On 6/30/21 1:05 AM, William Douglas wrote:
Add 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 | 77 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index ac958d73a8..b0b8e19120 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);
priv->chrdevs doesn't exist. You need to define the struct member and
then initialize it using virChrdevAlloc() and free using virChrdevFree().
+
+ 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)
6:05 p.m.
New subject: [PATCH 10/10] ch_driver: Turn on the domainOpenConsole API
With all the rest of the enablement work out of the way, add the final
call for the cloud-hypervisor driver to handle domainOpenConsole.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index b0b8e19120..a11357f83a 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -990,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:05 a.m.
New subject: [PATCH 10/10] ch_driver: Turn on the domainOpenConsole API
On 6/30/21 1:05 AM, William Douglas wrote:
With all the rest of the enablement work out of the way, add the
final
call for the cloud-hypervisor driver to handle domainOpenConsole.
Signed-off-by: William Douglas <william.douglas(a)intel.com>
---
src/ch/ch_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index b0b8e19120..a11357f83a 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -990,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 */
};
Again, this can be merged into previous patch.
Michal
7:05 a.m.
On 6/30/21 1:05 AM, William Douglas wrote:
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.
I wasn't sure if the controller device should have some additonal
validation handling so I just did the minimal to get the console open
functioning.
William Douglas (10):
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_monitor: Make use of the PTY json builder
ch_process: Handle enabled console devices
ch_process: Update the domain with console path information
ch_domain: Allow controller and chr devices
ch_driver: Add handler for console API
ch_driver: Turn on the domainOpenConsole API
src/ch/ch_domain.c | 4 +-
src/ch/ch_driver.c | 78 +++++++++++++++++++++++++++++
src/ch/ch_monitor.c | 117 ++++++++++++++++++++++++++++++++++++++++++--
src/ch/ch_monitor.h | 3 ++
src/ch/ch_process.c | 67 +++++++++++++++++++++++++
5 files changed, 263 insertions(+), 6 deletions(-)
Hey, we require that the code is able to compile and test suite passes
after each individual commit (because it's more cherry-pick friendly).
Basically 'ninja -C build/ test' should pass after each commit.
Otherwise, see individual comments.
Michal
5:04 p.m.
On Mon, 2021-07-12 at 14:05 +0200, Michal Prívozník wrote:
On 6/30/21 1:05 AM, William Douglas wrote:
Hey, we require that the code is able to compile and test suite
passes
after each individual commit (because it's more cherry-pick
friendly).
Basically 'ninja -C build/ test' should pass after each commit.
I just now noticed this was missing the first commit in the series,
sorry about that.
1230
days inactive
1243
days old
18 comments
3 participants
participants (3)
-
Douglas, William -
Michal Prívozník -
William Douglas