Recently, bhyve started supporting specifying guest CPU topology. It looks this way: bhyve -c cpus=C,sockets=S,cores=C,threads=T ... The old behaviour with bhyve -c C, where C is a number of vCPUs, is still supported. So if we have CPU topology in the domain XML, use the new syntax, otherwise keeps the old behaviour. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; --- src/bhyve/bhyve_capabilities.c | 7 +++-- src/bhyve/bhyve_capabilities.h | 1 + src/bhyve/bhyve_command.c | 26 ++++++++++++++++++- ...yvexml2argv-cputopology-nvcpu-mismatch.xml | 26 +++++++++++++++++++ .../bhyvexml2argv-cputopology.args | 9 +++++++ .../bhyvexml2argv-cputopology.ldargs | 3 +++ .../bhyvexml2argv-cputopology.xml | 26 +++++++++++++++++++ tests/bhyvexml2argvtest.c | 8 +++++- 8 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-cputopology-nvcpu-mismatch.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.xml diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index e13085b1d5..a3229cea75 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -227,7 +227,7 @@ bhyveProbeCapsDeviceHelper(unsigned int *caps, } static int -bhyveProbeCapsRTC_UTC(unsigned int *caps, char *binary) +bhyveProbeCapsFromHelp(unsigned int *caps, char *binary) { char *help; virCommandPtr cmd = NULL; @@ -244,6 +244,9 @@ bhyveProbeCapsRTC_UTC(unsigned int *caps, char *binary) if (strstr(help, "-u:") != NULL) *caps |= BHYVE_CAP_RTC_UTC; + if (strstr(help, "sockets=n][,cores=n][,threads=n") != NULL) + *caps |= BHYVE_CAP_CPUTOPOLOGY; + out: VIR_FREE(help); virCommandFree(cmd); @@ -314,7 +317,7 @@ virBhyveProbeCaps(unsigned int *caps) if (binary == NULL) goto out; - if ((ret = bhyveProbeCapsRTC_UTC(caps, binary))) + if ((ret = bhyveProbeCapsFromHelp(caps, binary))) goto out; if ((ret = bhyveProbeCapsAHCI32Slot(caps, binary))) diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 0e310e6eda..873bc9c12d 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -49,6 +49,7 @@ typedef enum { BHYVE_CAP_LPC_BOOTROM = 1 << 3, BHYVE_CAP_FBUF = 1 << 4, BHYVE_CAP_XHCI = 1 << 5, + BHYVE_CAP_CPUTOPOLOGY = 1 << 6, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index e3f7ded7db..802997bd2d 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -462,12 +462,36 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, size_t i; bool add_lpc = false; int nusbcontrollers = 0; + unsigned int nvcpus = virDomainDefGetVcpus(def); virCommandPtr cmd = virCommandNew(BHYVE); /* CPUs */ virCommandAddArg(cmd, "-c"); - virCommandAddArgFormat(cmd, "%d", virDomainDefGetVcpus(def)); + if (def->cpu && def->cpu->sockets) { + if (nvcpus != def->cpu->sockets * def->cpu->cores * def->cpu->threads) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid CPU topology: total number of vCPUs " + "must equal the product of sockets, cores, " + "and threads")); + goto error; + } + + if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_CPUTOPOLOGY) != 0) { + virCommandAddArgFormat(cmd, "cpus=%d,sockets=%d,cores=%d,threads=%d", + nvcpus, + def->cpu->sockets, + def->cpu->cores, + def->cpu->threads); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Installed bhyve binary does not support " + "defining CPU topology")); + goto error; + } + } else { + virCommandAddArgFormat(cmd, "%d", nvcpus); + } /* Memory */ virCommandAddArg(cmd, "-m"); diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology-nvcpu-mismatch.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology-nvcpu-mismatch.xml new file mode 100644 index 0000000000..5bd05fb7da --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology-nvcpu-mismatch.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>4</vcpu> + <cpu> + <topology sockets='1' cores='2' threads='1'/> + </cpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args new file mode 100644 index 0000000000..2d175a4178 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.args @@ -0,0 +1,9 @@ +/usr/sbin/bhyve \ +-c cpus=2,sockets=1,cores=2,threads=1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs new file mode 100644 index 0000000000..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.xml new file mode 100644 index 0000000000..83c7d423c4 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-cputopology.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>2</vcpu> + <cpu> + <topology sockets='1' cores='2' threads='1'/> + </cpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <mac address='52:54:00:b9:94:02'/> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index b08b1675f3..6d5f19e2c6 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -176,7 +176,8 @@ mymain(void) driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV; driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_AHCI32SLOT | \ BHYVE_CAP_NET_E1000 | BHYVE_CAP_LPC_BOOTROM | \ - BHYVE_CAP_FBUF | BHYVE_CAP_XHCI; + BHYVE_CAP_FBUF | BHYVE_CAP_XHCI | \ + BHYVE_CAP_CPUTOPOLOGY; DO_TEST("base"); DO_TEST("wired"); @@ -207,6 +208,8 @@ mymain(void) DO_TEST("vnc-vgaconf-off"); DO_TEST("vnc-vgaconf-io"); DO_TEST("vnc-autoport"); + DO_TEST("cputopology"); + DO_TEST_FAILURE("cputopology-nvcpu-mismatch"); /* Address allocation tests */ DO_TEST("addr-single-sata-disk"); @@ -243,6 +246,9 @@ mymain(void) driver.bhyvecaps &= ~BHYVE_CAP_FBUF; DO_TEST_FAILURE("vnc"); + driver.bhyvecaps &= ~BHYVE_CAP_CPUTOPOLOGY; + DO_TEST_FAILURE("cputopology"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); virPortAllocatorRangeFree(driver.remotePorts); -- 2.17.0

John Ferlan wrote: Yeah, there's no i18n for bhyve. What is the issue with the long exact string? I guess it's worse performance-wise, but probably not that critical compared to running the bhyve binary... OTOH, checking it this way seems to be more straightforward. Roman Bogorodskiy

John Ferlan wrote: Yes, I guess your suggestion to check absence "-c vcpus" is less error prone for small formatting changes / extensions of the '-c' switch. I've changed this check accordingly, and re-rolled commits to look like 1. function rename 2. topology support + driver page update 3. news update. I plan to push it today without sending out v3. Thanks Roman Bogorodskiy

On 05/29/2018 12:57 PM, Roman Bogorodskiy wrote: This should be split to keep the news.xml in its own patch. You could move the drvbhyve.html.in to patch 1 4.5.0 This one will obviously require a merge with the current top - just be careful to ensure it's in the 4.5.0 section and that syntax-check passes - news.xml can be onery ;-). Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John