[libvirt] [PATCH v9] bhyve: add a basic driver

Changes from v8: - Implement domainUndefine and two more functions it depends on: domainIsActive and domainIsPersistent Changes from v7: - Squashed in ACL support - Check for disk and bus type for bhyve and disk type for bhyveload - Handle case when URI == NULL in ConnectOpen - Call bhyveload only after we've built bhyve command to avoid unneeded load/reload for wrong domain configuration - Cleanup unload calls on errors - Minor style fixes Changes from v6: - Fix typo: s/LIBIVRT_DRIVER_RESULT_BHYVE/LIBVIRT_DRIVER_RESULT_BHYVE/ - Report domain state in 'dominfo' - Add a patch which implements ACL support Now both 'make check' and 'make syntax-check' pass. Changes from v5: - Obtain version using uname(3) - Cleanup driver global objects in StateCleanup instead of ConnectClose Changes from v4: - Set acpi and apic flags based on domain definition - Add more detailed description about -H and -P flags of bhyve to justify theirs usage Roman Bogorodskiy (1): bhyve: add a basic driver configure.ac | 7 + daemon/libvirtd.c | 9 + include/libvirt/virterror.h | 1 + m4/virt-driver-bhyve.m4 | 57 ++++ po/POTFILES.in | 3 + src/Makefile.am | 31 ++ src/bhyve/bhyve_command.c | 314 ++++++++++++++++++++ src/bhyve/bhyve_command.h | 41 +++ src/bhyve/bhyve_driver.c | 707 ++++++++++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_driver.h | 28 ++ src/bhyve/bhyve_process.c | 227 ++++++++++++++ src/bhyve/bhyve_process.h | 36 +++ src/bhyve/bhyve_utils.h | 49 +++ src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + src/driver.h | 1 + src/libvirt.c | 3 + src/util/virerror.c | 1 + 18 files changed, 1518 insertions(+), 1 deletion(-) create mode 100644 m4/virt-driver-bhyve.m4 create mode 100644 src/bhyve/bhyve_command.c create mode 100644 src/bhyve/bhyve_command.h create mode 100644 src/bhyve/bhyve_driver.c create mode 100644 src/bhyve/bhyve_driver.h create mode 100644 src/bhyve/bhyve_process.c create mode 100644 src/bhyve/bhyve_process.h create mode 100644 src/bhyve/bhyve_utils.h -- 1.8.4.3

