From e523057e7068e283b74d78bb8d707d83f6e73c24 Mon Sep 17 00:00:00 2001 From: Christian Loehle <cloehle@linutronix.de> Date: Wed, 11 Nov 2015 09:21:54 +0100 Subject: [PATCH] hypervisor driver for Jailhouse

--- configure.ac | 21 ++ include/libvirt/virterror.h | 1 + po/POTFILES.in | 1 + src/Makefile.am | 15 + src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + src/jailhouse/README | 3 + src/jailhouse/jailhouse_driver.c | 622 +++++++++++++++++++++++++++++++++++++++ src/jailhouse/jailhouse_driver.h | 28 ++ src/libvirt.c | 11 + src/util/virerror.c | 1 + 11 files changed, 706 insertions(+), 1 deletion(-) create mode 100644 src/jailhouse/README create mode 100644 src/jailhouse/jailhouse_driver.c create mode 100644 src/jailhouse/jailhouse_driver.h diff --git a/configure.ac b/configure.ac index f481c50..8b68828 100644 --- a/configure.ac +++ b/configure.ac @@ -563,6 +563,10 @@ AC_ARG_WITH([hyperv], [AS_HELP_STRING([--with-hyperv], [add Hyper-V support @<:@default=check@:>@])]) m4_divert_text([DEFAULTS], [with_hyperv=check]) +AC_ARG_WITH([jailhouse], + [AS_HELP_STRING([--with-jailhouse], + [add Jailhouse support @<:@default=yes@:>@])]) +m4_divert_text([DEFAULTS], [with_jailhouse=yes]) AC_ARG_WITH([test], [AS_HELP_STRING([--with-test], [add test driver support @<:@default=yes@:>@])]) @@ -722,6 +726,16 @@ AM_CONDITIONAL([WITH_VMWARE], [test "$with_vmware" = "yes"]) dnl +dnl Checks for the Jailhouse driver +dnl + +if test "$with_jailhouse" = "yes"; then + AC_DEFINE_UNQUOTED([WITH_JAILHOUSE], 1, [whether Jailhouse driver is enabled]) +fi +AM_CONDITIONAL([WITH_JAILHOUSE], [test "$with_jailhouse" = "yes"]) + + +dnl dnl check for XDR dnl @@ -1087,6 +1101,12 @@ dnl LIBVIRT_DRIVER_CHECK_BHYVE dnl +dnl Checks for Jailhouse driver +dnl + +AM_CONDITIONAL([WITH_JAILHOUSE], [test "$with_jailhouse" = "yes"]) + +dnl dnl check for shell that understands <> redirection without truncation, dnl needed by src/qemu/qemu_monitor_{text,json}.c. dnl @@ -2830,6 +2850,7 @@ AC_MSG_NOTICE([ ESX: $with_esx]) AC_MSG_NOTICE([ Hyper-V: $with_hyperv]) LIBVIRT_DRIVER_RESULT_VZ LIBVIRT_DRIVER_RESULT_BHYVE +AC_MSG_NOTICE([Jailhouse: $with_jailhouse]) AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index f716cb9..c8fe2d3 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -127,6 +127,7 @@ typedef enum { VIR_FROM_POLKIT = 60, /* Error from polkit code */ VIR_FROM_THREAD = 61, /* Error from thread utils */ VIR_FROM_ADMIN = 62, /* Error from admin backend */ + VIR_FROM_JAILHOUSE = 63, /* Error from Jailhouse driver */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/po/POTFILES.in b/po/POTFILES.in index 0cc5b99..2b144bf 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -59,6 +59,7 @@ src/hyperv/hyperv_wmi.c src/interface/interface_backend_netcf.c src/interface/interface_backend_udev.c src/internal.h +src/jailhouse/jailhouse_driver.c src/libvirt.c src/libvirt-admin.c src/libvirt-domain.c diff --git a/src/Makefile.am b/src/Makefile.am index 99b4993..10d59de 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -578,6 +578,7 @@ DRIVER_SOURCE_FILES = \ $(VMWARE_DRIVER_SOURCES) \ $(XEN_DRIVER_SOURCES) \ $(XENAPI_DRIVER_SOURCES) \ + $(JAILHOUSE_DRIVER_SOURCES) \ $(NULL) STATEFUL_DRIVER_SOURCE_FILES = \ @@ -860,6 +861,11 @@ BHYVE_DRIVER_SOURCES = \ bhyve/bhyve_utils.h \ $(NULL) +JAILHOUSE_DRIVER_SOURCES = \ + jailhouse/jailhouse_driver.c \ + jailhouse/jailhouse_driver.h \ + $(NULL) + NETWORK_DRIVER_SOURCES = \ network/bridge_driver.h network/bridge_driver.c \ network/bridge_driver_platform.h \ @@ -1436,6 +1442,14 @@ libvirt_driver_vz_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS) libvirt_driver_vz_la_SOURCES = $(VZ_DRIVER_SOURCES) endif WITH_VZ +if WITH_JAILHOUSE +noinst_LTLIBRARIES += libvirt_driver_jailhouse.la +libvirt_la_BUILT_LIBADD += libvirt_driver_jailhouse.la +libvirt_driver_jailhouse_la_CFLAGS = \ + -I$(srcdir)/conf $(AM_CFLAGS) +libvirt_driver_jailhouse_la_SOURCES = $(JAILHOUSE_DRIVER_SOURCES) +endif WITH_JAILHOUSE + if WITH_BHYVE noinst_LTLIBRARIES += libvirt_driver_bhyve_impl.la libvirt_driver_bhyve_la_SOURCES = @@ -1801,6 +1815,7 @@ EXTRA_DIST += \ $(HYPERV_DRIVER_EXTRA_DIST) \ $(VZ_DRIVER_SOURCES) \ $(BHYVE_DRIVER_SOURCES) \ + $(JAILHOUSE_DRIVER_SOURCES) \ $(NETWORK_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(STORAGE_DRIVER_SOURCES) \ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2edf123..00d17e9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -121,7 +121,8 @@ VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST, "phyp", "parallels", "bhyve", - "vz") + "vz", + "jailhouse") VIR_ENUM_IMPL(virDomainOS, VIR_DOMAIN_OSTYPE_LAST, "hvm", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f10b534..27beef0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -225,6 +225,7 @@ typedef enum { VIR_DOMAIN_VIRT_PARALLELS, VIR_DOMAIN_VIRT_BHYVE, VIR_DOMAIN_VIRT_VZ, + VIR_DOMAIN_VIRT_JAILHOUSE, VIR_DOMAIN_VIRT_LAST } virDomainVirtType; diff --git a/src/jailhouse/README b/src/jailhouse/README new file mode 100644 index 0000000..96c32b1 --- /dev/null +++ b/src/jailhouse/README @@ -0,0 +1,3 @@ +The jailhouse hypervisor driver for the libvirt project aims to provide rudimentary support for managing jailhouse with the libvirt library. The main advantage of this is the possibility to use virt-manager as a GUI to manage Jailhouse cells. Thus the driver is mainly built around the API calls that virt-manager uses and needs. +Due to the concept of Jailhouse a lot of libvirt functions can't be realized, so this driver isn't as full-featured as upstream drivers of the libvirt project. +Currently the driver relies on the Jailhouse binary, which has to be passed when connecting a libvirt client to it(e.g. virt-manager -c jailhouse:///path/to/jailhouse/tools/jailhouse). Be aware though that the driver doesn't store any information about cells, so most API calls use "jailhouse cell list" every time they're called to get the current state. diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c new file mode 100644 index 0000000..9f1ed70 --- /dev/null +++ b/src/jailhouse/jailhouse_driver.c @@ -0,0 +1,622 @@ +/* + * jailhouse_driver.c: hypervisor driver for managing Jailhouse cells + * + * Copyright (C) 2015 Linutronix GmbH + * + * 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/>. + * + * Author: Christian Loehle + */ + +#include <config.h> +#include <string.h> +#include "jailhouse_driver.h" +#include "datatypes.h" +#include "virerror.h" +#include "viralloc.h" +#include "virlog.h" +#include "vircommand.h" +#include "virxml.h" +#include "configmake.h" +#include "virfile.h" +#include "virtypedparam.h" +#include "virstring.h" +#include "nodeinfo.h" + +#define VIR_FROM_THIS VIR_FROM_JAILHOUSE + +#define IDLENGTH 8 +#define NAMELENGTH 24 +#define STATELENGTH 16 +#define CPULENGTH 24 +#define STATERUNNING 0 +#define STATERUNNINGSTRING "running " +#define STATERUNNINGLOCKED 1 +#define STATERUNNINGLOCKEDSTRING "running/locked " +#define STATESHUTDOWN 2 +#define STATESHUTDOWNSTRING "shut down " +#define STATEFAILED 3 +#define STATEFAILEDSTRING "failed " +#define JAILHOUSEVERSIONOUTPUT "Jailhouse management tool" + +/* + * The driver requeries the cells on most calls, it stores the result of the last query, so it can copy the UUIDs in the new query if the cell is the same(otherwise it just generates a new one) + * not preserving the UUID results in a lot of bugs in libvirts clients. + */ +struct jailhouse_driver { + char *binary; + size_t lastQueryCellsCount; + struct jailhouse_cell* lastQueryCells; +}; + +/* + * CPUs are currently unused but this might change + */ +struct jailhouse_cell { + int id; + char name[NAMELENGTH+1]; + int state; + int *assignedCPUs; //Don't use cpumask because remote system might have different # of cpus + int assignedCPUsLength; + int *failedCPUs; + int failedCPUsLength; + unsigned char uuid[VIR_UUID_BUFLEN]; +}; + +/* + * helper function that returns the number as an integer and sets i to be the first char after the number + */ +static int +charsToInt(char* chars, size_t *i) +{ + int result = 0; + while (chars[*i] != ',' && chars[*i] != '-' && chars[*i] != ' ') { + result *= 10; + result += chars[*i] - '0'; + (*i)++; + } + return result; +} + +/* + * Takes a string in the format of "jailhouse cell list" as input, + * allocates an int array in which every CPU is explicitly listed and saves a pointer in cpusptr + */ +static size_t +parseCPUs(char* output, int **cpusptr) +{ + size_t i; + size_t count = 1; + int number; + int* cpus; + if (output[0] == ' ') { + *cpusptr = NULL; + return 0; + } + for (i = 0; i<CPULENGTH; i++) { + number = charsToInt(output, &i); + if (output[i] == ',') { + count++; + } else if (output[i] == '-') { + i++; + count += charsToInt(output, &i) - number; + } + } + if (VIR_ALLOC_N(cpus, count)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate CPUs array of size %zu"), count); + return 0; + } + size_t j = 0; + i = 0; + while (output[i] != ' ') { + number = charsToInt(output, &i); + if (output[i] == ',' || output[i] == ' ') { + cpus[j++] = number; + } else if (output[i] == '-') { + i++; + int nextNumber = charsToInt(output, &i); + for (; number <= nextNumber; number++) cpus[j++] = number; + } + i++; + } + *cpusptr = cpus; + return count; +} + +/* + * calls "jailhouse cell list" and parses the output in an array of jailhouse_cell + */ +static size_t +parseListOutput(virConnectPtr conn, struct jailhouse_cell **parsedOutput) +{ + virCommandPtr cmd = virCommandNew(((struct jailhouse_driver *)conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "list"); + virCommandAddEnvPassCommon(cmd); + char *output; + virCommandSetOutputBuffer(cmd, &output); + size_t count = -1; // Don't count table header line + size_t i = 0; + if (virCommandRun(cmd, NULL) < 0) + goto error; + while (output[i] != '\0') { + if (output[i] == '\n') count++; + i++; + } + if (VIR_ALLOC_N(*parsedOutput, count)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate jailhouse_cell array of size %zu"), count); + goto error; + } + if (*parsedOutput == NULL) + goto error; + i = 0; + size_t j; + while (output[i++] != '\n'); // Skip table header line + for (j = 0; j < count; j++) { + size_t k; + for (k = 0; k <= IDLENGTH; k++) // char after number needs to be NUL for virStrToLong + if (output[i+k] == ' ') { + output[i+k] = '\0'; + break; + } + char c = output[i+IDLENGTH]; + output[i+IDLENGTH] = '\0'; // in case ID is 8 chars long, so beginning of name won't get parsed + if (virStrToLong_i(output+i, NULL, 0, &(*parsedOutput)[j].id)) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse id to long: %s"), output+i); + output[i+IDLENGTH] = c; + i += IDLENGTH; + if (virStrncpy((*parsedOutput)[j].name, output+i, NAMELENGTH, NAMELENGTH+1) == NULL) + // should never happen + goto error; + (*parsedOutput)[j].name[NAMELENGTH] = '\0'; + for (k = 0; k < NAMELENGTH; k++) + if ((*parsedOutput)[j].name[k] == ' ') + break; + (*parsedOutput)[j].name[k] = '\0'; + i += NAMELENGTH; + if (STREQLEN(output+i, STATERUNNINGSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATERUNNING; + else if (STREQLEN(output+i, STATESHUTDOWNSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATESHUTDOWN; + else if (STREQLEN(output+i, STATEFAILEDSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATEFAILED; + else if (STREQLEN(output+i, STATERUNNINGLOCKEDSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATERUNNINGLOCKED; + i += STATELENGTH; + (*parsedOutput)[j].assignedCPUsLength = parseCPUs(output+i, &((*parsedOutput)[j].assignedCPUs)); + i += CPULENGTH; + (*parsedOutput)[j].failedCPUsLength = parseCPUs(output+i, &((*parsedOutput)[j].failedCPUs)); + i += CPULENGTH; + i++; // skip \n + } + VIR_FREE(output); + virCommandFree(cmd); + return count; + error: + for (i = 0; i < count; i++) { + VIR_FREE((*parsedOutput)[i].assignedCPUs); + VIR_FREE((*parsedOutput)[i].failedCPUs); + } + VIR_FREE(*parsedOutput); + *parsedOutput = NULL; + VIR_FREE(output); + output = NULL; + virCommandFree(cmd); + return -1; +} + +/* + * Returns the libvirts equivalent of the cell state passed to it + */ +static virDomainState +cellToVirDomainState(struct jailhouse_cell *cell) +{ + switch (cell->state) { + case STATERUNNING: return VIR_DOMAIN_RUNNING; + case STATERUNNINGLOCKED: return VIR_DOMAIN_RUNNING; + case STATESHUTDOWN: return VIR_DOMAIN_SHUTOFF; + case STATEFAILED: return VIR_DOMAIN_CRASHED; + default: return VIR_DOMAIN_NOSTATE; + } +} + +/* + * Returns a new virDomainPtr filled with the data of the jailhouse_cell + */ +static virDomainPtr +cellToVirDomainPtr(virConnectPtr conn, struct jailhouse_cell *cell) +{ + virDomainPtr dom = virGetDomain(conn, cell->name, cell->uuid); + dom->id = cell->id; + return dom; +} + +/* + * Check cells for cell and copies UUID if found, otherwise generates a new one, this is to preserve UUID in libvirt + */ +static void setUUID(struct jailhouse_cell *cells, size_t count, struct jailhouse_cell* cell) { + size_t i; + for (i = 0; i < count; i++) { + if (strncmp(cells[i].name, cell->name, NAMELENGTH+1)) + continue; + memcpy(cell->uuid, cells[i].uuid, VIR_UUID_BUFLEN); + return; + } + virUUIDGenerate(cell->uuid); +} + +/* + * Frees the old list of cells, gets the new one and preserves UUID if cells were present in the old + */ +static void +getCurrentCellList(virConnectPtr conn) +{ + size_t lastCount = ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; + struct jailhouse_cell *lastCells = ((struct jailhouse_driver *)conn->privateData)->lastQueryCells; + struct jailhouse_cell *cells = NULL; + size_t i; + size_t count = parseListOutput(conn, &cells); + for (i = 0; i < count; i++) + setUUID(lastCells, lastCount, cells+i); + for (i = 0; i < lastCount; i++) { + VIR_FREE(lastCells[i].assignedCPUs); + VIR_FREE(lastCells[i].failedCPUs); + } + VIR_FREE(lastCells); + ((struct jailhouse_driver *)conn->privateData)->lastQueryCells = cells; + ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount = count; +} + +/* + * Converts libvirts virDomainPtr to the internal jailhouse_cell by parsing the "jailhouse cell list" output + * and looking up the name of the virDomainPtr, returns NULL if cell is no longer present + */ +static struct jailhouse_cell * +virDomainPtrToCell(virDomainPtr dom) +{ + getCurrentCellList(dom->conn); + size_t cellsCount = ((struct jailhouse_driver *)dom->conn->privateData)->lastQueryCellsCount; + struct jailhouse_cell *cells = ((struct jailhouse_driver *)dom->conn->privateData)->lastQueryCells; + size_t i; + for (i = 0; i < cellsCount; i++) + if (dom->id == cells[i].id) + return cells+i; + return NULL; +} + +static virDrvOpenStatus +jailhouseConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) +{ + virCheckFlags(0, VIR_DRV_OPEN_ERROR); + if (conn->uri->scheme == NULL || + STRNEQ(conn->uri->scheme, "jailhouse")) + return VIR_DRV_OPEN_DECLINED; + char* binary; + if (conn->uri->path == NULL) { + if (VIR_STRDUP(binary, "jailhouse") != 1) + return VIR_DRV_OPEN_ERROR; + } else { + if (!virFileIsExecutable(conn->uri->path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Path '%s', is not a valid executable file."), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + if (VIR_STRDUP(binary, conn->uri->path) != 1) + return VIR_DRV_OPEN_ERROR; + } + virCommandPtr cmd = virCommandNew(binary); + virCommandAddArg(cmd, "--version"); + virCommandAddEnvPassCommon(cmd); + char *output; + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Executing '%s --version' failed."), + conn->uri->path); + goto error; + } + if (STRNEQLEN(JAILHOUSEVERSIONOUTPUT, output, strlen(JAILHOUSEVERSIONOUTPUT))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s doesn't seem to be a correct Jailhouse binary."), + conn->uri->path); + goto error; + } + VIR_FREE(output); + virCommandFree(cmd); + struct jailhouse_driver *driver; + if (VIR_ALLOC(driver)) + return VIR_DRV_OPEN_ERROR; + driver->binary = binary; + driver->lastQueryCells = NULL; + driver->lastQueryCellsCount = 0; + conn->privateData = driver; + return VIR_DRV_OPEN_SUCCESS; + error: + VIR_FREE(output); + virCommandFree(cmd); + return VIR_DRV_OPEN_ERROR; +} + +static int +jailhouseConnectClose(virConnectPtr conn) +{ + size_t i; + size_t cellsCount = ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; + struct jailhouse_cell *cells = ((struct jailhouse_driver *)conn->privateData)->lastQueryCells; + for (i = 0; i < cellsCount; i++) { + VIR_FREE(cells[i].assignedCPUs); + VIR_FREE(cells[i].failedCPUs); + } + VIR_FREE(cells); + VIR_FREE(((struct jailhouse_driver *)conn->privateData)->binary); + VIR_FREE(conn->privateData); + conn->privateData = NULL; + return 0; +} + +static int +jailhouseConnectNumOfDomains(virConnectPtr conn) +{ + getCurrentCellList(conn); + return ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; +} + +static int +jailhouseConnectListDomains(virConnectPtr conn, int * ids, int maxids) +{ + getCurrentCellList(conn); + size_t cellsCount = ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; + struct jailhouse_cell *cells = ((struct jailhouse_driver *)conn->privateData)->lastQueryCells; + size_t i; + for (i = 0; i < maxids && i < cellsCount; i++) + ids[i] = cells[i].id; + return i; +} + +static int +jailhouseConnectListAllDomains(virConnectPtr conn, virDomainPtr ** domains, unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_ACTIVE, 0); + getCurrentCellList(conn); + size_t cellsCount = ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; + struct jailhouse_cell *cells = ((struct jailhouse_driver *)conn->privateData)->lastQueryCells; + if (cellsCount == -1) + goto error; + if (VIR_ALLOC_N(*domains, cellsCount+1)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate virDomainPtr array of size %zu"), cellsCount+1); + goto error; + } + size_t i; + for (i = 0; i < cellsCount; i++) + (*domains)[i] = cellToVirDomainPtr(conn, cells+i); + (*domains)[cellsCount] = NULL; + return cellsCount; + error: + *domains = NULL; + return -1; +} + +static virDomainPtr +jailhouseDomainLookupByID(virConnectPtr conn, int id) +{ + getCurrentCellList(conn); + size_t cellsCount = ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; + if (cellsCount == -1) + return NULL; + struct jailhouse_cell *cells = ((struct jailhouse_driver *)conn->privateData)->lastQueryCells; + size_t i; + for (i = 0; i < cellsCount; i++) + if (cells[i].id == id) + return cellToVirDomainPtr(conn, cells+i); + virReportError(VIR_ERR_NO_DOMAIN, NULL); + return NULL; +} + +static virDomainPtr +jailhouseDomainLookupByName(virConnectPtr conn, const char *lookupName) +{ + getCurrentCellList(conn); + size_t cellsCount = ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; + if (cellsCount == -1) + return NULL; + struct jailhouse_cell *cells = ((struct jailhouse_driver *)conn->privateData)->lastQueryCells; + size_t i; + for (i = 0; i < cellsCount; i++) + if (STREQ(cells[i].name, lookupName)) + return cellToVirDomainPtr(conn, cells+i); + virReportError(VIR_ERR_NO_DOMAIN, NULL); + return NULL; +} + +static virDomainPtr +jailhouseDomainLookupByUUID(virConnectPtr conn, const unsigned char * uuid) +{ + getCurrentCellList(conn); + size_t cellsCount = ((struct jailhouse_driver *)conn->privateData)->lastQueryCellsCount; + if (cellsCount == -1) + return NULL; + struct jailhouse_cell *cells = ((struct jailhouse_driver *)conn->privateData)->lastQueryCells; + size_t i; + for (i = 0; i < cellsCount; i++) + if (memcmp(cells[i].uuid, (const char*)uuid, VIR_UUID_BUFLEN) == 0) + return cellToVirDomainPtr(conn, cells+i); + virReportError(VIR_ERR_NO_DOMAIN, NULL); + return NULL; +} + +/* + * There currently is no straightforward way for the driver to retrieve those, + * so maxMem, memory and cpuTime have dummy values + */ +static int +jailhouseDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) +{ + struct jailhouse_cell *cell = virDomainPtrToCell(domain); + if (cell == NULL) + return -1; + info->state = cellToVirDomainState(cell); + info->maxMem = 1; + info->memory = 1; + info->nrVirtCpu = cell->assignedCPUsLength; + info->cpuTime = 1; + return 0; +} + +static int +jailhouseDomainGetState(virDomainPtr domain, int *state, + int *reason ATTRIBUTE_UNUSED, unsigned int flags) +{ + virCheckFlags(0, 0); + struct jailhouse_cell *cell = virDomainPtrToCell(domain); + if (cell == NULL) + return -1; + *state = cellToVirDomainState(cell); + return 0; +} + +static int +jailhouseDomainShutdown(virDomainPtr domain) +{ + virCommandPtr cmd = virCommandNew(((struct jailhouse_driver *)domain->conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "shutdown"); + char buf[IDLENGTH+1]; + snprintf(buf, IDLENGTH+1, "%d", domain->id); + virCommandAddArg(cmd, buf); + virCommandAddEnvPassCommon(cmd); + int resultcode = virCommandRun(cmd, NULL); + virCommandFree(cmd); + if (resultcode < 0) + return -1; + return 0; +} + +/* + * CAREFUL, this is the Jailhouse destroy, not the libvirt destroy, cell will be deleted and would need to be created and loaded again. + * This is implemented anyway, so libvirt clients have an option to use jailhouse destroy too. + */ +static int +jailhouseDomainDestroy(virDomainPtr domain) +{ + virCommandPtr cmd = virCommandNew(((struct jailhouse_driver *)domain->conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "destroy"); + char buf[IDLENGTH+1]; + snprintf(buf, IDLENGTH+1, "%d", domain->id); + virCommandAddArg(cmd, buf); + virCommandAddEnvPassCommon(cmd); + int resultcode = virCommandRun(cmd, NULL); + virCommandFree(cmd); + if (resultcode < 0) + return -1; + return 0; +} + +static int +jailhouseDomainCreate(virDomainPtr domain) +{ + virCommandPtr cmd = virCommandNew(((struct jailhouse_driver *)domain->conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "start"); + char buf[IDLENGTH+1]; + snprintf(buf, IDLENGTH+1, "%d", domain->id); + virCommandAddArg(cmd, buf); + virCommandAddEnvPassCommon(cmd); + int resultcode = virCommandRun(cmd, NULL); + virCommandFree(cmd); + if (resultcode < 0) + return -1; + return 0; +} + +/* + * There currently is no reason why it shouldn't be + */ +static int +jailhouseConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + +static int +jailhouseNodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr info) +{ + return nodeGetInfo(NULL, info); +} + +/* + * Returns a dummy capabilities XML for virt-manager + */ +static char * +jailhouseConnectGetCapabilities(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + char* caps; + if (VIR_STRDUP(caps, "<capabilities></capabilities>") != 1) + return NULL; + return caps; +} + +/* + * Returns a dummy XML for virt-manager + */ +static char * +jailhouseDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) +{ + virCheckFlags(0, NULL); + char buf[200]; + char uuid[VIR_UUID_STRING_BUFLEN]; + virDomainGetUUIDString(domain, uuid); + snprintf(buf, 200, "<domain type =\"jailhouse\">\n\ + <name>%s</name>\n\ + <uuid>%s</uuid>\n\ + </domain>", domain->name, uuid); + char* result; + if (VIR_STRDUP(result, buf) != 1) + return NULL; + return result; +} + +static virHypervisorDriver jailhouseHypervisorDriver = { + .name = "jailhouse", + .connectOpen = jailhouseConnectOpen, /* 1.2.22 */ + .connectClose = jailhouseConnectClose, /* 1.2.22 */ + .connectGetCapabilities = jailhouseConnectGetCapabilities, /* 1.2.22 */ + .connectNumOfDomains = jailhouseConnectNumOfDomains, /* 1.2.22 */ + .connectListDomains = jailhouseConnectListDomains, /* 1.2.22 */ + .connectIsAlive = jailhouseConnectIsAlive, /* 1.2.22 */ + .connectListAllDomains = jailhouseConnectListAllDomains, /* 1.2.22 */ + .domainLookupByID = jailhouseDomainLookupByID, /* 1.2.22 */ + .domainLookupByName = jailhouseDomainLookupByName, /* 1.2.22 */ + .domainLookupByUUID = jailhouseDomainLookupByUUID, /* 1.2.22 */ + .domainGetInfo = jailhouseDomainGetInfo, /* 1.2.22 */ + .domainGetState = jailhouseDomainGetState, /* 1.2.22 */ + .domainGetXMLDesc = jailhouseDomainGetXMLDesc, /* 1.2.22 */ + .domainShutdown = jailhouseDomainShutdown, /* 1.2.22 */ + .domainDestroy = jailhouseDomainDestroy, /* 1.2.22 */ + .domainCreate = jailhouseDomainCreate, /* 1.2.22 */ + .nodeGetInfo = jailhouseNodeGetInfo /* 1.2.22 */ +}; + +static virConnectDriver jailhouseConnectDriver = { + .hypervisorDriver = &jailhouseHypervisorDriver, +}; + +int +jailhouseRegister(void) +{ + return virRegisterConnectDriver(&jailhouseConnectDriver, + false); +} diff --git a/src/jailhouse/jailhouse_driver.h b/src/jailhouse/jailhouse_driver.h new file mode 100644 index 0000000..47c17e7 --- /dev/null +++ b/src/jailhouse/jailhouse_driver.h @@ -0,0 +1,28 @@ +/* + * jailhouse_driver.h: hypervisor driver for managing Jailhouse cells + * + * Copyright (C) 2015 Linutronix GmbH + * + * 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/>. + * + * Author: Christian Loehle + */ + +#ifndef JAILHOUSE_DRIVER_H +# define JAILHOUSE_DRIVER_H + +int jailhouseRegister(void); + +#endif diff --git a/src/libvirt.c b/src/libvirt.c index 25a0040..7626353 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -98,6 +98,9 @@ #ifdef WITH_BHYVE # include "bhyve/bhyve_driver.h" #endif +#ifdef WITH_JAILHOUSE +# include "jailhouse/jailhouse_driver.h" +#endif #define VIR_FROM_THIS VIR_FROM_NONE @@ -437,12 +440,17 @@ virGlobalInit(void) if (vzRegister() == -1) goto error; # endif +#ifdef WITH_JAILHOUSE + if (jailhouseRegister() == -1) + goto error; +#endif #endif #ifdef WITH_REMOTE if (remoteRegister() == -1) goto error; #endif + return; error: @@ -1167,6 +1175,9 @@ do_open(const char *name, #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, __FILE__, __FUNCTION__, __LINE__, diff --git a/src/util/virerror.c b/src/util/virerror.c index 6dc05f4..0d480c0 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -134,6 +134,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Polkit", /* 60 */ "Thread jobs", "Admin Interface", + "Jailhouse Driver", ) -- 2.1.4

On Wed, Nov 25, 2015 at 02:39:17PM +0100, Christian Loehle wrote:

The suggestions in your review should all be implemented now.

NB, as mentioned on IRC, for future postings please send as a top-level patch, rather than in-reply to, and use --subject-prefix 'PATCH v3' to indicate the 3rd posting for example.

diff --git a/libvirt-jailhousev2.patch b/libvirt-jailhousev2.patch new file mode 100644 index 0000000..e69de29

This file probably shouldn't have been added

diff --git a/m4/virt-driver-jailhouse.m4 b/m4/virt-driver-jailhouse.m4 new file mode 100644 index 0000000..65d53f2 --- /dev/null +++ b/m4/virt-driver-jailhouse.m4 @@ -0,0 +1,31 @@ + +AC_DEFUN([LIBVIRT_DRIVER_CHECK_JAILHOUSE],[ + AC_ARG_WITH([jailhouse], + [AS_HELP_STRING([--with-jailhouse], + [add Jailhouse support @<:@default=yes@:>@])]) + m4_divert_text([DEFAULTS], [with_jailhouse=yes]) +

You need if test "$with_jailhouse" = "yes"; then AC_DEFINE_UNQUOTED([WITH_JAILHOUSE], 1, [whether jailhouse driver is enabled]) fi Without this, your code never gets enabled in libvirt.so, so I'm not sure how you tested this patch, as the driver is never loaded.

+ AM_CONDITIONAL([WITH_JAILHOUSE], [test "$with_jailhouse" = "yes"]) +]) + +AC_DEFUN([LIBVIRT_DRIVER_RESULT_JAILHOUSE],[ + AC_MSG_NOTICE([Jailhouse: $with_jailhouse]) +])

diff --git a/src/jailhouse/README b/src/jailhouse/README new file mode 100644 index 0000000..02ba87d --- /dev/null +++ b/src/jailhouse/README @@ -0,0 +1,14 @@ +The jailhouse hypervisor driver for the libvirt project aims to provide +rudimentary support for managing jailhouse with the libvirt library. +The main advantage of this is the possibility to use virt-manager as a GUI to +manage Jailhouse cells. Thus the driver is mainly built around the API calls +that virt-manager uses and needs. +Due to the concept of Jailhouse a lot of libvirt functions can't be realized, +so this driver isn't as full-featured as upstream drivers of the +libvirt project. +Currently the driver relies on the Jailhouse binary, which has to be in $PATH +or passed when connecting a libvirt client to it +(e.g. virt-manager -c jailhouse:///path/to/jailhouse/tools/jailhouse).

As mentioned before, I really don't think we should be passing binary names in the URI like this. For everything else in libvirt we just rely on the binary being in $PATH. If someone has put jailhouse in some unusual location, then can just pre-pend that location to $PATH

+#define STATERUNNINGSTRING "running " +#define STATERUNNINGLOCKED 1 +#define STATERUNNINGLOCKEDSTRING "running/locked " +#define STATESHUTDOWN 2 +#define STATESHUTDOWNSTRING "shut down " +#define STATEFAILED 3 +#define STATEFAILEDSTRING "failed "

Looks

+#define JAILHOUSEVERSIONOUTPUT "Jailhouse management tool" + +struct virJailhouseCell { + int id; + char name[NAMELENGTH+1]; + int state; + virBitmapPtr assignedCPUs; + virBitmapPtr failedCPUs; /* currently unused */ + unsigned char uuid[VIR_UUID_BUFLEN]; +}; +typedef struct virJailhouseCell virJailhouseCell; +typedef virJailhouseCell *virJailhouseCellPtr; + +/* + * Because virCommandRunRegex Callback gets called every line + */ +struct virJailhouseCellCallbackData { + size_t ncells; + virJailhouseCellPtr cells; +}; +typedef struct virJailhouseCellCallbackData virJailhouseCellCallbackData; +typedef virJailhouseCellCallbackData *virJailhouseCellCallbackDataPtr; + +/* + * The driver requeries the cells on most calls, it stores the result of the + * last query, so it can copy the UUIDs in the new query if the cell is the + * same(otherwise it just generates a new one). + * not preserving the UUID results in a lot of bugs in libvirts clients. + */ +struct virJailhouseDriver { + char *binary; + size_t lastQueryCellsCount; + virJailhouseCellPtr lastQueryCells; +}; +typedef struct virJailhouseDriver virJailhouseDriver; +typedef virJailhouseDriver *virJailhouseDriverPtr; + +static int virJailhouseParseListOutputCallback(char **const groups, void *data) +{ + virJailhouseCellCallbackDataPtr celldata = (virJailhouseCellCallbackDataPtr) data; + virJailhouseCellPtr cells = celldata->cells; + size_t count = celldata->ncells; + char* endptr = groups[0] + strlen(groups[0]) - 1; + char* state = groups[2]; + if (cells == NULL) { + if (VIR_ALLOC(cells)) + return -1; + } + else if (VIR_EXPAND_N(cells, count, 1)) + return -1;

You are not incrementing 'count' in the first patch of the conditional, but you are incrementing it in the second. As a result, as soon as you have more than one cell running the following code will be a buffer overflow causing libvirt to crash. VIR_EXPAND_N copes just fine with cells being NULL initially, so just get rid of the VIR_ALLOC and always use VIR_EXPAND_N

+ celldata->ncells++; + if (virStrToLong_i(groups[0], &endptr, 0, &cells[count].id)) + return -1;

Then use 'count - 1' here, and on all subsequent lines

+ if (!virStrcpy(cells[count].name, groups[1], NAMELENGTH+1)) + return -1; + if (STREQLEN(state, STATERUNNINGSTRING, STATELENGTH)) + cells[count].state = STATERUNNING; + else if (STREQLEN(state, STATESHUTDOWNSTRING, STATELENGTH)) + cells[count].state = STATESHUTDOWN; + else if (STREQLEN(state, STATERUNNINGLOCKEDSTRING, STATELENGTH)) + cells[count].state = STATERUNNINGLOCKED; + else + cells[count].state = STATEFAILED; + virBitmapParse(groups[3], 0, &cells[count].assignedCPUs, VIR_DOMAIN_CPUMASK_LEN); + virBitmapParse(groups[4], 0, &cells[count].failedCPUs, VIR_DOMAIN_CPUMASK_LEN); + celldata->cells = cells; + return 0; +} + +/* + * calls "jailhouse cell list" and parses the output in an array of virJailhouseCell + * example output: + * ID Name State Assigned CPUs Failed CPUs + * 0 QEMU-VM running 0-3 + */ +static ssize_t +virJailhouseParseListOutput(virConnectPtr conn, virJailhouseCellPtr *parsedCells) +{ + int nvars[] = { 5 }; + virJailhouseCellCallbackData callbackData; + const char *regex[] = { "([0-9]{1,8})\\s*([0-9a-zA-z-]{1,24})\\s*([a-z/ ]{1,16})\\s*([0-9,-]{1,24})?\\s*([0-9,-]{1,24})?\\s*" };

The regex is invalid, so again I'm not sure how you tested this successfully. If you want to include '-' in a character range, it must be the first thing that is listed. Also 'A-z' is not valid as the end is smaller than the start - you presumably mean 'A-Z'. All in all the working regex would be const char *regex[] = { "([0-9]{1,8})\\s*([-0-9a-zA-Z]{1,24})\\s*([a-z/ ]{1,16})\\s*([-0-9,]{1,24})?\\s*([-0-9,]{1,24})?\\s*" };

+ virCommandPtr cmd = virCommandNew(((virJailhouseDriverPtr)conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "list"); + virCommandAddEnvPassCommon(cmd); + callbackData.cells = NULL; + callbackData.ncells = 0; + if (virCommandRunRegex(cmd, 1, regex, nvars, &virJailhouseParseListOutputCallback, &callbackData, NULL) < 0) { + virCommandFree(cmd); + return -1; + } + virCommandFree(cmd); + *parsedCells = callbackData.cells; + return callbackData.ncells; +} + +/* + * Returns the libvirts equivalent of the cell state passed to it + */ +static virDomainState +virJailhouseCellToState(virJailhouseCellPtr cell) +{ + switch (cell->state) { + case STATERUNNING: return VIR_DOMAIN_RUNNING; + case STATERUNNINGLOCKED: return VIR_DOMAIN_RUNNING; + case STATESHUTDOWN: return VIR_DOMAIN_SHUTOFF; + case STATEFAILED: return VIR_DOMAIN_CRASHED; + default: return VIR_DOMAIN_NOSTATE; + } +} + +/* + * Returns a new virDomainPtr filled with the data of the virJailhouseCell + */ +static virDomainPtr +virJailhouseCellToDomainPtr(virConnectPtr conn, virJailhouseCellPtr cell) +{ + virDomainPtr dom = virGetDomain(conn, cell->name, cell->uuid); + dom->id = cell->id; + return dom; +} + +/* + * Check cells for cell and copies UUID if found, otherwise generates a new one, this is to preserve UUID in libvirt + */ +static void virJailhouseSetUUID(virJailhouseCellPtr cells, size_t count, virJailhouseCellPtr cell) +{ + size_t i; + for (i = 0; i < count; i++) { + if (strncmp(cells[i].name, cell->name, NAMELENGTH+1)) + continue; + memcpy(cell->uuid, cells[i].uuid, VIR_UUID_BUFLEN); + return; + } + virUUIDGenerate(cell->uuid); +} + +/* + * Frees the old list of cells, gets the new one and preserves UUID if cells were present in the old + */ +static void +virJailhouseGetCurrentCellList(virConnectPtr conn) +{ + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)conn->privateData; + ssize_t count; + size_t i; + size_t lastCount = driver->lastQueryCellsCount; + virJailhouseCellPtr lastCells = driver->lastQueryCells; + virJailhouseCellPtr cells = NULL; + count = virJailhouseParseListOutput(conn, &cells); + if (count == -1) + count = 0;

This causes you to silently ignore all errors from virJailhouseParseListOutput which is clearly wrong - this hid the fact that your regex was broken and so not returning any cell info for example. You need to make this function return an 'int' and make sure all callers check & propagate the error.

+ for (i = 0; i < count; i++) + virJailhouseSetUUID(lastCells, lastCount, cells+i); + for (i = 0; i < lastCount; i++) { + virBitmapFree(lastCells[i].assignedCPUs); + virBitmapFree(lastCells[i].failedCPUs); + } + VIR_FREE(lastCells); + driver->lastQueryCells = cells; + driver->lastQueryCellsCount = count; +}

+static virJailhouseCellPtr +virDomainPtrToCell(virDomainPtr dom) +{ + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)dom->conn->privateData;

The cast '(virJailhouseDriverPtr)' is not needed - 'void*' casts to any type automatically.

+ size_t cellsCount; + size_t i; + virJailhouseGetCurrentCellList(dom->conn);

eg this must check for errors.

+ cellsCount = driver->lastQueryCellsCount; + virJailhouseCellPtr cells = driver->lastQueryCells; + for (i = 0; i < cellsCount; i++) + if (dom->id == cells[i].id) + return cells+i; + return NULL; +} + +static virDrvOpenStatus +jailhouseConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) +{ + virCheckFlags(0, VIR_DRV_OPEN_ERROR); + char* binary; + char *output; + if (conn->uri->scheme == NULL || + STRNEQ(conn->uri->scheme, "jailhouse")) + return VIR_DRV_OPEN_DECLINED; + if (conn->uri->path == NULL) { + if (VIR_STRDUP(binary, "jailhouse") != 1) + return VIR_DRV_OPEN_ERROR; + } else { + if (!virFileIsExecutable(conn->uri->path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Path '%s', is not a valid executable file."), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + if (VIR_STRDUP(binary, conn->uri->path) != 1) + return VIR_DRV_OPEN_ERROR;

As mentioned earlier please drop this bit of URI parsing. We should just accept the empty path or '/' as we do with LXC, eg /* If path isn't '/' then they typoed, tell them correct path */ if (conn->uri->path != NULL && STRNEQ(conn->uri->path, "/")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unexpected Jailhouse URI path '%s', try jailhouse:///"), conn->uri->path); return VIR_DRV_OPEN_ERROR; }

+ } + virCommandPtr cmd = virCommandNew(binary); + virCommandAddArg(cmd, "--version"); + virCommandAddEnvPassCommon(cmd); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Executing '%s --version' failed."), + conn->uri->path); + goto error; + } + if (STRNEQLEN(JAILHOUSEVERSIONOUTPUT, output, strlen(JAILHOUSEVERSIONOUTPUT))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s doesn't seem to be a correct Jailhouse binary."), + conn->uri->path); + goto error; + } + VIR_FREE(output); + virCommandFree(cmd); + virJailhouseDriverPtr driver; + if (VIR_ALLOC(driver) < 0) + return VIR_DRV_OPEN_ERROR; + driver->binary = binary; + driver->lastQueryCells = NULL; + driver->lastQueryCellsCount = 0; + conn->privateData = driver; + return VIR_DRV_OPEN_SUCCESS; + error: + VIR_FREE(output); + virCommandFree(cmd); + return VIR_DRV_OPEN_ERROR; +}

