[libvirt] [PATCH] Add support for VNC autoport feature for bhyve hypervisor.

[Alexander Nusov]

---- On Wed, 10 May 2017 17:58:05 +0300 Roman Bogorodskiy <bogorodskiy at gmail.com> wrote ----



Alexander Nusov wrote: 



> This patch adds support for automatic VNC port assignment for bhyve guests. 

> 

> --- 

> src/bhyve/bhyve_command.c | 9 +++++++++ 

> src/bhyve/bhyve_driver.c | 5 +++++ 

> src/bhyve/bhyve_utils.h | 3 +++ 

> 3 files changed, 17 insertions(+) 



Hi Alexander, 



Thanks for implementing this! Overall it looks good, two comments: 



* It doesn't take into account domains that use VNC with port 

explicitly specified. For example, if I start a domain with VNC 

configuration "port=5900 autoport=no", I will not be able 

to start a domain with "autoport=yes" because it will try to 

to 5900 and will fail to bind. 



Looking at the qemu driver code, it seems it's done by calling 

virPortAllocatorSetUsed(). 





Hi Roman. Thanks for finding this. I tried adding virPortAllocatorSetUsed() call but it seems it should be 

placed somewhere at the initialization of libvirtd driver also in case of restaring the libvirtd daemon.





I'm also wondering how it's handled for autostarting domains, e.g. 

if I have vm1[5900], vm2[5901], vm3[autoport], vm4[autoport], 

they'll start fine in that order, but will fail to start if going 

this way: vm3, vm4, vm1, vm2 





To be honest, I'm concerned also.



https://www.redhat.com/archives/libvir-list/2011-April/msg00819.html

is that still actual?





* Nitpick: Commit message titles are usually don't end with a period. 


Also, they're usually prefixed with the relevant subsystem, so 

this one could be "bhyve: Add support for VNC autoport feature" 

for example (you might glance through 'git log' for examples.




good.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170512/3ce4f626/attachment-0001.htm>