[libvirt] [PATCH v2 Repost 0/5] bhyve: virConnectDomainXMLFromNative
I'm reposting this series because I got the following two error messages from the redhat.com MX: libvir-list@redhat.com SMTP error from remote mail server after RCPT TO:<libvir-list@redhat.com>: host mx1.redhat.com [209.132.183.28]: 554 5.7.1 <libvir-list@redhat.com>: Recipient address rejected: Access denied <libvir-list@redhat.com>: host int-mx.corp.redhat.com[10.4.122.10] said: 550 5.1.1 <libvir-list@redhat.com>... User unknown (in reply to RCPT TO command) Sorry for the noise. Differences to v1: - use gnulib's reentrant getopt implementation. This is necessary for thread safety. - config-post.h: __GNUC_PREREQ is defined here, since using gnulib's getopt pulls in other gnulib headers, which rely on __GNUC_PREREQ, which doesn't exist on FreeBSD. This approach is open for discussion: I chose config-post.h as this would likely always be the first header pulled in (through config.h). Link to v1: https://www.redhat.com/archives/libvir-list/2016-June/msg00001.html Fabian Freyer (5): config-post.h: define __GNUC_PREREQ if not defined gnulib: add getopt module bhyve: implement virConnectDomainXMLFromNative bhyve: implement bhyve argument parser bhyve: implement argument parser for loader bootstrap.conf | 1 + config-post.h | 11 + m4/virt-driver-bhyve.m4 | 3 + po/POTFILES.in | 1 + src/Makefile.am | 2 + src/bhyve/bhyve_driver.c | 42 ++ src/bhyve/bhyve_parse_command.c | 875 ++++++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_parse_command.h | 30 ++ 8 files changed, 965 insertions(+) create mode 100644 src/bhyve/bhyve_parse_command.c create mode 100644 src/bhyve/bhyve_parse_command.h -- 2.7.0
A simple getopt-based argument parser is added for the /usr/sbin/bhyve command, loosely based on its argument parser, which reads the following from the bhyve command line string: * vm name * number of vcpus * memory size * the time offset (UTC or localtime). This includes a capability check to see if this is actually supported by the bhyve version. * features: * acpi * ioapic: While this flag is deprecated in FreeBSD r257423, keep checking for it for backwards compatibiility. * the domain UUID; if not explicitely given, one will be generated. * lpc devices: for now only the com1 and com2 are supported. It is required for these to be /dev/nmdm[\d+][AB], and the slave devices are automatically inferred from these to be the corresponding end of the virtual null-modem cable: /dev/nmdm<N>A <-> /dev/nmdm<N>B * PCI devices: * Disks: these are numbered in the order they are found, for virtio and ahci disks separately. The destination is set to sdX or vdX with X='a'+index; therefore only 'z'-'a' disks are supported. Disks are considered to be block devices if the path starts with /dev, otherwise they are considered to be files. * Networks: only tap devices are supported. Since it isn't possible to tell the type of the network, VIR_DOMAIN_NET_TYPE_ETHERNET is assumed, since it is the most generic. If no mac is specified, one will be generated. Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de> --- src/bhyve/bhyve_parse_command.c | 494 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 492 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 72367bb..be4ff2a 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -23,6 +23,7 @@ */ #include <config.h> +#include <getopt_int.h> #include "bhyve_capabilities.h" #include "bhyve_command.h" @@ -225,10 +226,496 @@ bhyveCommandLine2argv(const char *nativeConfig, return -1; } +static int +bhyveParseBhyveLPCArg(virDomainDefPtr def, + unsigned caps ATTRIBUTE_UNUSED, + const char *arg) +{ + /* -l emulation[,config] */ + const char *separator = NULL; + const char *param = NULL; + size_t last = 0; + virDomainChrDefPtr chr = NULL; + char *type = NULL; + + separator = strchr(arg, ','); + param = separator + 1; + + if (!separator) + goto error; + + if (VIR_STRNDUP(type, arg, separator - arg) < 0) + goto error; + + /* Only support com%d */ + if (STRPREFIX(type, "com") && type[4] == 0) { + if (!(chr = virDomainChrDefNew())) + goto error; + + chr->source.type = VIR_DOMAIN_CHR_TYPE_NMDM; + chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; + + if (!STRPREFIX(param, "/dev/nmdm")) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to set com port %s: does not start with " + "'/dev/nmdm'."), type); + goto error; + } + + if (VIR_STRDUP(chr->source.data.file.path, param) < 0) { + virDomainChrDefFree(chr); + goto error; + } + + if (VIR_STRDUP(chr->source.data.nmdm.slave, chr->source.data.file.path) + < 0) { + virDomainChrDefFree(chr); + goto error; + } + + /* If the last character of the master is 'A', the slave will be 'B' + * and vice versa */ + last = strlen(chr->source.data.file.path) - 1; + switch (chr->source.data.file.path[last]) { + case 'A': + chr->source.data.file.path[last] = 'B'; + break; + case 'B': + chr->source.data.file.path[last] = 'A'; + break; + default: + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to set slave for %s: last letter not " + "'A' or 'B'"), + chr->source.data.file.path); + goto error; + } + + switch (type[3]-'0') { + case 1: + case 2: + chr->target.port = type[3] - '1'; + break; + default: + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse %s: only com1 and com2" + "supported."), type); + virDomainChrDefFree(chr); + goto error; + break; + } + + if (VIR_APPEND_ELEMENT(def->serials, def->nserials, chr) < 0) { + virDomainChrDefFree(chr); + goto error; + } + } + + VIR_FREE(type); + return 0; + +error: + VIR_FREE(chr); + VIR_FREE(type); + return -1; +} + +static int +bhyveParsePCISlot(const char *slotdef, + unsigned *pcislot, + unsigned *bus, + unsigned *function) +{ + /* slot[:function] | bus:slot:function */ + const char *curr = NULL; + const char *next = NULL; + unsigned values[3]; + int i; + + curr = slotdef; + for (i = 0; i < 3; i++) { + char *val = NULL; + + next = strchr(curr, ':'); + + if (VIR_STRNDUP(val, curr, next? next - curr : -1) < 0) + goto error; + + if (virStrToLong_ui(val, NULL, 10, &values[i]) < 0) + goto error; + + VIR_FREE(val); + + if (!next) + break; + + curr = next +1; + } + + *bus = 0; + *pcislot = 0; + *function = 0; + + switch (i + 1) { + case 2: + /* pcislot[:function] */ + *function = values[1]; + case 1: + *pcislot = values[0]; + break; + case 3: + /* bus:pcislot:function */ + *bus = values[0]; + *pcislot = values[1]; + *function = values[2]; + break; + } + + return 0; +error: + return -1; +} + +static int +bhyveParsePCIDisk(virDomainDefPtr def, + unsigned caps ATTRIBUTE_UNUSED, + unsigned pcislot, + unsigned pcibus, + unsigned function, + int bus, + int device, + unsigned *nvirtiodisk, + unsigned *nahcidisk, + char *config) +{ + /* -s slot,virtio-blk|ahci-cd|ahci-hd,/path/to/file */ + const char *separator = NULL; + int index = -1; + virDomainDiskDefPtr disk = NULL; + + if (VIR_ALLOC(disk) < 0) + goto cleanup; + if (VIR_ALLOC(disk->src) < 0) + goto error; + + disk->bus = bus; + disk->device = device; + + disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + disk->info.addr.pci.slot = pcislot; + disk->info.addr.pci.bus = pcibus; + disk->info.addr.pci.function = function; + + if (STRPREFIX(config, "/dev/")) + disk->src->type = VIR_STORAGE_TYPE_BLOCK; + else + disk->src->type = VIR_STORAGE_TYPE_FILE; + + if (!config) + goto error; + + separator = strchr(config, ','); + if (VIR_STRNDUP(disk->src->path, config, + separator? separator - config : -1) < 0) + goto error; + + if (bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { + index = *nvirtiodisk++; + if (VIR_STRDUP(disk->dst, "vda") < 0) + goto error; + } + + else if (bus == VIR_DOMAIN_DISK_BUS_SATA) { + index = *nahcidisk++; + if (VIR_STRDUP(disk->dst, "sda") < 0) + goto error; + } + + if (index > 'z' - 'a') + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("too many disks")); + + disk->dst[2] = 'a' + index; + + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) + goto error; + +cleanup: + return 0; + +error: + virDomainDiskDefFree(disk); + return -1; +} + +static int +bhyveParsePCINet(virDomainDefPtr def, + virDomainXMLOptionPtr xmlopt, + unsigned caps ATTRIBUTE_UNUSED, + unsigned pcislot, + unsigned pcibus, + unsigned function, + const char *config) +{ + /* -s slot,virtio-net,tapN[,mac=xx:xx:xx:xx:xx:xx] */ + + virDomainNetDefPtr net = NULL; + const char *separator = NULL; + const char *mac = NULL; + + if (VIR_ALLOC(net) < 0) + goto cleanup; + + /* Let's just assume it is VIR_DOMAIN_NET_TYPE_ETHERNET, it could also be + * a bridge, but this is the most generic option. */ + net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; + + net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + net->info.addr.pci.slot = pcislot; + net->info.addr.pci.bus = pcibus; + net->info.addr.pci.function = function; + + if (!config) + goto error; + + if (!STRPREFIX(config, "tap")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Only tap devices supported.")); + goto error; + } + + separator = strchr(config, ','); + if (VIR_STRNDUP(net->ifname, config, + separator? separator - config : -1) < 0) + goto error; + + if (!separator) + goto cleanup; + + if (!STRPREFIX(++separator, "mac=")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Only mac option can be specified for virt-net")); + goto error; + } + mac = separator + 4; + + if (virMacAddrParse(mac, &net->mac) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to parse mac address '%s'"), + mac); + goto cleanup; + } + +cleanup: + if (!mac) + virDomainNetGenerateMAC(xmlopt, &net->mac); + + if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) + goto error; + return 0; + +error: + virDomainNetDefFree(net); + return -1; +} + +static int +bhyveParseBhyvePCIArg(virDomainDefPtr def, + virDomainXMLOptionPtr xmlopt, + unsigned caps, + unsigned *nvirtiodisk, + unsigned *nahcidisk, + const char *arg) +{ + /* -s slot,emulation[,conf] */ + const char *separator = NULL; + char *slotdef = NULL; + char *emulation = NULL; + char *conf = NULL; + unsigned pcislot, bus, function; + + separator = strchr(arg, ','); + + if (!separator) + goto error; + else + separator++; /* Skip comma */ + + if (VIR_STRNDUP(slotdef, arg, separator - arg - 1) < 0) + goto error; + + conf = strchr(separator+1, ','); + if (conf) + conf++; /* Skip initial comma */ + + if (VIR_STRNDUP(emulation, separator, conf? conf - separator - 1 : -1) < 0) + goto error; + + if (bhyveParsePCISlot(slotdef, &pcislot, &bus, &function) < 0) + goto error; + + if (STREQ(emulation, "ahci-cd")) + bhyveParsePCIDisk(def, caps, pcislot, bus, function, + VIR_DOMAIN_DISK_BUS_SATA, + VIR_DOMAIN_DISK_DEVICE_CDROM, + nvirtiodisk, + nahcidisk, + conf); + else if (STREQ(emulation, "ahci-hd")) + bhyveParsePCIDisk(def, caps, pcislot, bus, function, + VIR_DOMAIN_DISK_BUS_SATA, + VIR_DOMAIN_DISK_DEVICE_DISK, + nvirtiodisk, + nahcidisk, + conf); + else if (STREQ(emulation, "virtio-blk")) + bhyveParsePCIDisk(def, caps, pcislot, bus, function, + VIR_DOMAIN_DISK_BUS_VIRTIO, + VIR_DOMAIN_DISK_DEVICE_DISK, + nvirtiodisk, + nahcidisk, + conf); + else if (STREQ(emulation, "virtio-net")) + bhyveParsePCINet(def, xmlopt, caps, pcislot, bus, function, conf); + + VIR_FREE(emulation); + VIR_FREE(slotdef); + return 0; +error: + VIR_FREE(emulation); + VIR_FREE(slotdef); + return -1; +} + +/* + * Parse the /usr/sbin/bhyve command line. + */ +static int +bhyveParseBhyveCommandLine(virDomainDefPtr def, + virDomainXMLOptionPtr xmlopt, + unsigned caps, + int argc, char **argv) +{ + int c; + const char optstr[] = "abehuwxACHIPSWYp:g:c:s:m:l:U:"; + int vcpus = 1; + size_t memory = 0; + unsigned nahcidisks = 0; + unsigned nvirtiodisks = 0; + struct _getopt_data *parser; + + if (!argv) + goto error; + + if (VIR_ALLOC(parser) < 0) + goto error; + + while ((c = _getopt_internal_r(argc, argv, optstr, + NULL, NULL, 0, parser, 0)) != -1) { + switch (c) { + case 'A': + def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_TRISTATE_SWITCH_ON; + break; + case 'c': + if (virStrToLong_i(parser->optarg, NULL, 10, &vcpus) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse number of vCPUs.")); + goto error; + } + if (virDomainDefSetVcpusMax(def, vcpus) < 0) + goto error; + if (virDomainDefSetVcpus(def, vcpus) < 0) + goto error; + break; + case 'l': + if (bhyveParseBhyveLPCArg(def, caps, parser->optarg)) + goto error; + break; + case 's': + if (bhyveParseBhyvePCIArg(def, + xmlopt, + caps, + &nahcidisks, + &nvirtiodisks, + parser->optarg)) + goto error; + break; + case 'm': + if (virStrToLong_ul(parser->optarg, NULL, 10, &memory) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse Memory.")); + goto error; + } + /* For compatibility reasons, assume memory is givin in MB + * when < 1024, otherwise it is given in bytes */ + if (memory < 1024) + memory *= 1024; + else + memory /= 1024UL; + if (def->mem.cur_balloon != 0 && def->mem.cur_balloon != memory) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse Memory: Memory size mismatch.")); + goto error; + } + def->mem.cur_balloon = memory; + virDomainDefSetMemoryTotal(def, memory); + break; + break; + case 'I': + /* While this flag was deprecated in FreeBSD r257423, keep checking + * for it for backwards compatibility. */ + def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_TRISTATE_SWITCH_ON; + break; + case 'u': + if ((caps & BHYVE_CAP_RTC_UTC) != 0) { + def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Installed bhyve binary does not support " + "UTC clock")); + goto error; + } + break; + case 'U': + if (virUUIDParse(parser->optarg, def->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, \ + _("cannot parse UUID '%s'"), parser->optarg); + goto error; + } + break; + } + } + + if (argc != parser->optind) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse arguments for bhyve command.")); + goto error; + } + + if (def->name == NULL) { + if (VIR_STRDUP(def->name, argv[argc]) < 0) + goto error; + } + else if (STRNEQ(def->name, argv[argc])) { + /* the vm name of the loader and the bhyverun command differ, throw an + * error here */ + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse arguments: VM name mismatch.")); + goto error; + } + + VIR_FREE(parser); + return 0; + +error: + VIR_FREE(parser); + return -1; +} + virDomainDefPtr bhyveParseCommandLineString(const char* nativeConfig, - unsigned caps ATTRIBUTE_UNUSED, - virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED) + unsigned caps, + virDomainXMLOptionPtr xmlopt) { virDomainDefPtr def = NULL; int bhyve_argc = 0; @@ -256,6 +743,9 @@ bhyveParseCommandLineString(const char* nativeConfig, goto cleanup; } + if (bhyveParseBhyveCommandLine(def, xmlopt, caps, bhyve_argc, bhyve_argv)) + goto cleanup; + cleanup: virStringFreeList(loader_argv); virStringFreeList(bhyve_argv); -- 2.7.0
A simple getopt-based argument parser is added for the /usr/sbin/bhyveload command, loosely based on its argument parser. The boot disk is guessed by iterating over all disks and matching their sources. If any non-default arguments are found, def->os.bootloaderArgs is set accordingly, and the bootloader is treated as a custom bootloader. Custom bootloader are supported by setting the def->os.bootloader and def->os.bootloaderArgs accordingly grub-bhyve is also treated as a custom bootloader. Since we don't get the device map in the native format anyways, we can't reconstruct the complete boot order. While it is possible to check what type the grub boot disk is by checking if the --root argument is "cd" or "hd0,msdos1", and then just use the first disk found, implementing the grub-bhyve argument parser as-is in the grub-bhyve source would mean adding a dependency to argp or duplicating lots of the code of argp. Therefore it's not really worth implementing that now. Signed-off-by: Fabian Freyer <fabian.freyer@physik.tu-berlin.de> --- src/bhyve/bhyve_parse_command.c | 122 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index be4ff2a..6600111 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -712,6 +712,121 @@ error: return -1; } +/* + * Parse the /usr/sbin/bhyveload command line. + */ +static int +bhyveParseBhyveLoadCommandLine(virDomainDefPtr def, + int argc, char **argv) +{ + int c; + /* bhyveload called with default arguments when only -m and -d are given. + * Store this in a bit field and check if only those two options are given + * later */ + unsigned arguments = 0; + size_t memory = 0; + struct _getopt_data *parser; + int i = 0; + + const char optstr[] = "CSc:d:e:h:l:m:"; + + if (!argv) + goto error; + + if (VIR_ALLOC(parser) < 0) + goto error; + + while ((c = _getopt_internal_r(argc, argv, optstr, + NULL, NULL, 0, parser, 0)) != -1) { + switch (c) { + case 'd': + arguments |= 1; + /* Iterate over the disks of the domain trying to match up the + * source */ + for (i = 0; i < def->ndisks; i++) { + if (STREQ(virDomainDiskGetSource(def->disks[i]), + parser->optarg)) { + def->disks[i]->info.bootIndex = i; + break; + } + } + break; + case 'm': + arguments |= 2; + if (virStrToLong_ul(parser->optarg, NULL, 10, &memory) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse Memory.")); + goto error; + } + if (memory < 1024) + memory *= 1024; + else + memory /= 1024UL; + if (def->mem.cur_balloon != 0 && def->mem.cur_balloon != memory) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse Memory: Memory size mismatch.")); + goto error; + } + def->mem.cur_balloon = memory; + virDomainDefSetMemoryTotal(def, memory); + break; + default: + arguments |= 4; + } + } + + if (arguments != 3) { + /* Set os.bootloader since virDomainDefFormatInternal will only format + * the bootloader arguments if os->bootloader is set. */ + if (VIR_STRDUP(def->os.bootloader, argv[0]) < 0) + goto error; + + def->os.bootloaderArgs = virStringJoin((const char**) &argv[1], " "); + } + + if (argc != parser->optind) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse arguments for bhyveload command.")); + goto error; + } + + if (def->name == NULL) { + if (VIR_STRDUP(def->name, argv[argc]) < 0) + goto error; + } + else if (STRNEQ(def->name, argv[argc])) { + /* the vm name of the loader and the bhyverun command differ, throw an + * error here */ + virReportError(VIR_ERR_OPERATION_FAILED, + _("Failed to parse arguments: VM name mismatch.")); + goto error; + } + + VIR_FREE(parser); + return 0; +error: + VIR_FREE(parser); + return -1; +} + +static int +bhyveParseCustomLoaderCommandLine(virDomainDefPtr def, + int argc ATTRIBUTE_UNUSED, + char **argv) +{ + if (!argv) + goto error; + + if (VIR_STRDUP(def->os.bootloader, argv[0]) < 0) + goto error; + + def->os.bootloaderArgs = virStringJoin((const char**) &argv[1], " "); + + return 0; +error: + return -1; +} + virDomainDefPtr bhyveParseCommandLineString(const char* nativeConfig, unsigned caps, @@ -745,6 +860,13 @@ bhyveParseCommandLineString(const char* nativeConfig, if (bhyveParseBhyveCommandLine(def, xmlopt, caps, bhyve_argc, bhyve_argv)) goto cleanup; + if (STREQ(loader_argv[0], "/usr/sbin/bhyveload")) { + if (bhyveParseBhyveLoadCommandLine(def, loader_argc, loader_argv)) + goto cleanup; + } + else if (loader_argv) + if (bhyveParseCustomLoaderCommandLine(def, loader_argc, loader_argv)) + goto cleanup; cleanup: virStringFreeList(loader_argv); -- 2.7.0
participants (1)
-
Fabian Freyer