At this point it has a limited functionality and is highly experimental. Supported domain operations are: * define * start * destroy * dumpxml * dominfo * undefine It's only possible to have only one disk device and only one network, which should be of type bridge. --- configure.ac | 7 + daemon/libvirtd.c | 9 + include/libvirt/virterror.h | 1 + m4/virt-driver-bhyve.m4 | 57 ++++ po/POTFILES.in | 3 + src/Makefile.am | 31 ++ src/bhyve/bhyve_command.c | 314 ++++++++++++++++++++ src/bhyve/bhyve_command.h | 41 +++ src/bhyve/bhyve_driver.c | 707 ++++++++++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_driver.h | 28 ++ src/bhyve/bhyve_process.c | 227 ++++++++++++++ src/bhyve/bhyve_process.h | 36 +++ src/bhyve/bhyve_utils.h | 49 +++ src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + src/driver.h | 1 + src/libvirt.c | 3 + src/util/virerror.c | 1 + 18 files changed, 1518 insertions(+), 1 deletion(-) create mode 100644 m4/virt-driver-bhyve.m4 create mode 100644 src/bhyve/bhyve_command.c create mode 100644 src/bhyve/bhyve_command.h create mode 100644 src/bhyve/bhyve_driver.c create mode 100644 src/bhyve/bhyve_driver.h create mode 100644 src/bhyve/bhyve_process.c create mode 100644 src/bhyve/bhyve_process.h create mode 100644 src/bhyve/bhyve_utils.h diff --git a/configure.ac b/configure.ac index 0d505d3..47bc427 100644 --- a/configure.ac +++ b/configure.ac @@ -1048,6 +1048,12 @@ fi AM_CONDITIONAL([WITH_PARALLELS], [test "$with_parallels" = "yes"]) dnl +dnl Checks for bhyve driver +dnl + +LIBVIRT_DRIVER_CHECK_BHYVE + +dnl dnl check for shell that understands <> redirection without truncation, dnl needed by src/qemu/qemu_monitor_{text,json}.c. dnl @@ -2705,6 +2711,7 @@ AC_MSG_NOTICE([ PHYP: $with_phyp]) AC_MSG_NOTICE([ ESX: $with_esx]) AC_MSG_NOTICE([ Hyper-V: $with_hyperv]) AC_MSG_NOTICE([Parallels: $with_parallels]) +LIBVIRT_DRIVER_RESULT_BHYVE AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network]) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 49c42ad..b27c6fd 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -77,6 +77,9 @@ # ifdef WITH_VBOX # include "vbox/vbox_driver.h" # endif +# ifdef WITH_BHYVE +# include "bhyve/bhyve_driver.h" +# endif # ifdef WITH_NETWORK # include "network/bridge_driver.h" # endif @@ -405,6 +408,9 @@ static void daemonInitialize(void) # ifdef WITH_VBOX virDriverLoadModule("vbox"); # endif +# ifdef WITH_BHYVE + virDriverLoadModule("bhyve"); +# endif #else # ifdef WITH_NETWORK networkRegister(); @@ -442,6 +448,9 @@ static void daemonInitialize(void) # ifdef WITH_VBOX vboxRegister(); # endif +# ifdef WITH_BHYVE + bhyveRegister(); +# endif #endif } diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index e31e9c4..7915bbb 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -121,6 +121,7 @@ typedef enum { VIR_FROM_ACCESS = 55, /* Error from access control manager */ VIR_FROM_SYSTEMD = 56, /* Error from systemd code */ + VIR_FROM_BHYVE = 57, /* Error from bhyve driver */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST # endif diff --git a/m4/virt-driver-bhyve.m4 b/m4/virt-driver-bhyve.m4 new file mode 100644 index 0000000..f447fd8 --- /dev/null +++ b/m4/virt-driver-bhyve.m4 @@ -0,0 +1,57 @@ +dnl The bhyve driver +dnl +dnl Copyright (C) 2014 Roman Bogorodskiy +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl + +AC_DEFUN([LIBVIRT_DRIVER_CHECK_BHYVE],[ + AC_ARG_WITH([bhyve], + [AS_HELP_STRING([--with-bhyve], + [add BHyVe support @<:@default=check@:>@])]) + m4_divert_text([DEFAULTS], [with_bhyve=check]) + + if test "$with_bhyve" != "no"; then + AC_PATH_PROG([BHYVE], [bhyve], [], [$PATH:/usr/sbin]) + AC_PATH_PROG([BHYVECTL], [bhyvectl], [$PATH:/usr/sbin]) + AC_PATH_PROG([BHYVELOAD], [bhyveload], [$PATH:/usr/sbin/]) + + if test -z "$BHYVE" || test -z "$BHYVECTL" \ + test -z "$BHYVELOAD" || test "$with_freebsd" = "no"; then + if test "$with_bhyve" = "check"; then + with_bhyve="no" + else + AC_MSG_ERROR([The bhyve driver cannot be enabled]) + fi + else + with_bhyve="yes" + fi + fi + + if test "$with_bhyve" = "yes"; then + AC_DEFINE_UNQUOTED([WITH_BHYVE], 1, [whether bhyve driver is enabled]) + AC_DEFINE_UNQUOTED([BHYVE], ["$BHYVE"], + [Location of the bhyve tool]) + AC_DEFINE_UNQUOTED([BHYVECTL], ["$BHYVECTL"], + [Location of the bhyvectl tool]) + AC_DEFINE_UNQUOTED([BHYVELOAD], ["$BHYVELOAD"], + [Location of the bhyveload tool]) + fi + AM_CONDITIONAL([WITH_BHYVE], [test "$with_bhyve" = "yes"]) +]) + +AC_DEFUN([LIBVIRT_DRIVER_RESULT_BHYVE],[ + AC_MSG_NOTICE([ Bhyve: $with_bhyve]) +]) diff --git a/po/POTFILES.in b/po/POTFILES.in index 0359b2f..5c9bc7e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -8,6 +8,9 @@ gnulib/lib/gai_strerror.c gnulib/lib/regcomp.c src/access/viraccessdriverpolkit.c src/access/viraccessmanager.c +src/bhyve/bhyve_command.c +src/bhyve/bhyve_driver.c +src/bhyve/bhyve_process.c src/conf/capabilities.c src/conf/cpu_conf.c src/conf/device_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index 3f8d22f..d0aa18d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -512,6 +512,7 @@ DRIVER_SOURCE_FILES = \ $(NULL) STATEFUL_DRIVER_SOURCE_FILES = \ + $(BHYVE_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(LIBXL_DRIVER_SOURCES) \ $(LXC_DRIVER_SOURCES) \ @@ -771,6 +772,15 @@ PARALLELS_DRIVER_SOURCES = \ parallels/parallels_storage.c \ parallels/parallels_network.c +BHYVE_DRIVER_SOURCES = \ + bhyve/bhyve_command.c \ + bhyve/bhyve_command.h \ + bhyve/bhyve_driver.h \ + bhyve/bhyve_driver.c \ + bhyve/bhyve_process.c \ + bhyve/bhyve_process.h \ + bhyve/bhyve_utils.h + NETWORK_DRIVER_SOURCES = \ network/bridge_driver.h network/bridge_driver.c \ network/bridge_driver_platform.h \ @@ -1307,6 +1317,26 @@ libvirt_driver_parallels_la_CFLAGS = \ libvirt_driver_parallels_la_SOURCES = $(PARALLELS_DRIVER_SOURCES) endif WITH_PARALLELS +if WITH_BHYVE +noinst_LTLIBRARIES += libvirt_driver_bhyve_impl.la +libvirt_driver_bhyve_la_SOURCES = +libvirt_driver_bhyve_la_LIBADD = libvirt_driver_bhyve_impl.la +if WITH_DRIVER_MODULES +mod_LTLIBRARIES += libvirt_driver_bhyve.la +libvirt_driver_bhyve_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_bhyve_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +else ! WITH_DRIVER_MODULES +noinst_LTLIBRARIES += libvirt_driver_bhyve.la +endif ! WITH_DRIVER_MODULES + +libvirt_driver_bhyve_impl_la_CFLAGS = \ + -I$(top_srcdir)/src/access \ + -I$(top_srcdir)/src/conf \ + $(AM_CFLAGS) +libvirt_driver_bhyve_impl_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_driver_bhyve_impl_la_SOURCES = $(BHYVE_DRIVER_SOURCES) +endif WITH_BHYVE + if WITH_NETWORK noinst_LTLIBRARIES += libvirt_driver_network_impl.la libvirt_driver_network_la_SOURCES = @@ -1641,6 +1671,7 @@ EXTRA_DIST += \ $(HYPERV_DRIVER_SOURCES) \ $(HYPERV_DRIVER_EXTRA_DIST) \ $(PARALLELS_DRIVER_SOURCES) \ + $(BHYVE_DRIVER_SOURCES) \ $(NETWORK_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(STORAGE_DRIVER_SOURCES) \ diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c new file mode 100644 index 0000000..1f9be9d --- /dev/null +++ b/src/bhyve/bhyve_command.c @@ -0,0 +1,314 @@ +/* + * bhyve_process.c: bhyve command generation + * + * Copyright (C) 2014 Roman Bogorodskiy + * + * 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 <fcntl.h> +#include <sys/types.h> +#include <dirent.h> +#include <sys/ioctl.h> +#include <net/if.h> +#include <net/if_tap.h> + +#include "bhyve_command.h" +#include "viralloc.h" +#include "virfile.h" +#include "virstring.h" +#include "virlog.h" +#include "virnetdev.h" +#include "virnetdevbridge.h" +#include "virnetdevtap.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +static char* +virBhyveTapGetRealDeviceName(char *name) +{ + /* This is an ugly hack, because if we rename + * tap device to vnet%d, its device name will be + * still /dev/tap%d, and bhyve tries too open /dev/tap%d, + * so we have to find the real name + */ + char *ret = NULL; + struct dirent *dp; + char *devpath = NULL; + int fd; + + DIR *dirp = opendir("/dev"); + if (dirp == NULL) { + virReportSystemError(errno, + _("Failed to opendir path '%s'"), + "/dev"); + return NULL; + } + + while ((dp = readdir(dirp)) != NULL) { + if (STRPREFIX(dp->d_name, "tap")) { + struct ifreq ifr; + if (virAsprintf(&devpath, "/dev/%s", dp->d_name) < 0) { + goto cleanup; + } + if ((fd = open(devpath, O_RDWR)) < 0) { + virReportSystemError(errno, _("Unable to open '%s'"), devpath); + goto cleanup; + } + + if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query tap interface name")); + goto cleanup; + } + + if (STREQ(name, ifr.ifr_name)) { + /* we can ignore the return value + * because we still have nothing + * to do but return; + */ + ignore_value(VIR_STRDUP(ret, dp->d_name)); + goto cleanup; + } + + VIR_FREE(devpath); + VIR_FORCE_CLOSE(fd); + } + } + +cleanup: + VIR_FREE(devpath); + VIR_FORCE_CLOSE(fd); + closedir(dirp); + return ret; +} + +static int +bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd) +{ + virDomainNetDefPtr net = NULL; + char *brname = NULL; + char *realifname = NULL; + int *tapfd = NULL; + + if (def->nnets != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("domain should have one and only one net defined")); + return -1; + } + + net = def->nets[0]; + + if (net != NULL) { + int actualType = virDomainNetGetActualType(net); + + if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0) + return -1L; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Network type %d is not supported"), + virDomainNetGetActualType(net)); + return -1; + } + + if (!net->ifname || + STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || + strchr(net->ifname, '%')) { + VIR_FREE(net->ifname); + if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0) { + VIR_FREE(brname); + return -1; + } + } + + if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, + def->uuid, tapfd, 1, + virDomainNetGetActualVirtPortProfile(net), + virDomainNetGetActualVlan(net), + VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { + VIR_FREE(net->ifname); + VIR_FREE(brname); + return -1; + } + } + + realifname = virBhyveTapGetRealDeviceName(net->ifname); + + if (realifname == NULL) { + VIR_FREE(net->ifname); + VIR_FREE(brname); + return -1; + } + + VIR_DEBUG("%s -> %s", net->ifname, realifname); + /* hack on top of other hack: we need to set + * interface to 'UP' again after re-opening to find its + * name + */ + if (virNetDevSetOnline(net->ifname, true) != 0) { + VIR_FREE(net->ifname); + VIR_FREE(brname); + return -1; + } + + virCommandAddArg(cmd, "-s"); + virCommandAddArg(cmd, "0:0,hostbridge"); + virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "1:0,virtio-net,%s", realifname); + + return 0; +} + +static int +bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) +{ + virDomainDiskDefPtr disk; + + if (def->ndisks != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("domain should have one and only one disk defined")); + return -1; + } + + disk = def->disks[0]; + + if (disk->bus != VIR_DOMAIN_DISK_BUS_SATA) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported disk bus type")); + return -1; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported disk type")); + return -1; + } + + virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "2:0,ahci-hd,%s", disk->src); + + return 0; +} + +virCommandPtr +virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + /* + * /usr/sbin/bhyve -c 2 -m 256 -AI -H -P \ + * -s 0:0,hostbridge \ + * -s 1:0,virtio-net,tap0 \ + * -s 2:0,ahci-hd,${IMG} \ + * -S 31,uart,stdio \ + * vm0 + */ + virCommandPtr cmd = virCommandNew(BHYVE); + + /* CPUs */ + virCommandAddArg(cmd, "-c"); + virCommandAddArgFormat(cmd, "%d", vm->def->vcpus); + + /* Memory */ + virCommandAddArg(cmd, "-m"); + virCommandAddArgFormat(cmd, "%llu", + VIR_DIV_UP(vm->def->mem.max_balloon, 1024)); + + /* Options */ + if (vm->def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_DOMAIN_FEATURE_STATE_ON) + virCommandAddArg(cmd, "-A"); /* Create an ACPI table */ + if (vm->def->features[VIR_DOMAIN_FEATURE_APIC] == VIR_DOMAIN_FEATURE_STATE_ON) + virCommandAddArg(cmd, "-I"); /* Present ioapic to the guest */ + + /* Clarification about -H and -P flags from Peter Grehan: + * -H and -P flags force the guest to exit when it executes IA32 HLT and PAUSE + * instructions respectively. + * + * For the HLT exit, bhyve uses that to infer that the guest is idling and can + * be put to sleep until an external event arrives. If this option is not used, + * the guest will always use 100% of CPU on the host. + * + * The PAUSE exit is most useful when there are large numbers of guest VMs running, + * since it forces the guest to exit when it spins on a lock acquisition. + */ + virCommandAddArg(cmd, "-H"); /* vmexit from guest on hlt */ + virCommandAddArg(cmd, "-P"); /* vmexit from guest on pause */ + + /* Devices */ + if (bhyveBuildNetArgStr(vm->def, cmd) < 0) + goto error; + if (bhyveBuildDiskArgStr(vm->def, cmd) < 0) + goto error; + virCommandAddArg(cmd, vm->def->name); + + return cmd; + +error: + virCommandFree(cmd); + return NULL; +} + +virCommandPtr +virBhyveProcessBuildDestroyCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + virCommandPtr cmd = virCommandNew(BHYVECTL); + + virCommandAddArg(cmd, "--destroy"); + virCommandAddArgPair(cmd, "--vm", vm->def->name); + + return cmd; +} + +virCommandPtr +virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + virCommandPtr cmd; + virDomainDiskDefPtr disk; + + if (vm->def->ndisks != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("domain should have one and only one disk defined")); + return NULL; + } + + disk = vm->def->disks[0]; + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported disk type")); + return NULL; + } + + cmd = virCommandNew(BHYVELOAD); + + /* Memory */ + virCommandAddArg(cmd, "-m"); + virCommandAddArgFormat(cmd, "%llu", + VIR_DIV_UP(vm->def->mem.max_balloon, 1024)); + + /* Image path */ + virCommandAddArg(cmd, "-d"); + virCommandAddArgFormat(cmd, disk->src); + + /* VM name */ + virCommandAddArg(cmd, vm->def->name); + + return cmd; +} diff --git a/src/bhyve/bhyve_command.h b/src/bhyve/bhyve_command.h new file mode 100644 index 0000000..8326971 --- /dev/null +++ b/src/bhyve/bhyve_command.h @@ -0,0 +1,41 @@ +/* + * bhyve_process.c: bhyve command generation + * + * Copyright (C) 2014 Roman Bogorodskiy + * + * 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/>. + * + */ + +#ifndef __BHYVE_COMMAND_H__ +# define __BHYVE_COMMAND_H__ + +# include "bhyve_utils.h" + +# include "domain_conf.h" +# include "vircommand.h" + +virCommandPtr virBhyveProcessBuildBhyveCmd(bhyveConnPtr, + virDomainObjPtr vm); + +virCommandPtr +virBhyveProcessBuildDestroyCmd(bhyveConnPtr driver, + virDomainObjPtr vm); + +virCommandPtr +virBhyveProcessBuildLoadCmd(bhyveConnPtr driver, + virDomainObjPtr vm); + +#endif /* __BHYVE_COMMAND_H__ */ diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c new file mode 100644 index 0000000..df7ce6a --- /dev/null +++ b/src/bhyve/bhyve_driver.c @@ -0,0 +1,707 @@ +/* + * bhyve_driver.c: core driver methods for managing bhyve guests + * + * Copyright (C) 2013 Roman Bogorodskiy + * + * 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: Roman Bogorodskiy + */ + +#include <config.h> + +#include <sys/utsname.h> + +#include "virerror.h" +#include "datatypes.h" +#include "virbuffer.h" +#include "viruuid.h" +#include "capabilities.h" +#include "configmake.h" +#include "viralloc.h" +#include "network_conf.h" +#include "interface_conf.h" +#include "domain_audit.h" +#include "domain_conf.h" +#include "snapshot_conf.h" +#include "fdstream.h" +#include "storage_conf.h" +#include "node_device_conf.h" +#include "virxml.h" +#include "virthread.h" +#include "virlog.h" +#include "virfile.h" +#include "virtypedparam.h" +#include "virrandom.h" +#include "virstring.h" +#include "cpu/cpu.h" +#include "viraccessapicheck.h" + +#include "bhyve_driver.h" +#include "bhyve_process.h" +#include "bhyve_utils.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +bhyveConnPtr bhyve_driver = NULL; + +void +bhyveDriverLock(bhyveConnPtr driver) +{ + virMutexLock(&driver->lock); +} + +void +bhyveDriverUnlock(bhyveConnPtr driver) +{ + virMutexUnlock(&driver->lock); +} + +static virCapsPtr +bhyveBuildCapabilities(void) +{ + virCapsPtr caps; + virCapsGuestPtr guest; + + if ((caps = virCapabilitiesNew(virArchFromHost(), + 0, 0)) == NULL) + return NULL; + + if ((guest = virCapabilitiesAddGuest(caps, "hvm", + VIR_ARCH_X86_64, + "bhyve", + NULL, 0, NULL)) == NULL) + goto error; + + if (virCapabilitiesAddGuestDomain(guest, + "bhyve", NULL, NULL, 0, NULL) == NULL) + goto error; + + return caps; + +error: + virObjectUnref(caps); + return NULL; +} + +static char * +bhyveConnectGetCapabilities(virConnectPtr conn) +{ + bhyveConnPtr privconn = conn->privateData; + char *xml; + + if (virConnectGetCapabilitiesEnsureACL(conn) < 0) + return NULL; + + bhyveDriverLock(privconn); + if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL) + virReportOOMError(); + bhyveDriverUnlock(privconn); + + return xml; +} + +static virDomainObjPtr +bhyveDomObjFromDomain(virDomainPtr domain) +{ + virDomainObjPtr vm; + bhyveConnPtr privconn = domain->conn->privateData; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid); + if (!vm) { + virUUIDFormat(domain->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s' (%s)"), + uuidstr, domain->name); + return NULL; + } + + return vm; +} + +static virDrvOpenStatus +bhyveConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + if (conn->uri == NULL) { + if (bhyve_driver == NULL) + return VIR_DRV_OPEN_DECLINED; + + if (!(conn->uri = virURIParse("bhyve:///system"))) + return VIR_DRV_OPEN_ERROR; + } else { + if (!conn->uri->scheme || STRNEQ(conn->uri->scheme, "bhyve")) + return VIR_DRV_OPEN_DECLINED; + + if (conn->uri->server) + return VIR_DRV_OPEN_DECLINED; + + if (!STREQ_NULLABLE(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected bhyve URI path '%s', try bhyve:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + + if (bhyve_driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("bhyve state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + conn->privateData = bhyve_driver; + + return VIR_DRV_OPEN_SUCCESS; +} + +static int +bhyveConnectClose(virConnectPtr conn) +{ + conn->privateData = NULL; + + return 0; +} + +static char * +bhyveConnectGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + if (virConnectGetHostnameEnsureACL(conn) < 0) + return NULL; + + return virGetHostname(); +} + +static int +bhyveConnectGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *version) +{ + struct utsname ver; + + if (virConnectGetVersionEnsureACL(conn) < 0) + return -1; + + uname(&ver); + + if (virParseVersionString(ver.release, version, true) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown release: %s"), ver.release); + return -1; + } + + return 0; +} + +static int +bhyveDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) +{ + virDomainObjPtr vm; + int ret = -1; + + if (!(vm = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainGetInfoEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + info->state = virDomainObjGetState(vm, NULL); + info->maxMem = vm->def->mem.max_balloon; + info->nrVirtCpu = vm->def->vcpus; + ret = 0; + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +bhyveDomainGetState(virDomainPtr domain, + int *state, + int *reason, + unsigned int flags) +{ + virDomainObjPtr vm; + int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainGetStateEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + *state = virDomainObjGetState(vm, reason); + ret = 0; + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +bhyveDomainIsActive(virDomainPtr domain) +{ + virDomainObjPtr obj; + int ret = -1; + + if (!(obj = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainIsActiveEnsureACL(domain->conn, obj->def) < 0) + goto cleanup; + + ret = virDomainObjIsActive(obj); + +cleanup: + if (obj) + virObjectUnlock(obj); + return ret; +} + +static int +bhyveDomainIsPersistent(virDomainPtr domain) +{ + virDomainObjPtr obj; + int ret = -1; + + if (!(obj = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainIsPersistentEnsureACL(domain->conn, obj->def) < 0) + goto cleanup; + + ret = obj->persistent; + +cleanup: + if (obj) + virObjectUnlock(obj); + return ret; +} + +static char * +bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) +{ + virDomainObjPtr vm; + char *ret = NULL; + + if (!(vm = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainGetXMLDescEnsureACL(domain->conn, vm->def, flags) < 0) + goto cleanup; + + ret = virDomainDefFormat(vm->def, flags); + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static virDomainPtr +bhyveDomainDefineXML(virConnectPtr conn, const char *xml) +{ + bhyveConnPtr privconn = conn->privateData; + virDomainPtr dom = NULL; + virDomainDefPtr def = NULL; + virDomainDefPtr oldDef = NULL; + virDomainObjPtr vm = NULL; + + if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, + 1 << VIR_DOMAIN_VIRT_BHYVE, + VIR_DOMAIN_XML_INACTIVE)) == NULL) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Can't parse XML desc")); + goto cleanup; + } + + if (virDomainDefineXMLEnsureACL(conn, def) < 0) + goto cleanup; + + if (!(vm = virDomainObjListAdd(privconn->domains, def, + privconn->xmlopt, + 0, &oldDef))) + goto cleanup; + def = NULL; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (dom) + dom->id = vm->def->id; + + if (virDomainSaveConfig(BHYVE_CONFIG_DIR, vm->def) < 0) + goto cleanup; + +cleanup: + virDomainDefFree(def); + if (vm) + virObjectUnlock(vm); + + return dom; +} + +static int +bhyveDomainUndefine(virDomainPtr domain) +{ + bhyveConnPtr privconn = domain->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + if (!(vm = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainUndefineEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (!vm->persistent) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Cannot undefine transient domain")); + goto cleanup; + } + + if (virDomainDeleteConfig(BHYVE_CONFIG_DIR, + BHYVE_AUTOSTART_DIR, + vm) < 0) + goto cleanup; + + if (virDomainObjIsActive(vm)) { + vm->persistent = 0; + } else { + virDomainObjListRemove(privconn->domains, vm); + vm = NULL; + } + + ret = 0; + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +bhyveConnectListDomains(virConnectPtr conn, int *ids, int maxids) +{ + bhyveConnPtr privconn = conn->privateData; + int n; + + if (virConnectListDomainsEnsureACL(conn) < 0) + return -1; + + n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids, + virConnectListDomainsCheckACL, conn); + + return n; +} + +static int +bhyveConnectNumOfDomains(virConnectPtr conn) +{ + bhyveConnPtr privconn = conn->privateData; + int count; + + if (virConnectNumOfDomainsEnsureACL(conn) < 0) + return -1; + + count = virDomainObjListNumOfDomains(privconn->domains, true, + virConnectNumOfDomainsCheckACL, conn); + + return count; +} + +static int +bhyveConnectListDefinedDomains(virConnectPtr conn, char **const names, + int maxnames) +{ + bhyveConnPtr privconn = conn->privateData; + int n; + + if (virConnectListDefinedDomainsEnsureACL(conn) < 0) + return -1; + + memset(names, 0, sizeof(*names) * maxnames); + n = virDomainObjListGetInactiveNames(privconn->domains, names, + maxnames, virConnectListDefinedDomainsCheckACL, conn); + + return n; +} + +static int +bhyveConnectNumOfDefinedDomains(virConnectPtr conn) +{ + bhyveConnPtr privconn = conn->privateData; + int count; + + if (virConnectNumOfDefinedDomainsEnsureACL(conn) < 0) + return -1; + + count = virDomainObjListNumOfDomains(privconn->domains, false, + virConnectNumOfDefinedDomainsCheckACL, conn); + + return count; +} + +static int +bhyveConnectListAllDomains(virConnectPtr conn, + virDomainPtr **domains, + unsigned int flags) +{ + bhyveConnPtr privconn = conn->privateData; + int ret = -1; + + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); + + if (virConnectListAllDomainsEnsureACL(conn) < 0) + return -1; + + ret = virDomainObjListExport(privconn->domains, conn, domains, + virConnectListAllDomainsCheckACL, flags); + + return ret; +} + +static virDomainPtr +bhyveDomainLookupByUUID(virConnectPtr conn, + const unsigned char *uuid) +{ + bhyveConnPtr privconn = conn->privateData; + virDomainObjPtr vm; + virDomainPtr dom = NULL; + + vm = virDomainObjListFindByUUID(privconn->domains, uuid); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("No domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0) + goto cleanup; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (dom) + dom->id = vm->def->id; + +cleanup: + if (vm) + virObjectUnlock(vm); + return dom; +} + +static virDomainPtr bhyveDomainLookupByName(virConnectPtr conn, + const char *name) +{ + bhyveConnPtr privconn = conn->privateData; + virDomainObjPtr vm; + virDomainPtr dom = NULL; + + vm = virDomainObjListFindByName(privconn->domains, name); + + if (!vm) { + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching name '%s'"), name); + goto cleanup; + } + + if (virDomainLookupByNameEnsureACL(conn, vm->def) < 0) + goto cleanup; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (dom) + dom->id = vm->def->id; + +cleanup: + if (vm) + virObjectUnlock(vm); + return dom; +} + +static int +bhyveDomainCreate(virDomainPtr dom) +{ + bhyveConnPtr privconn = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + if (!(vm = bhyveDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainCreateEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is already running")); + goto cleanup; + } + + ret = virBhyveProcessStart(dom->conn, privconn, vm, + VIR_DOMAIN_RUNNING_BOOTED); + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +bhyveDomainDestroy(virDomainPtr dom) +{ + bhyveConnPtr privconn = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + if (!(vm = bhyveDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainDestroyEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +bhyveStateCleanup(void) +{ + VIR_INFO("bhyve state cleanup"); + + if (bhyve_driver == NULL) + return -1; + + virObjectUnref(bhyve_driver->domains); + virObjectUnref(bhyve_driver->caps); + virObjectUnref(bhyve_driver->xmlopt); + + virMutexDestroy(&bhyve_driver->lock); + VIR_FREE(bhyve_driver); + + return 0; +} + +static int +bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (!priveleged) { + VIR_INFO("Not running priveleged, disabling driver"); + return 0; + } + + if (VIR_ALLOC(bhyve_driver) < 0) { + return -1; + } + + if (virMutexInit(&bhyve_driver->lock) < 0) { + VIR_FREE(bhyve_driver); + return -1; + } + + if (!(bhyve_driver->caps = bhyveBuildCapabilities())) + goto cleanup; + + if (!(bhyve_driver->xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL))) + goto cleanup; + + if (!(bhyve_driver->domains = virDomainObjListNew())) + goto cleanup; + + if (virFileMakePath(BHYVE_LOG_DIR) < 0) { + virReportSystemError(errno, + _("Failed to mkdir %s"), + BHYVE_LOG_DIR); + goto cleanup; + } + + if (virFileMakePath(BHYVE_STATE_DIR) < 0) { + virReportSystemError(errno, + _("Failed to mkdir %s"), + BHYVE_LOG_DIR); + goto cleanup; + } + + if (virDomainObjListLoadAllConfigs(bhyve_driver->domains, + BHYVE_CONFIG_DIR, + NULL, 0, + bhyve_driver->caps, + bhyve_driver->xmlopt, + 1 << VIR_DOMAIN_VIRT_BHYVE, + NULL, NULL) < 0) + goto cleanup; + + return 0; + +cleanup: + bhyveStateCleanup(); + return -1; +} + + +static virDriver bhyveDriver = { + .no = VIR_DRV_BHYVE, + .name = "bhyve", + .connectOpen = bhyveConnectOpen, /* 0.1.0 */ + .connectClose = bhyveConnectClose, /* 0.1.0 */ + .connectGetVersion = bhyveConnectGetVersion, /* 0.1.0 */ + .connectGetHostname = bhyveConnectGetHostname, /* 0.1.0 */ + .domainGetInfo = bhyveDomainGetInfo, /* 0.1.0 */ + .domainGetState = bhyveDomainGetState, /* 0.1.0 */ + .connectGetCapabilities = bhyveConnectGetCapabilities, /* 0.1.0 */ + .connectListDomains = bhyveConnectListDomains, /* 0.1.0 */ + .connectNumOfDomains = bhyveConnectNumOfDomains, /* 0.1.0 */ + .connectListAllDomains = bhyveConnectListAllDomains, /* 0.1.0 */ + .connectListDefinedDomains = bhyveConnectListDefinedDomains, /* 0.1.0 */ + .connectNumOfDefinedDomains = bhyveConnectNumOfDefinedDomains, /* 0.1.0 */ + .domainCreate = bhyveDomainCreate, /* 0.1.0 */ + .domainDestroy = bhyveDomainDestroy, /* 0.1.0 */ + .domainLookupByUUID = bhyveDomainLookupByUUID, /* 0.1.0 */ + .domainLookupByName = bhyveDomainLookupByName, /* 0.1.0 */ + .domainDefineXML = bhyveDomainDefineXML, /* 0.1.0 */ + .domainUndefine = bhyveDomainUndefine, /* 0.1.0 */ + .domainGetXMLDesc = bhyveDomainGetXMLDesc, /* 0.1.0 */ + .domainIsActive = bhyveDomainIsActive, /* 0.1.0 */ + .domainIsPersistent = bhyveDomainIsPersistent, /* 0.1.0 */ +}; + + +static virStateDriver bhyveStateDriver = { + .name = "bhyve", + .stateInitialize = bhyveStateInitialize, + .stateCleanup = bhyveStateCleanup, +}; + +int +bhyveRegister(void) +{ + virRegisterDriver(&bhyveDriver); + virRegisterStateDriver(&bhyveStateDriver); + return 0; +} diff --git a/src/bhyve/bhyve_driver.h b/src/bhyve/bhyve_driver.h new file mode 100644 index 0000000..ffe91e5 --- /dev/null +++ b/src/bhyve/bhyve_driver.h @@ -0,0 +1,28 @@ +/* + * bhyve_driver.h: core driver methods for managing bhyve guests + * + * Copyright (C) 2013 Roman Bogorodskiy + * + * 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: Roman Bogorodskiy <bogorodskiy@gmail.com> + */ + +#ifndef __BHYVE_DRIVER_H__ +# define __BHYVE_DRIVER_H__ + +int bhyveRegister(void); + +#endif /* __BHYVE_DRIVER_H__ */ diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c new file mode 100644 index 0000000..80bd919 --- /dev/null +++ b/src/bhyve/bhyve_process.c @@ -0,0 +1,227 @@ +/* + * bhyve_process.c: bhyve process management + * + * Copyright (C) 2013 Roman Bogorodskiy + * + * 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 <fcntl.h> +#include <sys/types.h> +#include <sys/ioctl.h> +#include <net/if.h> +#include <net/if_tap.h> + +#include "bhyve_process.h" +#include "bhyve_command.h" +#include "datatypes.h" +#include "virerror.h" +#include "virlog.h" +#include "virfile.h" +#include "viralloc.h" +#include "vircommand.h" +#include "virstring.h" +#include "virpidfile.h" +#include "virprocess.h" +#include "virnetdev.h" +#include "virnetdevbridge.h" +#include "virnetdevtap.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +int +virBhyveProcessStart(virConnectPtr conn, + bhyveConnPtr driver, + virDomainObjPtr vm, + virDomainRunningReason reason) +{ + char *logfile = NULL; + int logfd = -1; + off_t pos = -1; + char ebuf[1024]; + virCommandPtr cmd = NULL; + virCommandPtr load_cmd = NULL; + bhyveConnPtr privconn = conn->privateData; + int ret = -1, status; + + if (virAsprintf(&logfile, "%s/%s.log", + BHYVE_LOG_DIR, vm->def->name) < 0) + return -1; + + + if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT, + S_IRUSR|S_IWUSR)) < 0) { + virReportSystemError(errno, + _("Failed to open '%s'"), + logfile); + goto cleanup; + } + + VIR_FREE(privconn->pidfile); + if (!(privconn->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR, + vm->def->name))) { + virReportSystemError(errno, + "%s", _("Failed to build pidfile path.")); + goto cleanup; + } + + if (unlink(privconn->pidfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Cannot remove state PID file %s"), + privconn->pidfile); + goto cleanup; + } + + /* Call bhyve to start the VM */ + if (!(cmd = virBhyveProcessBuildBhyveCmd(driver, + vm))) + goto cleanup; + + virCommandSetOutputFD(cmd, &logfd); + virCommandSetErrorFD(cmd, &logfd); + virCommandWriteArgLog(cmd, logfd); + virCommandSetPidFile(cmd, privconn->pidfile); + virCommandDaemonize(cmd); + + /* Now bhyve command is constructed, meaning the + * domain is ready to be started, so we can build + * and execute bhyveload command */ + if (!(load_cmd = virBhyveProcessBuildLoadCmd(driver, + vm))) + goto cleanup; + virCommandSetOutputFD(load_cmd, &logfd); + virCommandSetErrorFD(load_cmd, &logfd); + + /* Log generated command line */ + virCommandWriteArgLog(load_cmd, logfd); + if ((pos = lseek(logfd, 0, SEEK_END)) < 0) + VIR_WARN("Unable to seek to end of logfile: %s", + virStrerror(errno, ebuf, sizeof(ebuf))); + + VIR_INFO("Loading domain '%s'", vm->def->name); + if (virCommandRun(load_cmd, &status) < 0) + goto cleanup; + + if (status != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Guest failed to load: %d"), status); + goto cleanup; + } + + /* Now we can start the domain */ + VIR_INFO("Starting domain '%s'", vm->def->name); + ret = virCommandRun(cmd, NULL); + + if (ret == 0) { + if (virPidFileReadPath(privconn->pidfile, &vm->pid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Domain %s didn't show up"), vm->def->name); + goto cleanup; + } + + vm->def->id = vm->pid; + virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); + } else { + goto cleanup; + } + +cleanup: + if (ret < 0) { + virCommandPtr destroy_cmd; + if ((destroy_cmd = virBhyveProcessBuildDestroyCmd(driver, vm)) != NULL) { + virCommandSetOutputFD(load_cmd, &logfd); + virCommandSetErrorFD(load_cmd, &logfd); + ignore_value(virCommandRun(destroy_cmd, NULL)); + virCommandFree(destroy_cmd); + } + } + + virCommandFree(load_cmd); + virCommandFree(cmd); + VIR_FREE(logfile); + if (VIR_CLOSE(logfd) < 0) { + virReportSystemError(errno, "%s", _("could not close logfile")); + } + return ret; +} + +int +virBhyveProcessStop(bhyveConnPtr driver, + virDomainObjPtr vm, + virDomainShutoffReason reason ATTRIBUTE_UNUSED) +{ + size_t i; + int ret = -1; + int status; + virCommandPtr cmd = NULL; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("VM '%s' not active", vm->def->name); + return 0; + } + + if (vm->pid <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid PID %d for VM"), + (int)vm->pid); + return -1; + } + + /* First, try to kill 'bhyve' process */ + if (virProcessKillPainfully(vm->pid, true) != 0) + VIR_WARN("Failed to gracefully stop bhyve VM '%s' (pid: %d)", + vm->def->name, + (int)vm->pid); + + for (i = 0; i < vm->def->nnets; i++) { + virDomainNetDefPtr net = vm->def->nets[i]; + int actualType = virDomainNetGetActualType(net); + + if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + ignore_value(virNetDevBridgeRemovePort( + virDomainNetGetActualBridgeName(net), + net->ifname)); + ignore_value(virNetDevTapDelete(net->ifname)); + } + } + + /* No matter if shutdown was successful or not, we + * need to unload the VM */ + if (!(cmd = virBhyveProcessBuildDestroyCmd(driver, vm))) + goto cleanup; + + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + if (status != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Guest failed to stop: %d"), status); + goto cleanup; + } + + ret = 0; + + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); + vm->pid = -1; + vm->def->id = -1; + +cleanup: + virCommandFree(cmd); + return ret; +} diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h new file mode 100644 index 0000000..70afe0e --- /dev/null +++ b/src/bhyve/bhyve_process.h @@ -0,0 +1,36 @@ +/* + * bhyve_process.h: bhyve process management + * + * Copyright (C) 2013 Roman Bogorodskiy + * + * 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/>. + * + */ + +#ifndef __BHYVE_PROCESS_H__ +# define __BHYVE_PROCESS_H__ + +# include "bhyve_utils.h" + +int virBhyveProcessStart(virConnectPtr conn, + bhyveConnPtr driver, + virDomainObjPtr vm, + virDomainRunningReason reason); + +int virBhyveProcessStop(bhyveConnPtr driver, + virDomainObjPtr vm, + virDomainShutoffReason reason); + +#endif /* __BHYVE_PROCESS_H__ */ diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h new file mode 100644 index 0000000..0a38c93 --- /dev/null +++ b/src/bhyve/bhyve_utils.h @@ -0,0 +1,49 @@ +/* + * bhyve_utils.h: bhyve utils + * + * Copyright (C) 2013 Roman Bogorodskiy + * + * 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/>. + * + */ + +#ifndef __BHYVE_UTILS_H__ +# define __BHYVE_UTILS_H__ + +# include "driver.h" +# include "domain_conf.h" +# include "configmake.h" +# include "virthread.h" + +# define BHYVE_AUTOSTART_DIR (SYSCONFDIR "/libvirt/bhyve/autostart") +# define BHYVE_CONFIG_DIR (SYSCONFDIR "/libvirt/bhyve") +# define BHYVE_STATE_DIR (LOCALSTATEDIR "/run/libvirt/bhyve") +# define BHYVE_LOG_DIR (LOCALSTATEDIR "/log/libvirt/bhyve") + +struct _bhyveConn { + virMutex lock; + virDomainObjListPtr domains; + virCapsPtr caps; + virDomainXMLOptionPtr xmlopt; + char *pidfile; +}; + +typedef struct _bhyveConn bhyveConn; +typedef struct _bhyveConn *bhyveConnPtr; + +void bhyveDriverLock(bhyveConnPtr driver); +void bhyveDriverUnlock(bhyveConnPtr driver); + +#endif /* __BHYVE_UTILS_H__ */ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f6065ed..b7f8b65 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -122,7 +122,8 @@ VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST, "hyperv", "vbox", "phyp", - "parallels") + "parallels", + "bhyve") VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST, "fd", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4895e81..cb3a315 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -196,6 +196,7 @@ enum virDomainVirtType { VIR_DOMAIN_VIRT_VBOX, VIR_DOMAIN_VIRT_PHYP, VIR_DOMAIN_VIRT_PARALLELS, + VIR_DOMAIN_VIRT_BHYVE, VIR_DOMAIN_VIRT_LAST }; diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..fbfaac4 100644 --- a/src/driver.h +++ b/src/driver.h @@ -47,6 +47,7 @@ typedef enum { VIR_DRV_LIBXL = 14, VIR_DRV_HYPERV = 15, VIR_DRV_PARALLELS = 16, + VIR_DRV_BHYVE = 17, } virDrvNo; diff --git a/src/libvirt.c b/src/libvirt.c index 9cc5b1c..54ecdc8 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -96,6 +96,9 @@ #ifdef WITH_PARALLELS # include "parallels/parallels_driver.h" #endif +#ifdef WITH_BHYVE +# include "bhyve/bhyve_driver.h" +#endif #define VIR_FROM_THIS VIR_FROM_NONE diff --git a/src/util/virerror.c b/src/util/virerror.c index f0c159f..74c6807 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -124,6 +124,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Access Manager", /* 55 */ "Systemd", + "Bhyve", ) -- 1.8.4.3

