Hi Roman,
Sorry for the delay (I didn't setup a correct filter for the mailing list and missed your follow-up mail)

Updated patch looks awesome. thank you for your effort!

> 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.

Need to look at other drivers but I think it makes sense to use virPortAllocatorSetUsed (it's also thread-safe and has a lock)

> 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.

thanks.

--
alex




---- On Tue, 18 Jul 2017 14:03:23 +0300 Roman Bogorodskiy <bogorodskiy@gmail.com> wrote ----

Roman Bogorodskiy wrote:

> From: Alexander Nusov <alexander.nusov@nfvexpress.com>
>
> 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

ping?

Roman Bogorodskiy
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list