<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"><html><head><meta content="text/html;charset=UTF-8" http-equiv="Content-Type"></head><body ><div style='font-size:10pt;font-family:Verdana,Arial,Helvetica,sans-serif;'><div>---- On Wed, 10 May 2017 17:58:05 +0300 <b>Roman Bogorodskiy <<a href="mailto:bogorodskiy@gmail.com">bogorodskiy@gmail.com</a>></b> wrote ----<br></div><div class="zmail_extra"><div><br></div><blockquote style="border-left: 1px solid #cccccc; padding-left: 6px; margin:0 0 0 5px"><div><div>Alexander Nusov wrote: <br></div><div><br></div><div>> This patch adds support for automatic VNC port assignment for bhyve guests. <br></div><div>> <br></div><div>> --- <br></div><div>> src/bhyve/bhyve_command.c | 9 +++++++++ <br></div><div>> src/bhyve/bhyve_driver.c | 5 +++++ <br></div><div>> src/bhyve/bhyve_utils.h | 3 +++ <br></div><div>> 3 files changed, 17 insertions(+) <br></div><div><br></div><div>Hi Alexander, <br></div><div><br></div><div>Thanks for implementing this! Overall it looks good, two comments: <br></div><div><br></div><div>* It doesn't take into account domains that use VNC with port <br></div><div>explicitly specified. For example, if I start a domain with VNC <br></div><div>configuration "port=5900 autoport=no", I will not be able <br></div><div>to start a domain with "autoport=yes" because it will try to <br></div><div>to 5900 and will fail to bind. <br></div><div><br></div><div>Looking at the qemu driver code, it seems it's done by calling <br></div><div>virPortAllocatorSetUsed(). <br></div></div></blockquote></div><div><br></div><div>Hi Roman. Thanks for finding this. I tried adding virPortAllocatorSetUsed() call but it seems it should be <br></div><div>placed somewhere at the initialization of libvirtd driver also in case of restaring the libvirtd daemon.<br></div><div><br></div><div class="zmail_extra"><blockquote style="border-left: 1px solid #cccccc; padding-left: 6px; margin:0 0 0 5px"><div><div><br></div><div>I'm also wondering how it's handled for autostarting domains, e.g. <br></div><div>if I have vm1[5900], vm2[5901], vm3[autoport], vm4[autoport], <br></div><div>they'll start fine in that order, but will fail to start if going <br></div><div>this way: vm3, vm4, vm1, vm2 <br></div></div></blockquote></div><div><br></div><div>To be honest, I'm concerned also.<br></div><div><br></div><div><a href="https://www.redhat.com/archives/libvir-list/2011-April/msg00819.html">https://www.redhat.com/archives/libvir-list/2011-April/msg00819.html</a><br></div><div>is that still actual?<br></div><div><br></div><div><br></div><div class="zmail_extra"><blockquote style="border-left: 1px solid #cccccc; padding-left: 6px; margin:0 0 0 5px"><div><div>* Nitpick: Commit message titles are usually don't end with a period. <br></div></div><div>Also, they're usually prefixed with the relevant subsystem, so <br></div><div>this one could be "bhyve: Add support for VNC autoport feature" <br></div><div>for example (you might glance through 'git log' for examples.<br></div></blockquote></div><div><br></div><div>good.<br></div><div><br></div></div></body></html>