From: Alexander Nusov <alexander.nusov@xxxxxxxxxxxxxx> This patch adds support for automatic VNC port assignment for bhyve guests. --- Changes from v1: * Call virPortAllocatorRelease() in virBhyveProcessStop() to release VNC port; that's done unconditionally of using autoport * Call virPortAllocatorSetUsed(.., true) in virBhyveProcessReconnect() to reserve already used VNC ports after daemon restart * Call virPortAllocatorSetUsed(.., true) in bhyveBuildGraphicsArgStr() for domains that don't use autoport so allocator didn't try to use ports allocated by these domains * In dryRun mode (i.e. for domxml-to-native) don't allocate any ports * Add a couple of unit tests Note 1: while adding tests I noticed that port allocator will actually skip already bound ports, so I'm wondering if it makes any sense to use virPortAllocatorSetUsed(.., true)? Right now I cannot come up with any case to trigger this except probably some races when spawning guests simultaneously, but that's hard to reproduce. Note 2: there are still some cases where resources allocated during command preparation are not properly cleaned up; that's not only VNC ports, but also TAP devices. I plan to add proper cleanup routines separately. src/bhyve/bhyve_command.c | 25 +++++++++++-- src/bhyve/bhyve_driver.c | 5 +++ src/bhyve/bhyve_process.c | 20 +++++++++++ src/bhyve/bhyve_utils.h | 3 ++ .../bhyvexml2argv-vnc-autoport.args | 12 +++++++ .../bhyvexml2argv-vnc-autoport.ldargs | 1 + .../bhyvexml2argv-vnc-autoport.xml | 26 ++++++++++++++ tests/bhyvexml2argvtest.c | 7 ++++ .../bhyvexml2xmlout-vnc-autoport.xml | 41 ++++++++++++++++++++++ tests/bhyvexml2xmltest.c | 1 + 10 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index eae5cb3ca..e62b5df66 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -330,15 +330,19 @@ bhyveBuildLPCArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, } static int -bhyveBuildGraphicsArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, +bhyveBuildGraphicsArgStr(const virDomainDef *def, virDomainGraphicsDefPtr graphics, virDomainVideoDefPtr video, virConnectPtr conn, - virCommandPtr cmd) + virCommandPtr cmd, + bool dryRun) { virBuffer opt = VIR_BUFFER_INITIALIZER; virDomainGraphicsListenDefPtr glisten = NULL; bool escapeAddr; + unsigned short port; + + bhyveConnPtr driver = conn->privateData; if (!(bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM) || def->os.bootloader || @@ -401,6 +405,20 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, virBufferAdd(&opt, glisten->address, -1); } + if (!dryRun) { + if (graphics->data.vnc.autoport) { + if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) + return -1; + graphics->data.vnc.port = port; + } else { + if (virPortAllocatorSetUsed(driver->remotePorts, + graphics->data.vnc.port, + true) < 0) + VIR_WARN("Failed to mark VNC port '%d' as used by '%s'", + graphics->data.vnc.port, def->name); + } + } + virBufferAsprintf(&opt, ":%d", graphics->data.vnc.port); break; default: @@ -553,7 +571,8 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if (def->ngraphics && def->nvideos) { if (def->ngraphics == 1 && def->nvideos == 1) { - if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], conn, cmd) < 0) + if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], + conn, cmd, dryRun) < 0) goto error; add_lpc = true; } else { diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index ed2221a35..bffeea7d9 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -52,6 +52,7 @@ #include "viraccessapicheck.h" #include "virhostcpu.h" #include "virhostmem.h" +#include "virportallocator.h" #include "conf/domain_capabilities.h" #include "bhyve_conf.h" @@ -1219,6 +1220,7 @@ bhyveStateCleanup(void) virObjectUnref(bhyve_driver->closeCallbacks); virObjectUnref(bhyve_driver->domainEventState); virObjectUnref(bhyve_driver->config); + virObjectUnref(bhyve_driver->remotePorts); virMutexDestroy(&bhyve_driver->lock); VIR_FREE(bhyve_driver); @@ -1265,6 +1267,9 @@ bhyveStateInitialize(bool privileged, if (!(bhyve_driver->domainEventState = virObjectEventStateNew())) goto cleanup; + if (!(bhyve_driver->remotePorts = virPortAllocatorNew(_("display"), 5900, 65535, 0))) + goto cleanup; + bhyve_driver->hostsysinfo = virSysinfoRead(); if (!(bhyve_driver->config = virBhyveDriverConfigNew())) diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index a97e300ff..7211156ca 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -293,6 +293,16 @@ virBhyveProcessStop(bhyveConnPtr driver, /* Cleanup network interfaces */ bhyveNetCleanup(vm); + /* VNC autoport cleanup */ + if ((vm->def->ngraphics == 1) && + vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + if (virPortAllocatorRelease(driver->remotePorts, + vm->def->graphics[0]->data.vnc.port) < 0) { + VIR_WARN("Failed to release VNC port for '%s'", + vm->def->name); + } + } + ret = 0; virCloseCallbacksUnset(driver->closeCallbacks, vm, @@ -412,6 +422,16 @@ virBhyveProcessReconnect(virDomainObjPtr vm, if (STREQ(expected_proctitle, proc_argv[0])) { ret = 0; priv->mon = bhyveMonitorOpen(vm, data->driver); + if (vm->def->ngraphics == 1 && + vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + int vnc_port = vm->def->graphics[0]->data.vnc.port; + if (virPortAllocatorSetUsed(data->driver->remotePorts, + vnc_port, + true) < 0) { + VIR_WARN("Failed to mark VNC port '%d' as used by '%s'", + vnc_port, vm->def->name); + } + } } } diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h index db50e012a..8ad2698d4 100644 --- a/src/bhyve/bhyve_utils.h +++ b/src/bhyve/bhyve_utils.h @@ -28,6 +28,7 @@ # include "virdomainobjlist.h" # include "virthread.h" # include "virclosecallbacks.h" +# include "virportallocator.h" # define BHYVE_AUTOSTART_DIR SYSCONFDIR "/libvirt/bhyve/autostart" # define BHYVE_CONFIG_DIR SYSCONFDIR "/libvirt/bhyve" @@ -58,6 +59,8 @@ struct _bhyveConn { virCloseCallbacksPtr closeCallbacks; + virPortAllocatorPtr remotePorts; + unsigned bhyvecaps; unsigned grubcaps; }; diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args new file mode 100644 index 000000000..039526ff3 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.args @@ -0,0 +1,12 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 4:0,fbuf,tcp=127.0.0.1:5900 \ +-s 1,lpc bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.ldargs new file mode 100644 index 000000000..421376db9 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.ldargs @@ -0,0 +1 @@ +dummy diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.xml new file mode 100644 index 000000000..afb73f040 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc-autoport.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <loader readonly="yes" type="pflash">/path/to/test.fd</loader> + </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'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address' address='127.0.0.1'/> + </graphics> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index c8f8c685a..95fada0bd 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -145,6 +145,11 @@ mymain(void) if ((driver.xmlopt = virBhyveDriverCreateXMLConf(&driver)) == NULL) return EXIT_FAILURE; + if (!(driver.remotePorts = virPortAllocatorNew("display", 5900, 65535, + VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK))) + return EXIT_FAILURE; + + # define DO_TEST_FULL(name, flags) \ do { \ static struct testInfo info = { \ @@ -193,6 +198,7 @@ mymain(void) DO_TEST("net-e1000"); DO_TEST("uefi"); DO_TEST("vnc"); + DO_TEST("vnc-autoport"); /* Address allocation tests */ DO_TEST("addr-single-sata-disk"); @@ -231,6 +237,7 @@ mymain(void) virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); + virObjectUnref(driver.remotePorts); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml new file mode 100644 index 000000000..d6cfe76b7 --- /dev/null +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-autoport.xml @@ -0,0 +1,41 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>hvm</type> + <loader readonly='yes' type='pflash'>/path/to/test.fd</loader> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <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> + <controller type='pci' index='0' model='pci-root'/> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <interface type='bridge'> + <mac address='52:54:00:00:00:00'/> + <source bridge='virbr0'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='gop' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </video> + </devices> +</domain> diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index b3759919e..c16eb2b2c 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -105,6 +105,7 @@ mymain(void) DO_TEST_DIFFERENT("serial-grub"); DO_TEST_DIFFERENT("serial-grub-nocons"); DO_TEST_DIFFERENT("vnc"); + DO_TEST_DIFFERENT("vnc-autoport"); /* Address allocation tests */ DO_TEST_DIFFERENT("addr-single-sata-disk"); -- 2.13.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list