On Thu, Sep 17, 2020 at 04:23:52PM +0100, Daniel P. Berrangé wrote:
From: Prakhar Bansal <prakharbansal0910(a)gmail.com>
---
include/libvirt/virterror.h | 1 +
po/POTFILES.in | 2 +
src/jailhouse/Makefile.inc.am | 34 ++-
src/jailhouse/jailhouse.conf | 10 +
src/jailhouse/jailhouse_api.c | 372 ++++++++++++++++++++++++++++
src/jailhouse/jailhouse_api.h | 74 ++++++
src/jailhouse/jailhouse_driver.c | 302 +++++++++++++++++-----
src/jailhouse/jailhouse_driver.h | 51 ++++
src/jailhouse/meson.build | 1 +
src/libvirt.c | 10 -
src/remote/remote_daemon.c | 4 +
src/remote/remote_daemon_dispatch.c | 3 +-
12 files changed, 783 insertions(+), 81 deletions(-)
create mode 100644 src/jailhouse/jailhouse.conf
create mode 100644 src/jailhouse/jailhouse_api.c
create mode 100644 src/jailhouse/jailhouse_api.h
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 97f2ac16d8..9f1bca2684 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -137,6 +137,7 @@ typedef enum {
VIR_FROM_TPM = 70, /* Error from TPM */
VIR_FROM_BPF = 71, /* Error from BPF code */
VIR_FROM_JAILHOUSE = 72, /* Error from Jailhouse driver */
+
# ifdef VIR_ENUM_SENTINELS
VIR_ERR_DOMAIN_LAST
# endif
Should be in the previous patch.
diff --git a/src/jailhouse/Makefile.inc.am
b/src/jailhouse/Makefile.inc.am
index 02822b2ea1..324c3b1b16 100644
--- a/src/jailhouse/Makefile.inc.am
+++ b/src/jailhouse/Makefile.inc.am
This file shouldn't exist any more.
diff --git a/src/jailhouse/jailhouse_api.c
b/src/jailhouse/jailhouse_api.c
new file mode 100644
index 0000000000..cda00b50e7
--- /dev/null
+++ b/src/jailhouse/jailhouse_api.c
@@ -0,0 +1,372 @@
+/*
+ * jailhouse_api.c: Jailhouse API
+ *
+ * Copyright (C) 2020 Prakhar Bansal
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <
http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <config.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <dirent.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <linux/types.h>
+
+#include "viralloc.h"
+#include "virerror.h"
+#include "virfile.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "jailhouse_api.h"
+
+#define JAILHOUSE_DEVICE "/dev/jailhouse"
+#define JAILHOUSE_CELLS
"/sys/devices/jailhouse/cells"
+#define MAX_JAILHOUSE_SYS_CONFIG_FILE_SIZE 1024*1024
+#define MAX_JAILHOUSE_CELL_CONFIG_FILE_SIZE 1024
+
+
+#define JAILHOUSE_ENABLE _IOW(0, 0, void *)
+#define JAILHOUSE_DISABLE _IO(0, 1)
+#define JAILHOUSE_CELL_CREATE _IOW(0, 2, virJailhouseCellCreate)
+#define JAILHOUSE_CELL_DESTROY _IOW(0, 5, virJailhouseCellId)
+
+#define VIR_FROM_THIS VIR_FROM_JAILHOUSE
+
+VIR_LOG_INIT("jailhouse.jailhouse_api");
+
+#define JAILHOUSE_CELL_FILE_EXTENSION ".cell"
+
+/* Forward declarations */
+
+/* Open the Jailhouse device for ioctl APIs */
+int openDev(void);
+
+/* Reads cell's property given by 'entry' using sysfs API */
+char *readSysfsCellString(const unsigned int id, const char *entry);
+
+int cell_match(const struct dirent *dirent);
+
+int createCell(const char *conf_file);
+
+int destroyCell(virJailhouseCellId cell_id);
+
+int getCellInfo(const unsigned int id,
+ virJailhouseCellInfoPtr * cell_info);
We expect that all methods have a consistent name prefix.
So everything in this driver should really be named with
"virJailhouse" as an API prefix. This naming rule includes
static methods
+
+int
+jailhouseEnable(const char *sys_conf_file_path)
+{
+ int err = -1, len;
+ g_autofree char *buffer = NULL;
+ VIR_AUTOCLOSE fd = -1;
+
+ if (!virFileExists(sys_conf_file_path))
+ return 0;
+
+ len = virFileReadAll(sys_conf_file_path, MAX_JAILHOUSE_SYS_CONFIG_FILE_SIZE,
&buffer);
+ if (len < 0 || !buffer) {
buffer will be non-NULL if len >= 0, so the second test isn't needed.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("Failed to read the system configuration
file"));
+ return -1;
+ }
+
+ fd = openDev();
You need to check for error opening files...
+
+ err = ioctl(fd, JAILHOUSE_ENABLE, buffer);
or this uses a closed FD.
+ if (err) {
+ virReportSystemError(errno, "%s", _("Failed to enable
jailhouse"));
+ return err;
+ }
+
+ VIR_DEBUG("Jailhouse hypervisor is enabled");
+
+ return 1;
+}
+
+int
+jailhouseDisable(void)
+{
+ int err = -1;
+ VIR_AUTOCLOSE fd = -1;
+
+ fd = openDev();
Again check for error
+
+ err = ioctl(fd, JAILHOUSE_DISABLE);
+ if (err)
+ virReportSystemError(errno,
+ "%s",
+ _("Failed to disable jailhouse: %s"));
+
+ VIR_DEBUG("Jailhouse hypervisor is disabled");
+
+ return err;
+}
+
+int
+cell_match(const struct dirent *dirent)
+{
+ char *ext = strrchr(dirent->d_name, '.');
+
+ return dirent->d_name[0] != '.'
+ && (STREQ(ext, JAILHOUSE_CELL_FILE_EXTENSION) == 0);
You need to check for "ext" being NULL.
+}
+
+int
+createJailhouseCells(const char *dir_path)
+{
+
+ struct dirent **namelist;
+ int num_entries, ret = -1;
+ size_t i;
+
+ if (strlen(dir_path) == 0)
+ return ret;
+
+ num_entries = scandir(dir_path, &namelist, cell_match, alphasort);
+ if (num_entries == -1) {
+ if (errno == ENOENT) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("No cells found in %s, scandir failed."),
+ dir_path);
+ goto fail;
+ }
+
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Error reading cell configurations in %s."),
+ dir_path);
+ goto fail;
+ }
+
+
+ for (i = 0; i < num_entries; i++) {
+ g_autofree char *file_path = g_strdup_printf("%s/%s", dir_path,
namelist[i]->d_name);
+
+ if (createCell(file_path) != 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Cell creation failed with conf found in %s."),
+ namelist[i]->d_name);
+ goto fail;
+ }
+ }
+
+ ret = 0;
+
+ fail:
Our naming convention is "cleanup" for paths that are shared with
successful exit, or "error" for exclusively failure paths. So
this one should be "cleanup"
+ VIR_FREE(namelist);
+ return ret;
+}
+int
+createCell(const char *conf_file)
+{
+ virJailhouseCellCreate cell_create;
+ int err = -1, len;
+ g_autofree char *buffer = NULL;
+ VIR_AUTOCLOSE fd = -1;
+
+ if (strlen(conf_file) == 0)
+ return err;
+
+ len = virFileReadAll(conf_file, MAX_JAILHOUSE_CELL_CONFIG_FILE_SIZE, &buffer);
+ if (len < 0 || !buffer) {
No need for !buffer
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("Failed to read the system configuration
file"));
+ return err;
+ }
+
+ cell_create.config_address = (unsigned long) buffer;
+ cell_create.config_size = len;
+
+ fd = openDev();
Check error.
+
+ err = ioctl(fd, JAILHOUSE_CELL_CREATE, &cell_create);
+ if (err)
+ virReportSystemError(errno,
+ "%s",
+ _("Cell creation failed: %s"));
+
+ return err;
+}
+char *
+readSysfsCellString(const unsigned int id, const char *entry)
+{
+ g_autofree char *buffer = NULL;
+ g_autofree char *file_path = NULL;
+ int len = -1;
+
+ file_path = g_strdup_printf(JAILHOUSE_CELLS "%u/%s", id, entry);
+
+ len = virFileReadAll(file_path, 1024, &buffer);
+ if (len < 0 || !buffer) {
No need for !buffer
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Error reading cell(%u) %s from %s failed"),
+ id, entry, file_path);
+ return NULL;
+ }
+
+ virTrimSpaces(buffer, NULL);
+
+ return buffer;
+}
+
+ /* Allocate memory for 1 more than num_entries and make the last entry NULL. */
+ if (VIR_ALLOC_N(cell_info_list, num_entries + 1) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s",
+ _("Insufficient memory for cells info list"));
+ }
Don't need to check for failure as it aborts().
+
+ /* Set the last entry to NULL. */
+ cell_info_list[num_entries] = NULL;
+
+ for (i = 0; i < num_entries; i++) {
+ if (virStrToLong_ui(namelist[i]->d_name, NULL, 10, &id) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cell ID %s could not be converted to a long"),
+ namelist[i]->d_name);
+ continue;
+ }
+
+ /* get the cell's information(name, state etc.) using sysfs */
+ getCellInfo(id, &cell_info_list[i]);
+ VIR_FREE(namelist[i]);
+ }
+
+ VIR_FREE(namelist);
+ return cell_info_list;
+}
+
+int
+destroyCell(virJailhouseCellId cell_id)
+{
+ int err = -1;
+ VIR_AUTOCLOSE fd = -1;
+
+ fd = openDev();
Check for error
+
+ err = ioctl(fd, JAILHOUSE_CELL_DESTROY, &cell_id);
+ if (err)
+ virReportSystemError(errno,
+ _("Destroying cell %d failed"),
+ cell_id.id);
+
+ return err;
+}
diff --git a/src/jailhouse/jailhouse_api.h
b/src/jailhouse/jailhouse_api.h
new file mode 100644
index 0000000000..8362cb3d0f
--- /dev/null
+++ b/src/jailhouse/jailhouse_api.h
@@ -0,0 +1,74 @@
+/*
+ * jailhouse_api.h: Jailhouse hypervisor API implementation
+ *
+ * Copyright (C) 2020 Prakhar Bansal
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <
http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#define JAILHOUSE_CELL_ID_NAMELEN 31
+
+typedef struct _virJailhouseCellId virJailhouseCellId;
+
+struct _virJailhouseCellId {
+ __s32 id;
Shouldn't be using these types, instead
int32_t
+ __u32 padding;
uint32_t
+ char name[JAILHOUSE_CELL_ID_NAMELEN + 1];
+};
+
+typedef struct _virJailhouseCellInfo virJailhouseCellInfo;
+typedef virJailhouseCellInfo *virJailhouseCellInfoPtr;
+
+struct _virJailhouseCellInfo {
+ struct _virJailhouseCellId id;
+ char *state;
+ char *cpus_assigned_list;
+ char *cpus_failed_list;
+};
+
+typedef struct _virJailhouseCellCreate virJailhouseCellCreate;
+
+struct _virJailhouseCellCreate {
+ __u64 config_address;
uint64_t ..etc
> + __u32 config_size;
+ __u32 padding;
> +};
> +
> +// Enables the Jailhouse hypervisor by reading the hypervisor system
> +// configuration from the given file and calls the ioctl API to
> +// enable the hypervisor.
> +int jailhouseEnable(const char *sys_conf_file_path);
> +
> +// Disables the Jailhouse hypervisor.
> +int jailhouseDisable(void);
> +
> +/* Cell API methods */
> +
> +// Creates Jailhouse cells using the cells configurations
> +// provided in the dir_name.
> +int createJailhouseCells(const char *dir_path);
> +
> +// Destroys Jailhouse cells using the cell IDs provided in
> +// the cell_info_list.
> +int destroyJailhouseCells(virJailhouseCellInfoPtr *cell_info_list);
> +
> +// Returns cell's information in a null-terminated array of
> +// virJailhouseCellInfoPtr for all the Jailhouse cells.
> +virJailhouseCellInfoPtr *getJailhouseCellsInfo(void);
> +
> +// Free the cell info object.
> +void cellInfoFree(virJailhouseCellInfoPtr cell_info);
@@ -69,36 +254,16 @@ jailhouseConnectGetHostname(virConnectPtr conn)
}
static int
-jailhouseNodeGetInfo(virConnectPtr conn,
- virNodeInfoPtr info)
+jailhouseNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
This formatting change should be done in the original patch. Same
for the changes that follow
{
UNUSED(conn);
UNUSED(info);
return -1;
}
-static int
-jailhouseConnectListDomains(virConnectPtr conn,
- int *ids,
- int maxids)
-{
- UNUSED(conn);
- UNUSED(ids);
- UNUSED(maxids);
- return -1;
-}
-
-static int
-jailhouseConnectNumOfDomains(virConnectPtr conn)
-{
- UNUSED(conn);
- return -1;
-}
-
static int
jailhouseConnectListAllDomains(virConnectPtr conn,
- virDomainPtr **domain,
- unsigned int flags)
+ virDomainPtr ** domain, unsigned int flags)
{
UNUSED(conn);
UNUSED(domain);
@@ -107,8 +272,7 @@ jailhouseConnectListAllDomains(virConnectPtr conn,
}
static virDomainPtr
-jailhouseDomainLookupByID(virConnectPtr conn,
- int id)
+jailhouseDomainLookupByID(virConnectPtr conn, int id)
{
UNUSED(conn);
UNUSED(id);
@@ -116,8 +280,7 @@ jailhouseDomainLookupByID(virConnectPtr conn,
}
static virDomainPtr
-jailhouseDomainLookupByName(virConnectPtr conn,
- const char *name)
+jailhouseDomainLookupByName(virConnectPtr conn, const char *name)
{
UNUSED(conn);
UNUSED(name);
@@ -125,8 +288,7 @@ jailhouseDomainLookupByName(virConnectPtr conn,
}
static virDomainPtr
-jailhouseDomainLookupByUUID(virConnectPtr conn,
- const unsigned char *uuid)
+jailhouseDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
{
UNUSED(conn);
UNUSED(uuid);
@@ -157,8 +319,7 @@ jailhouseDomainDestroy(virDomainPtr domain)
}
static int
-jailhouseDomainGetInfo(virDomainPtr domain,
- virDomainInfoPtr info)
+jailhouseDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
{
UNUSED(domain);
UNUSED(info);
@@ -167,9 +328,7 @@ jailhouseDomainGetInfo(virDomainPtr domain,
static int
jailhouseDomainGetState(virDomainPtr domain,
- int *state,
- int *reason,
- unsigned int flags)
+ int *state, int *reason, unsigned int flags)
{
UNUSED(domain);
UNUSED(state);
@@ -179,8 +338,7 @@ jailhouseDomainGetState(virDomainPtr domain,
}
static char *
-jailhouseDomainGetXMLDesc(virDomainPtr domain,
- unsigned int flags)
+jailhouseDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
{
UNUSED(domain);
UNUSED(flags);
@@ -189,31 +347,43 @@ jailhouseDomainGetXMLDesc(virDomainPtr domain,
static virHypervisorDriver jailhouseHypervisorDriver = {
.name = "JAILHOUSE",
- .connectOpen = jailhouseConnectOpen, /* 6.3.0 */
- .connectClose = jailhouseConnectClose, /* 6.3.0 */
- .connectListDomains = jailhouseConnectListDomains, /* 6.3.0 */
- .connectNumOfDomains = jailhouseConnectNumOfDomains, /* 6.3.0 */
- .connectListAllDomains = jailhouseConnectListAllDomains, /* 6.3.0 */
- .domainLookupByID = jailhouseDomainLookupByID, /* 6.3.0 */
- .domainLookupByUUID = jailhouseDomainLookupByUUID, /* 6.3.0 */
- .domainLookupByName = jailhouseDomainLookupByName, /* 6.3.0 */
- .domainGetXMLDesc = jailhouseDomainGetXMLDesc, /* 6.3.0 */
- .domainCreate = jailhouseDomainCreate, /* 6.3.0 */
- .connectGetType = jailhouseConnectGetType, /* 6.3.0 */
- .connectGetHostname = jailhouseConnectGetHostname, /* 6.3.0 */
- .nodeGetInfo = jailhouseNodeGetInfo, /* 6.3.0 */
- .domainShutdown = jailhouseDomainShutdown, /* 6.3.0 */
- .domainDestroy = jailhouseDomainDestroy, /* 6.3.0 */
- .domainGetInfo = jailhouseDomainGetInfo, /* 6.3.0 */
- .domainGetState = jailhouseDomainGetState, /* 6.3.0 */
+ .connectOpen = jailhouseConnectOpen, /* 6.3.0 */
+ .connectClose = jailhouseConnectClose, /* 6.3.0 */
+ .connectListAllDomains = jailhouseConnectListAllDomains, /* 6.3.0 */
+ .domainLookupByID = jailhouseDomainLookupByID, /* 6.3.0 */
+ .domainLookupByUUID = jailhouseDomainLookupByUUID, /* 6.3.0 */
+ .domainLookupByName = jailhouseDomainLookupByName, /* 6.3.0 */
+ .domainGetXMLDesc = jailhouseDomainGetXMLDesc, /* 6.3.0 */
+ .domainCreate = jailhouseDomainCreate, /* 6.3.0 */
+ .connectGetType = jailhouseConnectGetType, /* 6.3.0 */
+ .connectGetHostname = jailhouseConnectGetHostname, /* 6.3.0 */
+ .nodeGetInfo = jailhouseNodeGetInfo, /* 6.3.0 */
+ .domainShutdown = jailhouseDomainShutdown, /* 6.3.0 */
+ .domainDestroy = jailhouseDomainDestroy, /* 6.3.0 */
+ .domainGetInfo = jailhouseDomainGetInfo, /* 6.3.0 */
+ .domainGetState = jailhouseDomainGetState, /* 6.3.0 */
Don't do this formatting change - trying to horizontally align such
comments is a waste of time in the long term.
diff --git a/src/jailhouse/jailhouse_driver.h
b/src/jailhouse/jailhouse_driver.h
index b0dbc8d033..8a0e111676 100644
--- a/src/jailhouse/jailhouse_driver.h
+++ b/src/jailhouse/jailhouse_driver.h
@@ -20,4 +20,55 @@
#pragma once
+#include <linux/types.h>
+
+#include "jailhouse_api.h"
+
int jailhouseRegister(void);
+
+#define JAILHOUSE_CONFIG_FILE SYSCONFDIR "/libvirt/jailhouse/jailhouse.conf"
+#define JAILHOUSE_STATE_DIR RUNSTATEDIR "/libvirt/jailhouse"
+
+#define JAILHOUSE_DEV "/dev/jailhouse"
+
+#define JAILHOUSE_SYSFS_DEV "/sys/devices/jailhouse/"
+
+typedef struct _virJailhouseDriver virJailhouseDriver;
+typedef virJailhouseDriver *virJailhouseDriverPtr;
+
+typedef struct _virJailhouseDriverConfig virJailhouseDriverConfig;
+typedef virJailhouseDriverConfig *virJailhouseDriverConfigPtr;
+
+struct _virJailhouseDriverConfig {
+ virObject parent;
+
+ char *stateDir;
+
+ // File path of the jailhouse system configuration
+ // for jailhouse enable/disable.
+ char *sys_config_file_path;
+
+ // Config directory where all jailhouse cell configurations
+ // are stored.
+ char *cell_config_dir;
+};
+
+struct _virJailhouseDriver {
+ virMutex lock;
+
+ // Jailhouse configuration read from the jailhouse.conf
+ virJailhouseDriverConfigPtr config;
+
+ /* pid file FD, ensures two copies of the driver can't use the same root */
+ int lockFD;
+
+ // All the cells created during connect open on the hypervisor.
+ virJailhouseCellInfoPtr *cell_info_list;
+};
+
+struct _jailhouseCell {
+ __s32 id;
int32_t
+ char *state;
+ char *cpus_assigned_list;
+ char *cpus_failed_list;
+};
diff --git a/src/libvirt.c b/src/libvirt.c
index 8a78cbcf3a..0748eb2352 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -75,9 +75,6 @@
#ifdef WITH_BHYVE
# include "bhyve/bhyve_driver.h"
#endif
-#ifdef WITH_JAILHOUSE
-# include "jailhouse/jailhouse_driver.h"
-#endif
#include "access/viraccessmanager.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -274,10 +271,6 @@ virGlobalInit(void)
if (hypervRegister() == -1)
goto error;
#endif
-#ifdef WITH_JAILHOUSE
- if (jailhouseRegister() == -1)
- goto error;
-#endif
#ifdef WITH_REMOTE
if (remoteRegister() == -1)
goto error;
@@ -1052,9 +1045,6 @@ virConnectOpenInternal(const char *name,
#endif
#ifndef WITH_VZ
STRCASEEQ(ret->uri->scheme, "parallels") ||
-#endif
-#ifndef WITH_JAILHOUSE
- STRCASEEQ(ret->uri->scheme, "jailhouse") ||
#endif
false)) {
virReportErrorHelper(VIR_FROM_NONE, VIR_ERR_CONFIG_UNSUPPORTED,
You just added all these lines in the previous patch. If they're not
relevant, then don't add them in the first place.
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 :|