On Thu, Jan 23, 2014 at 11:05:26PM +0400, Roman Bogorodskiy wrote:
At this point it has a limited functionality and is highly experimental. Supported domain operations are:
* define * start * destroy * dumpxml * dominfo
It's only possible to have only one disk device and only one network, which should be of type bridge.
diff --git a/configure.ac b/configure.ac index 3a70375..bfbd3a8 100644 --- a/configure.ac +++ b/configure.ac @@ -524,6 +524,10 @@ AC_ARG_WITH([parallels], [AS_HELP_STRING([--with-parallels], [add Parallels Cloud Server support @<:@default=check@:>@])]) m4_divert_text([DEFAULTS], [with_parallels=check]) +AC_ARG_WITH([bhyve], + [AS_HELP_STRING([--with-bhyve], + [add BHyVe support @<:@default=check@:>@])]) +m4_divert_text([DEFAULTS], [with_bhyve=check])
I think it is probably possible to move this into the LIBVIRT_DRIVER_CHECK_BHYVE macro too. If that doesn't work then just create a LIBVIRT_DRIVER_ARG_BHYVE macro for it. Then we have everything in the isolated .m4 file
AC_ARG_WITH([test], [AS_HELP_STRING([--with-test], [add test driver support @<:@default=yes@:>@])]) @@ -1031,6 +1035,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 @@ -2677,6 +2687,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]) +LIBIVRT_DRIVER_RESULT_BHYVE AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) AC_MSG_NOTICE([ Network: $with_network])
diff --git a/src/Makefile.am b/src/Makefile.am index 8f77658..6b95f64 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -515,6 +515,7 @@ DRIVER_SOURCE_FILES = \ $(NULL)
STATEFUL_DRIVER_SOURCE_FILES = \ + $(BHYVE_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ $(LIBXL_DRIVER_SOURCES) \ $(LXC_DRIVER_SOURCES) \
Nit-pick - inconsistent indentation - the other lines use tabs, but you've used spaces. Although we dislike tabs, just follow existing practice in this file
@@ -772,6 +773,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
Same indentation note here.
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c new file mode 100644 index 0000000..f2988cf --- /dev/null +++ b/src/bhyve/bhyve_command.c @@ -0,0 +1,269 @@
+static char* +virTapGetRealDeviceName(char *name)
Can you aim to use 'virBhyve' as the pefix for absolutely every function and struct you define in src/bhyve/ files. We've not been all that good at this in other virt drivers but its the rough goal we're aiming for is to have all function prefixes match filename prefix.
+{ + /* 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");
'*' should associate with the variable rather than the type.
+ if (dirp == NULL) { + return NULL; + }
virReportSystemError() when this fails.
+ + 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) + goto cleanup;
virReportSystemError()
+ + if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) + goto cleanup;
virReportSystemError()
+ + 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 = virTapGetRealDeviceName(net->ifname); + + if (realifname == NULL) { + VIR_FREE(net->ifname); + VIR_FREE(brname); + return -1; + } + + VIR_INFO("%s -> %s", net->ifname, realifname);
s/INFO/DEBUG/
+ /* 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; +}
+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 = NULL; + cmd = virCommandNew(BHYVE);
simpler to merge these onto 1 line.
+ + /* 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 */ + virCommandAddArg(cmd, "-A"); /* Create an ACPI table */
You can look at the 'acpi' features flag to turn this on/off
+ virCommandAddArg(cmd, "-I"); /* Present ioapic to the guest */
Likewise you can look at the 'apic' features flag to turn this on/off
+ virCommandAddArg(cmd, "-H"); /* vmexit from guest on hlt */ + virCommandAddArg(cmd, "-P"); /* vmexit from guest on pause */
What's the functional effect of having these set, or not ?
+ + /* Devices */ + if (bhyveBuildNetArgStr(vm->def, cmd) < 0) + goto error; + if (bhyveBuildDiskArgStr(vm->def, cmd) < 0) + goto error; + virCommandAddArg(cmd, "-S"); + virCommandAddArg(cmd, "31,uart"); + 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 = vm->def->disks[0]; + + 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_driver.c b/src/bhyve/bhyve_driver.c new file mode 100644 index 0000000..5ddb26a --- /dev/null +++ b/src/bhyve/bhyve_driver.c
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c new file mode 100644 index 0000000..95553d7 --- /dev/null +++ b/src/bhyve/bhyve_process.c @@ -0,0 +1,205 @@ +/* + * 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 <dirent.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 ATTRIBUTE_UNUSED) +{ + char *logfile = NULL; + int logfd = -1; + off_t pos = -1; + char ebuf[1024]; + virCommandPtr cmd = NULL; + bhyveConnPtr privconn = conn->privateData; + int ret = -1, status; + + if (vm->def->ndisks != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Domain should have one and only one disk defined")); + return -1; + }
I'd suggest that can live in just the virBhyveProcessBuildBhyveCmd method
+int +virBhyveProcessStop(bhyveConnPtr driver, + virDomainObjPtr vm, + virDomainShutoffReason reason ATTRIBUTE_UNUSED) +{ + int i; + int ret = -1; + int status; + virCommandPtr cmd = NULL; +
Still want to sanity check with virDomainObjIsActive() before proceeeding there
+ /* First, try to kill 'bhyve' process */ + if (virProcessKillPainfully(vm->pid, true) != 0) + VIR_WARN("Failed to gracefully stop bhyve VM '%s' (pid: %llu)", + vm->def->name, + (unsigned long long)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; + +cleanup: + virCommandFree(cmd); + return ret; +} +
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 :|