[libvirt] [PATCHv2] qemu: Do fake auto-allocation of ports when generating native command
When attempting to generate the native command line from an XML file that uses graphics port auto allocation, the generated commandline wouldn't be valid. This patch adds fake autoallocation of ports as done when starting the actual machine. --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d3b24a..068d6c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5271,6 +5271,58 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, if (qemuAssignDeviceAliases(def, qemuCaps) < 0) goto cleanup; + /* do fake auto-alloc of graphics ports, if such config is used */ + for (i = 0 ; i < def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.socket && graphics->data.vnc.autoport) { + graphics->data.vnc.port = 5900; + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + int j; + bool needTLSPort = false; + bool needPort = false; + int defaultMode = graphics->data.spice.defaultMode; + + if (graphics->data.spice.autoport) { + /* check if tlsPort or port need allocation */ + for (j = 0 ; j < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; j++) { + switch (graphics->data.spice.channels[j]) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + needTLSPort = true; + break; + + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + needPort = true; + break; + + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + switch (defaultMode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + needTLSPort = true; + break; + + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + needPort = true; + break; + + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + needTLSPort = true; + needPort = true; + break; + } + break; + } + } + } + + if (needPort || graphics->data.spice.port == -1) + graphics->data.spice.port = 5901; + + if (needTLSPort || graphics->data.spice.tlsPort == -1) + graphics->data.spice.tlsPort = 5902; + } + } + if (!(cmd = qemuBuildCommandLine(conn, driver, def, &monConfig, monitor_json, qemuCaps, NULL, -1, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) -- 1.8.2.1
On 29.04.2013 14:42, Peter Krempa wrote:
When attempting to generate the native command line from an XML file that uses graphics port auto allocation, the generated commandline wouldn't be valid.
This patch adds fake autoallocation of ports as done when starting the actual machine. --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d3b24a..068d6c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5271,6 +5271,58 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, ... + + if (needPort || graphics->data.spice.port == -1) + graphics->data.spice.port = 5901; + + if (needTLSPort || graphics->data.spice.tlsPort == -1) + graphics->data.spice.tlsPort = 5902;
Shouldn't we be using virPortAllocator to set these ports instead of hardcoding these values? What if these ports are already taken (by previously started domain for instance)? Michal
On 04/29/13 14:48, Michal Privoznik wrote:
On 29.04.2013 14:42, Peter Krempa wrote:
When attempting to generate the native command line from an XML file that uses graphics port auto allocation, the generated commandline wouldn't be valid.
This patch adds fake autoallocation of ports as done when starting the actual machine. --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d3b24a..068d6c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5271,6 +5271,58 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, ... + + if (needPort || graphics->data.spice.port == -1) + graphics->data.spice.port = 5901; + + if (needTLSPort || graphics->data.spice.tlsPort == -1) + graphics->data.spice.tlsPort = 5902;
Shouldn't we be using virPortAllocator to set these ports instead of hardcoding these values? What if these ports are already taken (by previously started domain for instance)?
This is in domxml-to-native. I don't think we care if the ports are taken when that API is used. (I'm not even sure if we care enough to work around auto allocation there).
Michal
Peter
On Mon, Apr 29, 2013 at 02:48:54PM +0200, Michal Privoznik wrote:
On 29.04.2013 14:42, Peter Krempa wrote:
When attempting to generate the native command line from an XML file that uses graphics port auto allocation, the generated commandline wouldn't be valid.
This patch adds fake autoallocation of ports as done when starting the actual machine. --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d3b24a..068d6c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5271,6 +5271,58 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, ... + + if (needPort || graphics->data.spice.port == -1) + graphics->data.spice.port = 5901; + + if (needTLSPort || graphics->data.spice.tlsPort == -1) + graphics->data.spice.tlsPort = 5902;
Shouldn't we be using virPortAllocator to set these ports instead of hardcoding these values? What if these ports are already taken (by previously started domain for instance)?
Ensuring the ports are not in use, is outside the scope of the XMLToNative API, so I think hardcoding them is correct. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Michal Privoznik -
Peter Krempa