On Thu, Feb 13, 2014 at 02:14:32PM +0400, Roman Bogorodskiy wrote:
+static int +bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) +{ + virDomainDiskDefPtr disk; + + if (def->ndisks != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("domain should have one and only one disk defined")); + return -1; + } + + disk = def->disks[0]; + + if (disk->bus != VIR_DOMAIN_DISK_BUS_SATA) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported disk bus type")); + return -1; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported disk type")); + return -1; + }
We still need to validate disk->type before accessing disk->src otherwise you'll crash if someone uses type=network
+ + virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "2:0,ahci-hd,%s", disk->src);
What is the '2:0' bit here ? Is that disk controller/unit number ?
+virCommandPtr +virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm) +{ + virCommandPtr cmd; + virDomainDiskDefPtr disk; + + if (vm->def->ndisks != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("domain should have one and only one disk defined")); + return NULL; + } + + disk = vm->def->disks[0]; + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported disk type")); + return NULL; + }
And validate disk->type again here
+ + cmd = virCommandNew(BHYVELOAD); + + /* Memory */ + virCommandAddArg(cmd, "-m"); + virCommandAddArgFormat(cmd, "%llu", + VIR_DIV_UP(vm->def->mem.max_balloon, 1024)); + + /* Image path */ + virCommandAddArg(cmd, "-d"); + virCommandAddArgFormat(cmd, disk->src); + + /* VM name */ + virCommandAddArg(cmd, vm->def->name); + + return cmd; +}
+static char * +bhyveConnectGetCapabilities(virConnectPtr conn) +{ + bhyveConnPtr privconn = conn->privateData; + char *xml; + + if (virConnectGetCapabilitiesEnsureACL(conn) < 0) + return NULL; + + bhyveDriverLock(privconn); + if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL) + virReportOOMError(); + bhyveDriverUnlock(privconn); + + return xml; +}
Technically the lock/unlock isn't needed since you never change privconn->caps once you've created it.
+static virDrvOpenStatus +bhyveConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + + if (conn->uri == NULL) { + if (bhyve_driver == NULL) + return VIR_DRV_OPEN_DECLINED; + + if (!(conn->uri = virURIParse("bhyve:///system"))) + return VIR_DRV_OPEN_ERROR;
Nitpick indentation is too great in the two 'return' lines
+ } else { + if (!conn->uri->scheme || STRNEQ(conn->uri->scheme, "bhyve")) + return VIR_DRV_OPEN_DECLINED; + + if (conn->uri->server) + return VIR_DRV_OPEN_DECLINED; + + if (!STREQ_NULLABLE(conn->uri->path, "/system")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected bhyve URI path '%s', try bhyve:///system"), + conn->uri->path); + return VIR_DRV_OPEN_ERROR; + } + + if (bhyve_driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("bhyve state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + } + + if (virConnectOpenEnsureACL(conn) < 0) + return VIR_DRV_OPEN_ERROR; + + conn->privateData = bhyve_driver; + + return VIR_DRV_OPEN_SUCCESS; +}
+static virDomainPtr +bhyveDomainDefineXML(virConnectPtr conn, const char *xml) +{ + bhyveConnPtr privconn = conn->privateData; + virDomainPtr dom = NULL; + virDomainDefPtr def = NULL; + virDomainDefPtr oldDef = NULL; + virDomainObjPtr vm = NULL; + + if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, + 1 << VIR_DOMAIN_VIRT_BHYVE, + VIR_DOMAIN_XML_INACTIVE)) == NULL) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Can't parse XML desc"));
Remove the 'virReportError' call, since that's already done for you by the parsing code.
+ goto cleanup; + } + + if (virDomainDefineXMLEnsureACL(conn, def) < 0) + goto cleanup; + + if (!(vm = virDomainObjListAdd(privconn->domains, def, + privconn->xmlopt, + 0, &oldDef))) + goto cleanup; + def = NULL; + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (dom) + dom->id = vm->def->id; + + if (virDomainSaveConfig(BHYVE_CONFIG_DIR, vm->def) < 0) + goto cleanup; + +cleanup: + virDomainDefFree(def); + if (vm) + virObjectUnlock(vm); + + return dom; +} +
+ +int +virBhyveProcessStart(virConnectPtr conn, + bhyveConnPtr driver, + virDomainObjPtr vm, + virDomainRunningReason reason) +{ + char *logfile = NULL; + int logfd = -1; + off_t pos = -1; + char ebuf[1024]; + virCommandPtr cmd = NULL; + virCommandPtr load_cmd = NULL; + bhyveConnPtr privconn = conn->privateData; + int ret = -1, status; + + if (virAsprintf(&logfile, "%s/%s.log", + BHYVE_LOG_DIR, vm->def->name) < 0) + return -1; + + + if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT, + S_IRUSR|S_IWUSR)) < 0) { + virReportSystemError(errno, + _("Failed to open '%s'"), + logfile); + goto cleanup; + } + + VIR_FREE(privconn->pidfile); + if (!(privconn->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR, + vm->def->name))) { + virReportSystemError(errno, + "%s", _("Failed to build pidfile path.")); + goto cleanup; + } + + if (unlink(privconn->pidfile) < 0 && + errno != ENOENT) { + virReportSystemError(errno, + _("Cannot remove state PID file %s"), + privconn->pidfile); + goto cleanup; + } + + /* Call bhyve to start the VM */ + if (!(cmd = virBhyveProcessBuildBhyveCmd(driver, + vm))) + goto cleanup; + + virCommandSetOutputFD(cmd, &logfd); + virCommandSetErrorFD(cmd, &logfd); + virCommandWriteArgLog(cmd, logfd); + virCommandSetPidFile(cmd, privconn->pidfile); + virCommandDaemonize(cmd); + + /* Now bhyve command is constructed, meaning the + * domain is ready to be started, so we can build + * and execute bhyveload command */ + if (!(load_cmd = virBhyveProcessBuildLoadCmd(driver, + vm))) + goto cleanup; + virCommandSetOutputFD(load_cmd, &logfd); + virCommandSetErrorFD(load_cmd, &logfd); + + /* Log generated command line */ + virCommandWriteArgLog(load_cmd, logfd); + if ((pos = lseek(logfd, 0, SEEK_END)) < 0) + VIR_WARN("Unable to seek to end of logfile: %s", + virStrerror(errno, ebuf, sizeof(ebuf))); + + VIR_INFO("Loading domain '%s'", vm->def->name); + if (virCommandRun(load_cmd, &status) < 0) + goto cleanup; + + if (status != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Guest failed to load: %d"), status); + goto cleanup; + } + + /* Now we can start the domain */ + VIR_INFO("Starting domain '%s'", vm->def->name);
I'd suggest s/INFO/DEBUG/ here and earlier. If you want user visible log messages about key operations, then we should talk about what is needed to make viraudit.{c,h} work on FreeBSD, and use domain_audit.c for this.
+ ret = virCommandRun(cmd, NULL); + + if (ret == 0) { + if (virPidFileReadPath(privconn->pidfile, &vm->pid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Domain %s didn't show up"), vm->def->name); + goto cleanup; + } + + vm->def->id = vm->pid; + virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); + } else { + goto cleanup; + } + +cleanup: + if (ret < 0) { + virCommandPtr destroy_cmd; + if ((destroy_cmd = virBhyveProcessBuildDestroyCmd(driver, vm)) != NULL) { + virCommandSetOutputFD(load_cmd, &logfd); + virCommandSetErrorFD(load_cmd, &logfd); + ignore_value(virCommandRun(destroy_cmd, NULL)); + virCommandFree(destroy_cmd); + } + } + + virCommandFree(load_cmd); + virCommandFree(cmd); + VIR_FREE(logfile); + if (VIR_CLOSE(logfd) < 0) { + virReportSystemError(errno, "%s", _("could not close logfile")); + }
You can use VIR_FORCE_CLOSE and ignore the error message here.
+ return ret; +}
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 :|

Daniel P. Berrange wrote:
+ virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "2:0,ahci-hd,%s", disk->src);
What is the '2:0' bit here ? Is that disk controller/unit number ?
Quoting the manpage [1]: pcislot[:function] The pcislot value is 0 to 31 and the optional function value is 0 to 7. If not specified, the function value defaults to 0.
I'd suggest s/INFO/DEBUG/ here and earlier. If you want user visible log messages about key operations, then we should talk about what is needed to make viraudit.{c,h} work on FreeBSD, and use domain_audit.c for this.
Will do s/INFO/DEBUG/. I'll take a look at viraudit later too. 1: http://www.freebsd.org/cgi/man.cgi?query=bhyve&apropos=0&sektion=0&manpath=FreeBSD+10.0-RELEASE&arch=default&format=html Thanks for the review! Roman Bogorodskiy

On 02/13/2014 03:14 AM, Roman Bogorodskiy wrote:
At this point it has a limited functionality and is highly experimental. Supported domain operations are:
* define * start * destroy * dumpxml * dominfo * undefine
It's only possible to have only one disk device and only one network, which should be of type bridge. ---
I didn't see any edits to cfg.mk; not sure if you were trying 'make syntax-check' or if we may have some tweaks to clean up. Oh, and I guess you're still waiting on me to find time to tweak upstream gnulib to use $(SED) during syntax-check.
+++ b/include/libvirt/virterror.h @@ -121,6 +121,7 @@ typedef enum { VIR_FROM_ACCESS = 55, /* Error from access control manager */ VIR_FROM_SYSTEMD = 56, /* Error from systemd code */
+ VIR_FROM_BHYVE = 57, /* Error from bhyve driver */ # ifdef VIR_ENUM_SENTINELS
Blank line is in wrong place; the rest of the enum uses groups of 5.
+ if test "$with_bhyve" != "no"; then + AC_PATH_PROG([BHYVE], [bhyve], [], [$PATH:/usr/sbin]) + AC_PATH_PROG([BHYVECTL], [bhyvectl], [$PATH:/usr/sbin]) + AC_PATH_PROG([BHYVELOAD], [bhyveload], [$PATH:/usr/sbin/])
The first call is okay, but the 2nd and 3rd call are missing a third argument (the use of PATH should be the 4th argument).
+++ b/src/Makefile.am
+BHYVE_DRIVER_SOURCES = \ + bhyve/bhyve_command.c \ + bhyve/bhyve_command.h \ + bhyve/bhyve_driver.h \ + bhyve/bhyve_driver.c \ + bhyve/bhyve_process.c \ + bhyve/bhyve_process.h \ + bhyve/bhyve_utils.h
Might as well use $(NULL) at the end of your list; it makes adding new files to the list easier since they can be straight additions (we haven't always used it in the past, but new code has been gradually switching more of Makefile over to that style).
+++ b/src/bhyve/bhyve_command.c
+static char* +virBhyveTapGetRealDeviceName(char *name) +{ + /* This is an ugly hack, because if we rename + * tap device to vnet%d, its device name will be + * still /dev/tap%d, and bhyve tries too open /dev/tap%d,
s/too/to/
+ while ((dp = readdir(dirp)) != NULL) {
+ VIR_FREE(devpath); + VIR_FORCE_CLOSE(fd); + } + }
Is it worth setting errno=0 before each readdir, and checking for failure to iterate? You raise a virError on all other failure paths, but readdir failure is currently silent.
+static int +bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd) +{
+ + if (net != NULL) {
We aren't always consistent, but you can use 'if (net) {' instead of longhand.
+ int actualType = virDomainNetGetActualType(net); + + if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0) + return -1L;
Why return a long when the function type is int? '-1' is sufficient.
+ + virCommandAddArg(cmd, "-s"); + virCommandAddArg(cmd, "0:0,hostbridge"); + virCommandAddArg(cmd, "-s");
These could be joined with virCommandAddArgList, if desired.
+ /* Memory */ + virCommandAddArg(cmd, "-m"); + virCommandAddArgFormat(cmd, "%llu", + VIR_DIV_UP(vm->def->mem.max_balloon, 1024)); + + /* Image path */ + virCommandAddArg(cmd, "-d"); + virCommandAddArgFormat(cmd, disk->src);
Security hole if disk->src contains % (particular %n). Either use virCommandAddArg(cmd, disk->src) or virCommandAddArgFormat(cmd, "%s", disk->src).
+++ b/src/bhyve/bhyve_driver.c
+static virDrvOpenStatus +bhyveConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags)
Indentation is off.
+static int +bhyveDomainGetState(virDomainPtr domain, + int *state, + int *reason, + unsigned int flags)
+ +cleanup: + if (vm) + virObjectUnlock(vm);
virObjectUnlock() is safe on NULL; you could drop the 'if' (here and in several other cleanups).
+static virDomainPtr +bhyveDomainDefineXML(virConnectPtr conn, const char *xml) +{
+ + if (!(vm = virDomainObjListAdd(privconn->domains, def, + privconn->xmlopt, + 0, &oldDef))) + goto cleanup;
Indentation is off.
+static int +bhyveConnectListDefinedDomains(virConnectPtr conn, char **const names, + int maxnames) +{ + bhyveConnPtr privconn = conn->privateData; + int n; + + if (virConnectListDefinedDomainsEnsureACL(conn) < 0) + return -1; + + memset(names, 0, sizeof(*names) * maxnames);
Not a problem with your patch, but maybe something worth cleaning up in ALL drivers in a later patch - we should probably move this memset() into libvirt.c to occur even when returning an error. As-is, an ACL failure leaves the data in an indeterminate state, which may cause a client to misbehave if they aren't paying attention to errors.
+static virDriver bhyveDriver = { + .no = VIR_DRV_BHYVE, + .name = "bhyve", + .connectOpen = bhyveConnectOpen, /* 0.1.0 */ + .connectClose = bhyveConnectClose, /* 0.1.0 */
Everything in this struct should use 1.2.2 (or even 1.3.0), not 0.1.0 - it is the version number that will first contain bhyve.
+++ b/src/bhyve/bhyve_driver.h
+ * Copyright (C) 2013 Roman Bogorodskiy
It's 2014.
+++ b/src/bhyve/bhyve_process.c @@ -0,0 +1,227 @@ +/* + * bhyve_process.c: bhyve process management + * + * Copyright (C) 2013 Roman Bogorodskiy
and again
+int +virBhyveProcessStart(virConnectPtr conn, + bhyveConnPtr driver, + virDomainObjPtr vm, + virDomainRunningReason reason) +{ + char *logfile = NULL; + int logfd = -1; + off_t pos = -1; + char ebuf[1024]; + virCommandPtr cmd = NULL; + virCommandPtr load_cmd = NULL; + bhyveConnPtr privconn = conn->privateData; + int ret = -1, status; + + if (virAsprintf(&logfile, "%s/%s.log", + BHYVE_LOG_DIR, vm->def->name) < 0) + return -1; + + + if ((logfd = open(logfile, O_WRONLY | O_APPEND | O_CREAT, + S_IRUSR|S_IWUSR)) < 0) {
Spaces around |
+ virReportSystemError(errno, + _("Failed to open '%s'"), + logfile); + goto cleanup; + } + + VIR_FREE(privconn->pidfile); + if (!(privconn->pidfile = virPidFileBuildPath(BHYVE_STATE_DIR, + vm->def->name))) { + virReportSystemError(errno, + "%s", _("Failed to build pidfile path."));
No trailing '.' on error messages
+ /* Now bhyve command is constructed, meaning the + * domain is ready to be started, so we can build + * and execute bhyveload command */ + if (!(load_cmd = virBhyveProcessBuildLoadCmd(driver, + vm)))
Indentation is off; would this fit on one line?
+ goto cleanup; + virCommandSetOutputFD(load_cmd, &logfd); + virCommandSetErrorFD(load_cmd, &logfd); + + /* Log generated command line */ + virCommandWriteArgLog(load_cmd, logfd); + if ((pos = lseek(logfd, 0, SEEK_END)) < 0) + VIR_WARN("Unable to seek to end of logfile: %s", + virStrerror(errno, ebuf, sizeof(ebuf)));
Indentation is off.
+cleanup: + if (ret < 0) { + virCommandPtr destroy_cmd; + if ((destroy_cmd = virBhyveProcessBuildDestroyCmd(driver, vm)) != NULL) { + virCommandSetOutputFD(load_cmd, &logfd); + virCommandSetErrorFD(load_cmd, &logfd); + ignore_value(virCommandRun(destroy_cmd, NULL)); + virCommandFree(destroy_cmd); + } + } + + virCommandFree(load_cmd); + virCommandFree(cmd); + VIR_FREE(logfile); + if (VIR_CLOSE(logfd) < 0) { + virReportSystemError(errno, "%s", _("could not close logfile")); + }
I'd use VIR_FORCE_CLOSE here - reporting a system error does no good if you return success, and wipes out a better error message if you are already returning an error.
+++ b/src/bhyve/bhyve_process.h @@ -0,0 +1,36 @@ +/* + * bhyve_process.h: bhyve process management + * + * Copyright (C) 2013 Roman Bogorodskiy
2014
+++ b/src/bhyve/bhyve_utils.h @@ -0,0 +1,49 @@ +/* + * bhyve_utils.h: bhyve utils + * + * Copyright (C) 2013 Roman Bogorodskiy
again
+ +# define BHYVE_AUTOSTART_DIR (SYSCONFDIR "/libvirt/bhyve/autostart") +# define BHYVE_CONFIG_DIR (SYSCONFDIR "/libvirt/bhyve") +# define BHYVE_STATE_DIR (LOCALSTATEDIR "/run/libvirt/bhyve") +# define BHYVE_LOG_DIR (LOCALSTATEDIR "/log/libvirt/bhyve")
The () aren't necessary here. In fact, they get in the way of someone trying to do BHYVE_AUTOSTART_DIR "/file" because of the way string concatenation works. Overall, looks pretty good; thanks to others for doing reviews on the earlier versions, since it took me until v9 to take a peek :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
I didn't see any edits to cfg.mk; not sure if you were trying 'make syntax-check' or if we may have some tweaks to clean up. Oh, and I guess you're still waiting on me to find time to tweak upstream gnulib to use $(SED) during syntax-check.
Indeed, I'm waiting for gnulib support for that. Anyway, I keep the stashed version of that change to cfg.mk to be able to run 'make syntax-check', but I don't include it in commits because it's a change on its own and doesn't belong here IMHO.
Overall, looks pretty good; thanks to others for doing reviews on the earlier versions, since it took me until v9 to take a peek :)
Thanks for the thoughtful review! I've addressed issues Daniel and you point out and uploaded v10. Hope I didn't miss anything, there were quite a few changes. Roman Bogorodskiy

On Tue, Feb 18, 2014 at 02:23:31PM +0400, Roman Bogorodskiy wrote:
Eric Blake wrote:
I didn't see any edits to cfg.mk; not sure if you were trying 'make syntax-check' or if we may have some tweaks to clean up. Oh, and I guess you're still waiting on me to find time to tweak upstream gnulib to use $(SED) during syntax-check.
Indeed, I'm waiting for gnulib support for that. Anyway, I keep the stashed version of that change to cfg.mk to be able to run 'make syntax-check', but I don't include it in commits because it's a change on its own and doesn't belong here IMHO.
I ran syntax-check against the patch on linux and it passed, so that's fine anyway. 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 :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Roman Bogorodskiy