[libvirt] [PATCH 0/4] bhyve integration: pie, blk, MAC, host API
Hi, I'm working on enabling OpenStack/libvirt support for FreeBSD hosts. Please look into some patches I'd like to submit for rewiev. 1. PIE-flag: on some FreeBSD-10 the clang toolchain is broken and cannot build executables with PIE option, so I added the flag to let the user choose. 2. bhyve MAC: add MAC address configuration for bhyve virtio network adapter 3. bhyve blk: add support for up to 8 blk devices, atapi/sata/virtio bus with file/block backend 4. bhyve host API: initial support for "host" functions on FreeBSD Regards, Wojtek Wojciech Macek (4): pie: add Position-Independent-Executable flag bhyve: MAC address configuration bhyve: multiple virtio-blk devices support bhyve: host API support configure.ac | 14 +++++++- src/bhyve/bhyve_command.c | 92 ++++++++++++++++++++++++++++++----------------- src/bhyve/bhyve_command.h | 8 +++++ src/bhyve/bhyve_driver.c | 66 ++++++++++++++++++++++++++++++++++ src/nodeinfo.c | 19 ++++++++++ 5 files changed, 165 insertions(+), 34 deletions(-) -- 1.9.0
Add possibility to choose whether the code is compiled with PIE or without. New configuration flags: --enable-pie (default) --disable-pie --- configure.ac | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 62b74c5..96a7038 100644 --- a/configure.ac +++ b/configure.ac @@ -215,7 +215,6 @@ fi # Check for compiler and library settings. LIBVIRT_COMPILE_WARNINGS -LIBVIRT_COMPILE_PIE LIBVIRT_LINKER_RELRO LIBVIRT_LINKER_NO_INDIRECT @@ -581,7 +580,20 @@ if test x"$enable_debug" = x"yes"; then AC_DEFINE([ENABLE_DEBUG], [], [whether debugging is enabled]) fi +dnl --enable-pie=(yes|no) +AC_ARG_ENABLE([pie], + [AS_HELP_STRING([--enable-pie=@<:@no|yes@:>@], + [enable Position-Independent-Executables @<:@default=yes@:>@])], + [],[enable_pie=yes]) +AM_CONDITIONAL([ENABLE_PIE], test x"$enable_pie" = x"yes") +if test x"$enable_debug" = x"yes"; then + AC_DEFINE([ENABLE_PIE], [], [whether Position-Independent-Executables are enabled]) +fi +# Check if Position-Independent-Executables should be supported +if test "$enable_pie" = "yes" ; then +LIBVIRT_COMPILE_PIE +fi dnl dnl init script flavor -- 1.9.0
On Thu, Mar 20, 2014 at 09:39:20AM +0100, Wojciech Macek wrote:
Add possibility to choose whether the code is compiled with PIE or without. New configuration flags: --enable-pie (default) --disable-pie --- configure.ac | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac index 62b74c5..96a7038 100644 --- a/configure.ac +++ b/configure.ac @@ -215,7 +215,6 @@ fi # Check for compiler and library settings.
LIBVIRT_COMPILE_WARNINGS -LIBVIRT_COMPILE_PIE LIBVIRT_LINKER_RELRO LIBVIRT_LINKER_NO_INDIRECT
@@ -581,7 +580,20 @@ if test x"$enable_debug" = x"yes"; then AC_DEFINE([ENABLE_DEBUG], [], [whether debugging is enabled]) fi
+dnl --enable-pie=(yes|no) +AC_ARG_ENABLE([pie], + [AS_HELP_STRING([--enable-pie=@<:@no|yes@:>@], + [enable Position-Independent-Executables @<:@default=yes@:>@])], + [],[enable_pie=yes]) +AM_CONDITIONAL([ENABLE_PIE], test x"$enable_pie" = x"yes") +if test x"$enable_debug" = x"yes"; then + AC_DEFINE([ENABLE_PIE], [], [whether Position-Independent-Executables are enabled]) +fi
+# Check if Position-Independent-Executables should be supported +if test "$enable_pie" = "yes" ; then +LIBVIRT_COMPILE_PIE +fi
IMHO the rationale behind this is pretty dubious. Can you elaborate on exactly what errors you're getting with PIE builds, since presumably they're working find for FreeBSD devs in general. If there are problems that can't be avoided, then adding a configure flag is still the wrong way to deal with them. We already probe to see if PIE mode works, so if that's not detecting a problem we must improve that detection. 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 :|
Ok, it seems I isolated the problem. The linking error shows up only on FreeBSD with gcc toolchain installed, because it does not support pie. With clang everything is fine. There is an issue with detection mechanism. Currently, it is done in configure script with: gcc -std=gnu99 -o conftest -g -O2 -x c -fPIE -DPIE -D_THREAD_SAFE conftest.c -lintl It succeeds, so the scripts enables PIE. However, all further binaries are built with additional "-pie" flag. /bin/sh ../libtool --tag=CC --mode=link gcc -std=gnu99 <some_stuff...> -fPIE -DPIE -g <other_stuff...> -pie -o libvirt_iohelper util/libvirt_iohelper-iohelper.o libvirt_util.la../gnulib/lib/ libgnu.la -lintl This one fails with /usr/local/bin/ld: /usr/lib/crt1.o: relocation R_X86_64_32 against `_DYNAMIC' can not be used when making a shared object; recompile with -fPIC /usr/lib/crt1.o: error adding symbols: Bad value collect2: ld returned 1 exit status If I modify configuration line to gcc -std=gnu99 -o conftest -g -O2 -x c -fPIE -DPIE -pie -D_THREAD_SAFE conftest.c -lintl then it correctly detects lack of pie and build succeeds. configure:61033: checking whether C compiler handles -fPIE -DPIE configure:61052: gcc -std=gnu99 -o conftest -g -O2 -pie -fPIE -DPIE -D_THREAD_SAFE conftest.c -lintl >&5 /usr/local/bin/ld: /usr/lib/crt1.o: relocation R_X86_64_32 against `_DYNAMIC' can not be used when making a shared object; recompile with -fPIC /usr/lib/crt1.o: error adding symbols: Bad value collect2: ld returned 1 exit status configure:61052: $? = 1 Do you think it would be reasonable to add "-pie" to configure script? Regards, Wojtek 2014-03-20 15:51 GMT+01:00 Daniel P. Berrange <berrange@redhat.com>:
On Thu, Mar 20, 2014 at 09:39:20AM +0100, Wojciech Macek wrote:
Add possibility to choose whether the code is compiled with PIE or without. New configuration flags: --enable-pie (default) --disable-pie --- configure.ac | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac index 62b74c5..96a7038 100644 --- a/configure.ac +++ b/configure.ac @@ -215,7 +215,6 @@ fi # Check for compiler and library settings.
LIBVIRT_COMPILE_WARNINGS -LIBVIRT_COMPILE_PIE LIBVIRT_LINKER_RELRO LIBVIRT_LINKER_NO_INDIRECT
@@ -581,7 +580,20 @@ if test x"$enable_debug" = x"yes"; then AC_DEFINE([ENABLE_DEBUG], [], [whether debugging is enabled]) fi
+dnl --enable-pie=(yes|no) +AC_ARG_ENABLE([pie], + [AS_HELP_STRING([--enable-pie=@<:@no|yes@:>@], + [enable Position-Independent-Executables @<:@default=yes@:>@])], + [],[enable_pie=yes]) +AM_CONDITIONAL([ENABLE_PIE], test x"$enable_pie" = x"yes") +if test x"$enable_debug" = x"yes"; then + AC_DEFINE([ENABLE_PIE], [], [whether Position-Independent-Executables are enabled]) +fi
+# Check if Position-Independent-Executables should be supported +if test "$enable_pie" = "yes" ; then +LIBVIRT_COMPILE_PIE +fi
IMHO the rationale behind this is pretty dubious. Can you elaborate on exactly what errors you're getting with PIE builds, since presumably they're working find for FreeBSD devs in general. If there are problems that can't be avoided, then adding a configure flag is still the wrong way to deal with them. We already probe to see if PIE mode works, so if that's not detecting a problem we must improve that detection.
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:|
Add support for MAC address configuration no network bridge interface. --- src/bhyve/bhyve_command.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 15029cd..42cab10 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -176,7 +176,11 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd) } virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "1:0,virtio-net,%s", realifname); + virCommandAddArgFormat(cmd, "1:0,virtio-net,%s,mac=%02x:%02x:%02x:%02x:%02x:%02x", + realifname, + net->mac.addr[0], net->mac.addr[1], + net->mac.addr[2], net->mac.addr[3], + net->mac.addr[4], net->mac.addr[5]); return 0; } -- 1.9.0
On Thu, Mar 20, 2014 at 09:39:21AM +0100, Wojciech Macek wrote:
Add support for MAC address configuration no network bridge interface. --- src/bhyve/bhyve_command.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 15029cd..42cab10 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -176,7 +176,11 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd) }
virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "1:0,virtio-net,%s", realifname); + virCommandAddArgFormat(cmd, "1:0,virtio-net,%s,mac=%02x:%02x:%02x:%02x:%02x:%02x", + realifname, + net->mac.addr[0], net->mac.addr[1], + net->mac.addr[2], net->mac.addr[3], + net->mac.addr[4], net->mac.addr[5]);
return 0; }
ACK 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:
On Thu, Mar 20, 2014 at 09:39:21AM +0100, Wojciech Macek wrote:
Add support for MAC address configuration no network bridge interface.
s/ no / on /
--- src/bhyve/bhyve_command.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 15029cd..42cab10 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -176,7 +176,11 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd) }
virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "1:0,virtio-net,%s", realifname); + virCommandAddArgFormat(cmd, "1:0,virtio-net,%s,mac=%02x:%02x:%02x:%02x:%02x:%02x", + realifname, + net->mac.addr[0], net->mac.addr[1], + net->mac.addr[2], net->mac.addr[3], + net->mac.addr[4], net->mac.addr[5]);
return 0; }
ACK
Pushed with changing MAC address formatting to use virMacAddrFormat(). Roman Bogorodskiy
Add support for multiple virtio-blk devices. Current implementation offers room for up to 8 disks and enumerates them as functions on PCI bus: 2:0, 2:1 ... 2:7 Bootable disk must be present on the first place in XML file. --- src/bhyve/bhyve_command.c | 86 +++++++++++++++++++++++++++++------------------ src/bhyve/bhyve_command.h | 8 +++++ 2 files changed, 62 insertions(+), 32 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 42cab10..7aa6ea9 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -189,43 +189,64 @@ static int bhyveBuildDiskArgStr(const virDomainDef *def, virCommandPtr cmd) { virDomainDiskDefPtr disk; - const char *bus_type; - - if (def->ndisks != 1) { + size_t diskno; + virBhyveDiskCmdMappingPtr pmapping; + /* + * Supported combinations: + * - BUS_SATA:DEVICE_DISK:TYPE_FILE + * - BUS_VIRTIO:DEVICE_DISK:TYPE_FILE + * - BUS_IDE:DEVICE_CDROM:TYPE_FILE + * - BUS_SATA:DEVICE_DISK:TYPE_BLOCK + * - BUS_VIRTIO:DEVICE_DISK:TYPE_BLOCK + * - BUS_IDE:DEVICE_CDROM:TYPE_BLOCK + */ + virBhyveDiskCmdMapping mapping[] = { + {VIR_DOMAIN_DISK_BUS_SATA, VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_TYPE_FILE, "ahci-hd"}, + {VIR_DOMAIN_DISK_BUS_VIRTIO, VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_TYPE_FILE, "virtio-blk"}, + {VIR_DOMAIN_DISK_BUS_IDE, VIR_DOMAIN_DISK_DEVICE_CDROM, + VIR_DOMAIN_DISK_TYPE_FILE, "ahci-cd"}, + {VIR_DOMAIN_DISK_BUS_SATA, VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_TYPE_BLOCK, "ahci-hd"}, + {VIR_DOMAIN_DISK_BUS_VIRTIO, VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_TYPE_BLOCK, "virtio-blk"}, + {VIR_DOMAIN_DISK_BUS_IDE, VIR_DOMAIN_DISK_DEVICE_CDROM, + VIR_DOMAIN_DISK_TYPE_BLOCK, "ahci-cd"}, + {-1, -1, -1, NULL} + }; + + if ((def->ndisks > 8) || (def->ndisks < 1)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("domain should have one and only one disk defined")); + _("domain supports 1 to 8 disks")); return -1; } - disk = def->disks[0]; + for (diskno = 0; diskno < def->ndisks; diskno++) { + const char *busname = NULL; - switch (disk->bus) { - case VIR_DOMAIN_DISK_BUS_SATA: - bus_type = "ahci-hd"; - break; - case VIR_DOMAIN_DISK_BUS_VIRTIO: - bus_type = "virtio-blk"; - break; - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unsupported disk bus type")); - return -1; - } + disk = def->disks[diskno]; - if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unsupported disk device")); - return -1; - } + pmapping = &mapping[0]; + while (pmapping->type != -1) { + if ((disk->bus == pmapping->bus) && + (disk->device == pmapping->device) && + (disk->type == pmapping->type)) { + busname = pmapping->devname; + break; + } + pmapping++; + } - if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unsupported disk type")); - return -1; - } + if (busname == NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("unsupported disk:bus:type combination")); + return -1; + } - virCommandAddArg(cmd, "-s"); - virCommandAddArgFormat(cmd, "2:0,%s,%s", bus_type, disk->src); + virCommandAddArg(cmd, "-s"); + virCommandAddArgFormat(cmd, "2:%zu,%s,%s", diskno, busname, disk->src); + } return 0; } @@ -307,9 +328,9 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, virCommandPtr cmd; virDomainDiskDefPtr disk; - if (vm->def->ndisks != 1) { + if (vm->def->ndisks < 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("domain should have one and only one disk defined")); + _("domain should have at least one disk defined")); return NULL; } @@ -321,7 +342,8 @@ virBhyveProcessBuildLoadCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED, return NULL; } - if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) { + if ((disk->type != VIR_DOMAIN_DISK_TYPE_FILE) && + (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unsupported disk type")); return NULL; diff --git a/src/bhyve/bhyve_command.h b/src/bhyve/bhyve_command.h index 66d934d..e93d1f0 100644 --- a/src/bhyve/bhyve_command.h +++ b/src/bhyve/bhyve_command.h @@ -27,6 +27,14 @@ # include "domain_conf.h" # include "vircommand.h" +typedef struct _virBhyveDiskCmdMapping { + int bus; + int device; + int type; + const char *devname; +} virBhyveDiskCmdMapping; +typedef virBhyveDiskCmdMapping *virBhyveDiskCmdMappingPtr; + virCommandPtr virBhyveProcessBuildBhyveCmd(bhyveConnPtr, virDomainObjPtr vm); -- 1.9.0
On Thu, Mar 20, 2014 at 09:39:22AM +0100, Wojciech Macek wrote:
Add support for multiple virtio-blk devices. Current implementation offers room for up to 8 disks and enumerates them as functions on PCI bus: 2:0, 2:1 ... 2:7
Use of PCI functions should be avoided by default, since it prevents you from supporting hotplug/unplug. You want virtio-blk devices to be set on individual PCI slots instead, which allows upto 32 devices in total (though VGA, host bridge & nic consume some of those). Also we need to assign & track PCI addresses in the XML explicitly so that upon migraton the addresses don't change. IOW, the bhyve driver really needs to copy the qemuDomainAssignPCIAddresses code in this area, and then honour the address info when building the command line 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 :|
On 03/20/2014 06:03 AM, Daniel P. Berrange wrote:
On Thu, Mar 20, 2014 at 09:39:22AM +0100, Wojciech Macek wrote:
Add support for multiple virtio-blk devices. Current implementation offers room for up to 8 disks and enumerates them as functions on PCI bus: 2:0, 2:1 ... 2:7 Use of PCI functions should be avoided by default, since it prevents you from supporting hotplug/unplug. You want virtio-blk devices to be set on individual PCI slots instead, which allows upto 32 devices in total (though VGA, host bridge & nic consume some of those).
Does bhyve support multiple buses and a pci-bridge device? If so, it's probably better for libvirt's bhyve driver to add support for that sooner rather than later, to avoid hard-coding a bunch of assumptions about bus=0 that would need to be audited/changed later. (Thinking about this is making me wonder if the PCI slot allocation that is in the qemu driver should be moved into a library in util so that other hypervisors can use it. It would take some work to extract the qemu idiosyncracies out of it, but some of that is already half done due to the differences between the i440fx based machinetypes and q35. Of course it would probably be pointless unless other hypervisors also support multiple buses).
New functionalities: - connectGetMaxVcpus - on bhyve hardcode this value to 16 - nodeGetFreeMemory - do not use physmem_get on FreeBSD, since it might get wrong value on systems with more than 100GB of RAM - nodeGetCPUMap - wrapper only for mapping function, currently not supported by FreeBSD - nodeSet/GetMemoryParameters - wrapper only for future improvements, currently not supported by FreeBSD --- src/bhyve/bhyve_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ src/nodeinfo.c | 19 ++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index baa3340..2e6a8cb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -694,6 +694,67 @@ cleanup: return -1; } +static int +bhyveConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *type) { + if (virConnectGetMaxVcpusEnsureACL(conn) < 0) + return -1; + + /* + * Bhyve supports up to 16 VCPUs, but offers no method to check this + * value. Hardcode 16... + */ + if (!type || STRCASEEQ(type, "bhyve")) + return 16; + + virReportError(VIR_ERR_INVALID_ARG, _("unknown type '%s'"), type); + return -1; +} + +static unsigned long long +bhyveNodeGetFreeMemory(virConnectPtr conn) +{ + if (virNodeGetFreeMemoryEnsureACL(conn) < 0) + return 0; + + return nodeGetFreeMemory(); +} + +static int +bhyveNodeGetCPUMap(virConnectPtr conn, + unsigned char **cpumap, + unsigned int *online, + unsigned int flags) +{ + if (virNodeGetCPUMapEnsureACL(conn) < 0) + return -1; + + return nodeGetCPUMap(cpumap, online, flags); +} + +static int +bhyveNodeGetMemoryParameters(virConnectPtr conn, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + if (virNodeGetMemoryParametersEnsureACL(conn) < 0) + return -1; + + return nodeGetMemoryParameters(params, nparams, flags); +} + +static int +bhyveNodeSetMemoryParameters(virConnectPtr conn, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + if (virNodeSetMemoryParametersEnsureACL(conn) < 0) + return -1; + + return nodeSetMemoryParameters(params, nparams, flags); +} static virDriver bhyveDriver = { .no = VIR_DRV_BHYVE, @@ -722,6 +783,11 @@ static virDriver bhyveDriver = { .nodeGetCPUStats = bhyveNodeGetCPUStats, /* 1.2.2 */ .nodeGetMemoryStats = bhyveNodeGetMemoryStats, /* 1.2.2 */ .nodeGetInfo = bhyveNodeGetInfo, /* 1.2.3 */ + .connectGetMaxVcpus = bhyveConnectGetMaxVcpus, /* 1.2.3 */ + .nodeGetFreeMemory = bhyveNodeGetFreeMemory, /* 1.2.3 */ + .nodeGetCPUMap = bhyveNodeGetCPUMap, /* 1.2.3 */ + .nodeGetMemoryParameters = bhyveNodeGetMemoryParameters, /* 1.2.3 */ + .nodeSetMemoryParameters = bhyveNodeSetMemoryParameters, /* 1.2.3 */ }; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 6d33f64..7996d55 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1699,6 +1699,24 @@ nodeGetCellsFreeMemoryFake(unsigned long long *freeMems, static unsigned long long nodeGetFreeMemoryFake(void) { +#if defined(__FreeBSD__) + unsigned long pagesize = getpagesize(); + u_int value; + size_t value_size = sizeof(value); + unsigned long long freemem; + + if (sysctlbyname("vm.stats.vm.v_free_count", &value, + &value_size, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("sysctl failed for vm.stats.vm.v_free_count")); + return 0; + } + + freemem = value; + freemem = freemem * (unsigned long long)pagesize; + + return freemem; +#else double avail = physmem_available(); unsigned long long ret; @@ -1709,6 +1727,7 @@ nodeGetFreeMemoryFake(void) } return ret; +#endif } /* returns 1 on success, 0 if the detection failed and -1 on hard error */ -- 1.9.0
On Thu, Mar 20, 2014 at 09:39:23AM +0100, Wojciech Macek wrote:
New functionalities: - connectGetMaxVcpus - on bhyve hardcode this value to 16 - nodeGetFreeMemory - do not use physmem_get on FreeBSD, since it might get wrong value on systems with more than 100GB of RAM - nodeGetCPUMap - wrapper only for mapping function, currently not supported by FreeBSD - nodeSet/GetMemoryParameters - wrapper only for future improvements, currently not supported by FreeBSD --- src/bhyve/bhyve_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ src/nodeinfo.c | 19 ++++++++++++++ 2 files changed, 85 insertions(+)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index baa3340..2e6a8cb 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -694,6 +694,67 @@ cleanup: return -1; }
+static int +bhyveConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *type) {
Mis-aligned indent
+ if (virConnectGetMaxVcpusEnsureACL(conn) < 0) + return -1; + + /* + * Bhyve supports up to 16 VCPUs, but offers no method to check this + * value. Hardcode 16... + */ + if (!type || STRCASEEQ(type, "bhyve")) + return 16; + + virReportError(VIR_ERR_INVALID_ARG, _("unknown type '%s'"), type); + return -1; +} + +static unsigned long long +bhyveNodeGetFreeMemory(virConnectPtr conn) +{ + if (virNodeGetFreeMemoryEnsureACL(conn) < 0) + return 0; + + return nodeGetFreeMemory(); +} + +static int +bhyveNodeGetCPUMap(virConnectPtr conn, + unsigned char **cpumap, + unsigned int *online, + unsigned int flags)
miss-aligned indent
+{ + if (virNodeGetCPUMapEnsureACL(conn) < 0) + return -1; + + return nodeGetCPUMap(cpumap, online, flags); +} + +static int +bhyveNodeGetMemoryParameters(virConnectPtr conn, + virTypedParameterPtr params, + int *nparams, + unsigned int flags)
miss-aligned indent
+{ + if (virNodeGetMemoryParametersEnsureACL(conn) < 0) + return -1; + + return nodeGetMemoryParameters(params, nparams, flags); +} + +static int +bhyveNodeSetMemoryParameters(virConnectPtr conn, + virTypedParameterPtr params, + int nparams, + unsigned int flags) miss-aligned indent
+{ + if (virNodeSetMemoryParametersEnsureACL(conn) < 0) + return -1; + + return nodeSetMemoryParameters(params, nparams, flags); +}
static virDriver bhyveDriver = { .no = VIR_DRV_BHYVE, @@ -722,6 +783,11 @@ static virDriver bhyveDriver = { .nodeGetCPUStats = bhyveNodeGetCPUStats, /* 1.2.2 */ .nodeGetMemoryStats = bhyveNodeGetMemoryStats, /* 1.2.2 */ .nodeGetInfo = bhyveNodeGetInfo, /* 1.2.3 */ + .connectGetMaxVcpus = bhyveConnectGetMaxVcpus, /* 1.2.3 */ + .nodeGetFreeMemory = bhyveNodeGetFreeMemory, /* 1.2.3 */ + .nodeGetCPUMap = bhyveNodeGetCPUMap, /* 1.2.3 */ + .nodeGetMemoryParameters = bhyveNodeGetMemoryParameters, /* 1.2.3 */ + .nodeSetMemoryParameters = bhyveNodeSetMemoryParameters, /* 1.2.3 */ };
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 6d33f64..7996d55 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1699,6 +1699,24 @@ nodeGetCellsFreeMemoryFake(unsigned long long *freeMems, static unsigned long long nodeGetFreeMemoryFake(void) { +#if defined(__FreeBSD__) + unsigned long pagesize = getpagesize(); + u_int value; + size_t value_size = sizeof(value); + unsigned long long freemem; + + if (sysctlbyname("vm.stats.vm.v_free_count", &value, + &value_size, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("sysctl failed for vm.stats.vm.v_free_count")); + return 0; + } + + freemem = value; + freemem = freemem * (unsigned long long)pagesize;
Huh, why not just do it on 1 line freemem = value * (unsigned long long)pagesize;
+ + return freemem; +#else double avail = physmem_available(); unsigned long long ret;
@@ -1709,6 +1727,7 @@ nodeGetFreeMemoryFake(void) }
return ret; +#endif }
ACK if the nitpicks are fixed. 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:
On Thu, Mar 20, 2014 at 09:39:23AM +0100, Wojciech Macek wrote:
New functionalities: - connectGetMaxVcpus - on bhyve hardcode this value to 16 - nodeGetFreeMemory - do not use physmem_get on FreeBSD, since it might get wrong value on systems with more than 100GB of RAM - nodeGetCPUMap - wrapper only for mapping function, currently not supported by FreeBSD - nodeSet/GetMemoryParameters - wrapper only for future improvements, currently not supported by FreeBSD --- src/bhyve/bhyve_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ src/nodeinfo.c | 19 ++++++++++++++ 2 files changed, 85 insertions(+)
ACK if the nitpicks are fixed.
I'll give it a test and then push with the nitpick fixes squashed in. Roman Bogorodskiy
Roman Bogorodskiy wrote:
Daniel P. Berrange wrote:
On Thu, Mar 20, 2014 at 09:39:23AM +0100, Wojciech Macek wrote:
New functionalities: - connectGetMaxVcpus - on bhyve hardcode this value to 16 - nodeGetFreeMemory - do not use physmem_get on FreeBSD, since it might get wrong value on systems with more than 100GB of RAM - nodeGetCPUMap - wrapper only for mapping function, currently not supported by FreeBSD - nodeSet/GetMemoryParameters - wrapper only for future improvements, currently not supported by FreeBSD --- src/bhyve/bhyve_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ src/nodeinfo.c | 19 ++++++++++++++ 2 files changed, 85 insertions(+)
ACK if the nitpicks are fixed.
I'll give it a test and then push with the nitpick fixes squashed in.
Pushed, thanks! Roman Bogorodskiy
participants (4)
-
Daniel P. Berrange -
Laine Stump -
Roman Bogorodskiy -
Wojciech Macek