In generall all this code would benefit from having some blank lines inserted between logical blocks

+ +static int +jailhouseConnectClose(virConnectPtr conn) +{ + + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)conn->privateData; + size_t i; + size_t cellsCount = driver->lastQueryCellsCount; + virJailhouseCellPtr cells = driver->lastQueryCells; + for (i = 0; i < cellsCount; i++) { + virBitmapFree(cells[i].assignedCPUs); + virBitmapFree(cells[i].failedCPUs); + } + VIR_FREE(cells); + VIR_FREE(driver->binary); + VIR_FREE(driver); + conn->privateData = NULL; + return 0; +} + +static int +jailhouseConnectNumOfDomains(virConnectPtr conn) +{ + virJailhouseGetCurrentCellList(conn); + return ((virJailhouseDriverPtr)conn->privateData)->lastQueryCellsCount; +} + +static int +jailhouseConnectListDomains(virConnectPtr conn, int * ids, int maxids) +{ + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)conn->privateData; + size_t cellsCount; + size_t i; + virJailhouseGetCurrentCellList(conn); + cellsCount = driver->lastQueryCellsCount; + virJailhouseCellPtr cells = driver->lastQueryCells; + for (i = 0; i < maxids && i < cellsCount; i++) + ids[i] = cells[i].id; + return i; +} + +static int +jailhouseConnectListAllDomains(virConnectPtr conn, virDomainPtr ** domains, unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_ACTIVE, 0); + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)conn->privateData; + size_t cellsCount; + size_t i; + virJailhouseGetCurrentCellList(conn); + cellsCount = driver->lastQueryCellsCount; + virJailhouseCellPtr cells = driver->lastQueryCells;

