[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [GSoC PATCH 2/9] Jailhouse driver: Implementation of ConnectOpen



On a Monday in 2020, Prakhar Bansal wrote:
---
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 */
+

Unrelated whitespace change - more fitting for the first patch.

# ifdef VIR_ENUM_SENTINELS
    VIR_ERR_DOMAIN_LAST
# endif
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 3d6c20c55f..a94285817f 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -85,6 +85,8 @@
@SRCDIR src/interface/interface_backend_netcf.c
@SRCDIR src/interface/interface_backend_udev.c
@SRCDIR src/internal.h
+ SRCDIR@src/jailhouse/jailhouse_api.c
+ SRCDIR@src/jailhouse/jailhouse_driver.c
@SRCDIR src/libvirt-domain-checkpoint.c
@SRCDIR src/libvirt-domain-snapshot.c
@SRCDIR src/libvirt-domain.c
--- a/src/jailhouse/jailhouse_driver.c
+++ b/src/jailhouse/jailhouse_driver.c
@@ -16,43 +16,228 @@
 * 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 <string.h>


No need to include string.h

+#include "configmake.h"
+#include "datatypes.h"
+#include "domain_conf.h"

#include "jailhouse_driver.h"

jailhouse_driver.h should stay at the top

#include "virtypedparam.h"
#include "virerror.h"
#include "virstring.h"
#include "viralloc.h"
-#include "domain_conf.h"
#include "virfile.h"
-#include "datatypes.h"
+#include "virlog.h"
#include "vircommand.h"
-#include <string.h>
+#include "virpidfile.h"

-#define UNUSED(x) (void)(x)
static const char *
jailhouseConnectGetType(virConnectPtr conn)
{


Lots of unnecessary changes below here:

@@ -69,36 +254,16 @@ jailhouseConnectGetHostname(virConnectPtr conn)
}

static int
-jailhouseNodeGetInfo(virConnectPtr conn,
-                     virNodeInfoPtr info)
+jailhouseNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
{
    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 */

Please don't try to align the comments, that way any change in spacing
results in a huge diff like this.

};

+
static virConnectDriver jailhouseConnectDriver = {
+    .localOnly = true,
+    .uriSchemes = (const char *[]){ "jailhouse", NULL },
    .hypervisorDriver = &jailhouseHypervisorDriver,
};

+
+static virStateDriver jailhouseStateDriver = {
+    .name = "JAILHOUSE",
+    .stateInitialize = jailhouseStateInitialize,
+    .stateCleanup = jailhouseStateCleanup,
+};
+
int
jailhouseRegister(void)
{
-    return virRegisterConnectDriver(&jailhouseConnectDriver, false);
+    if (virRegisterConnectDriver(&jailhouseConnectDriver, false) < 0)
+        return -1;
+    if (virRegisterStateDriver(&jailhouseStateDriver) < 0)
+        return -1;
+    return 0;
}
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

These should be using /* */ as the comment marker.

+    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;
+    char *state;
+    char *cpus_assigned_list;
+    char *cpus_failed_list;
+};
diff --git a/src/jailhouse/meson.build b/src/jailhouse/meson.build
index 45ceeecca3..a706985169 100644
--- a/src/jailhouse/meson.build
+++ b/src/jailhouse/meson.build
@@ -1,5 +1,6 @@
jailhouse_sources = files(
  'jailhouse_driver.c',
+  'jailhouse_api.c',
)

driver_source_files += jailhouse_sources

diff --git a/src/libvirt.c b/src/libvirt.c
index 59b75c6f7b..b2d0ba3d23 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;
@@ -1010,9 +1003,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,

These changes were all added in the first patch. Why remove them?

diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 1aa9bfc0d2..9d1b208a38 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -145,6 +145,10 @@ static int daemonInitialize(void)
    if (virDriverLoadModule("interface", "interfaceRegister", false) < 0)
        return -1;
# endif
+# ifdef WITH_JAILHOUSE
+    if (virDriverLoadModule("jailhouse", "jailhouseRegister", false) < 0)
+        return -1;
+# endif
# ifdef WITH_SECRETS
    if (virDriverLoadModule("secret", "secretRegister", false) < 0)
        return -1;

Jano

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]