Normally we expect all variables to be decalred before any code is run. So please re-arrange this accordingly

+ if (cellsCount == -1) + goto error; + if (VIR_ALLOC_N(*domains, cellsCount+1) < 0) + goto error; + for (i = 0; i < cellsCount; i++) + (*domains)[i] = virJailhouseCellToDomainPtr(conn, cells+i); + (*domains)[cellsCount] = NULL; + return cellsCount; + error: + *domains = NULL; + return -1; +} + +static virDomainPtr +jailhouseDomainLookupByID(virConnectPtr conn, int id) +{ + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)conn->privateData; + size_t cellsCount; + size_t i; + virJailhouseGetCurrentCellList(conn); + cellsCount = driver->lastQueryCellsCount; + if (cellsCount == -1) + return NULL; + virJailhouseCellPtr cells = driver->lastQueryCells; + for (i = 0; i < cellsCount; i++) + if (cells[i].id == id) + return virJailhouseCellToDomainPtr(conn, cells+i); + virReportError(VIR_ERR_NO_DOMAIN, NULL); + return NULL; +} + +static virDomainPtr +jailhouseDomainLookupByName(virConnectPtr conn, const char *lookupName) +{ + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)conn->privateData; + size_t cellsCount; + size_t i; + virJailhouseGetCurrentCellList(conn); + cellsCount = driver->lastQueryCellsCount; + if (cellsCount == -1) + return NULL; + virJailhouseCellPtr cells = driver->lastQueryCells; + for (i = 0; i < cellsCount; i++) + if (STREQ(cells[i].name, lookupName)) + return virJailhouseCellToDomainPtr(conn, cells+i); + virReportError(VIR_ERR_NO_DOMAIN, NULL); + return NULL; +} + +static virDomainPtr +jailhouseDomainLookupByUUID(virConnectPtr conn, const unsigned char * uuid) +{ + virJailhouseDriverPtr driver = (virJailhouseDriverPtr)conn->privateData; + size_t cellsCount; + size_t i; + virJailhouseGetCurrentCellList(conn); + cellsCount = driver->lastQueryCellsCount; + if (cellsCount == -1) + return NULL; + virJailhouseCellPtr cells = driver->lastQueryCells; + for (i = 0; i < cellsCount; i++) + if (memcmp(cells[i].uuid, (const char*)uuid, VIR_UUID_BUFLEN) == 0) + return virJailhouseCellToDomainPtr(conn, cells+i); + virReportError(VIR_ERR_NO_DOMAIN, NULL); + return NULL; +} + +/* + * There currently is no straightforward way for the driver to retrieve those, + * so maxMem, memory and cpuTime have dummy values + */ +static int +jailhouseDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) +{ + virJailhouseCellPtr cell = virDomainPtrToCell(domain); + if (cell == NULL) + return -1; + info->state = virJailhouseCellToState(cell); + info->maxMem = 0; + info->memory = 0; + info->nrVirtCpu = virBitmapCountBits(cell->assignedCPUs); + info->cpuTime = 0; + return 0; +} + +static int +jailhouseDomainGetState(virDomainPtr domain, int *state, + int *reason ATTRIBUTE_UNUSED, unsigned int flags) +{ + virCheckFlags(0, 0); + virJailhouseCellPtr cell = virDomainPtrToCell(domain); + if (cell == NULL) + return -1; + *state = virJailhouseCellToState(cell); + return 0; +} + +static int +jailhouseDomainShutdown(virDomainPtr domain) +{ + int resultcode; + virCommandPtr cmd = virCommandNew(((virJailhouseDriverPtr)domain->conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "shutdown"); + virCommandAddArgFormat(cmd, "%d", domain->id); + virCommandAddEnvPassCommon(cmd); + resultcode = virCommandRun(cmd, NULL); + virCommandFree(cmd); + if (resultcode < 0) + return -1; + return 0; +}

The 'shutdown' command is intended to tell the guest init system todo a graceful shutdown. AFAICT, this is not what 'cell shutdown' does - it is akin to immediately turning off the power. So this should be called from the destroy method in libvirt.

+/* + * CAREFUL, this is the Jailhouse destroy, not the libvirt destroy, + * cell will be deleted and would need to be created and loaded again. + * This is implemented anyway, so libvirt clients have an option to use jailhouse destroy too. + */

THis is bad - drivers should have consistent semantics for the same operations, as apps using libvirt are not going to read this comment.

+static int +jailhouseDomainDestroy(virDomainPtr domain) +{ + int resultcode; + virCommandPtr cmd = virCommandNew(((virJailhouseDriverPtr)domain->conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "destroy"); + virCommandAddArgFormat(cmd, "%d", domain->id); + virCommandAddEnvPassCommon(cmd); + resultcode = virCommandRun(cmd, NULL); + virCommandFree(cmd); + if (resultcode < 0) + return -1; + return 0;

So this should be running 'shutdown' not 'destroy' IMHO

+} + +static int +jailhouseDomainCreate(virDomainPtr domain) +{ + int resultcode; + virCommandPtr cmd = virCommandNew(((virJailhouseDriverPtr)domain->conn->privateData)->binary); + virCommandAddArg(cmd, "cell"); + virCommandAddArg(cmd, "start"); + virCommandAddArgFormat(cmd, "%d", domain->id); + virCommandAddEnvPassCommon(cmd); + resultcode = virCommandRun(cmd, NULL); + virCommandFree(cmd); + if (resultcode < 0) + return -1; + return 0; +} + +/* + * There currently is no reason why it shouldn't be + */ +static int +jailhouseConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 1; +} + +static int +jailhouseNodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr info) +{ + return nodeGetInfo(NULL, info); +} + +/* + * Returns a dummy capabilities XML for virt-manager + */ +static char * +jailhouseConnectGetCapabilities(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + virCapsPtr caps = virCapabilitiesNew(VIR_ARCH_NONE, false, false); + char* xml = virCapabilitiesFormatXML(caps); + virObjectUnref(caps); + return xml; +} + +static char * +jailhouseDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) +{ + virCheckFlags(0, NULL); + char* xml; + virDomainDefPtr domainDef = virDomainDefNewFull(domain->name, domain->uuid, domain->id); + xml = virDomainDefFormat(domainDef, 0); + virObjectUnref(domainDef);

This causes libvirt to crash, because virDOmainDef is not an object, you must call virDomainDefFree() instead

+ return xml; +} + +static virHypervisorDriver jailhouseHypervisorDriver = { + .name = "jailhouse", + .connectOpen = jailhouseConnectOpen, /* 1.3.0 */ + .connectClose = jailhouseConnectClose, /* 1.3.0 */ + .connectGetCapabilities = jailhouseConnectGetCapabilities, /* 1.3.0 */ + .connectNumOfDomains = jailhouseConnectNumOfDomains, /* 1.3.0 */ + .connectListDomains = jailhouseConnectListDomains, /* 1.3.0 */ + .connectIsAlive = jailhouseConnectIsAlive, /* 1.3.0 */ + .connectListAllDomains = jailhouseConnectListAllDomains, /* 1.3.0 */ + .domainLookupByID = jailhouseDomainLookupByID, /* 1.3.0 */ + .domainLookupByName = jailhouseDomainLookupByName, /* 1.3.0 */ + .domainLookupByUUID = jailhouseDomainLookupByUUID, /* 1.3.0 */ + .domainGetInfo = jailhouseDomainGetInfo, /* 1.3.0 */ + .domainGetState = jailhouseDomainGetState, /* 1.3.0 */ + .domainGetXMLDesc = jailhouseDomainGetXMLDesc, /* 1.3.0 */ + .domainShutdown = jailhouseDomainShutdown, /* 1.3.0 */ + .domainDestroy = jailhouseDomainDestroy, /* 1.3.0 */ + .domainCreate = jailhouseDomainCreate, /* 1.3.0 */ + .nodeGetInfo = jailhouseNodeGetInfo /* 1.3.0 */ +};

Next time you post bump these versions to 1.3.1 since we've released 1.3.0 now.

+ +static virConnectDriver jailhouseConnectDriver = { + .hypervisorDriver = &jailhouseHypervisorDriver, +}; + +int +jailhouseRegister(void) +{ + return virRegisterConnectDriver(&jailhouseConnectDriver, + false); +}

In summary, there's nothing particularly bad about this, but this patch clearly hasn't been tested, since there are 3 show stopping bugs which prevent it working at all. Please make sure patches are at least somewhat tested to work before posting them Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|