[libvirt] [PATCHv2] qemu: Redundant listen address entry in quest xml
When editing guest's XML (on QEMU), it was possible to add multiple listen elements into graphics parent element. However QEMU does not support listening on multiple addresses. Configuration is tested for multiple 'listen address' and if positive, an error is raised. --- src/qemu/qemu_process.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9e6a9ae..1810e6c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3934,6 +3934,14 @@ int qemuProcessStart(virConnectPtr conn, } graphics->listens[0].fromConfig = true; } + /* multiple listen addresses are unsupported configuration in qemu + */ + else if (graphics->nListens > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("QEMU does not support multiple listen " + "addresses for a domain.")); + goto cleanup; + } } } -- 1.9.3
On 08/14/2014 02:44 PM, Erik Skultety wrote:
When editing guest's XML (on QEMU), it was possible to add multiple listen elements into graphics parent element. However QEMU does not support listening on multiple addresses. Configuration is tested for multiple 'listen address' and if positive, an error is raised.
We have a public bug open for this issue. It's nice to add those in the commit message, if someone wants to know why the commit was added in the future. https://bugzilla.redhat.com/show_bug.cgi?id=1119212
--- src/qemu/qemu_process.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9e6a9ae..1810e6c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3934,6 +3934,14 @@ int qemuProcessStart(virConnectPtr conn, } graphics->listens[0].fromConfig = true; } + /* multiple listen addresses are unsupported configuration in qemu + */
This comment is redundant - it basically repeats the error message.
+ else if (graphics->nListens > 1) {
We prefer putting 'else' on the same line as the closing brace of the 'if' block: http://libvirt.org/hacking.html#curly_braces
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("QEMU does not support multiple listen " + "addresses for a domain."));
This is not true. For example: you can use one VNC and one SPICE graphics, both with different listen addresses. How about 'QEMU does not support multiple listen addresses for one graphics device'? Jan
On 2014/8/14 20:44, Erik Skultety wrote:
When editing guest's XML (on QEMU), it was possible to add multiple listen elements into graphics parent element. However QEMU does not support listening on multiple addresses. Configuration is tested for multiple 'listen address' and if positive, an error is raised. --- src/qemu/qemu_process.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9e6a9ae..1810e6c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3934,6 +3934,14 @@ int qemuProcessStart(virConnectPtr conn, } graphics->listens[0].fromConfig = true; } + /* multiple listen addresses are unsupported configuration in qemu + */ + else if (graphics->nListens > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("QEMU does not support multiple listen " + "addresses for a domain.")); + goto cleanup; + } } }
Think about the libvirt updating scenario. VMs with such XMLs are already defined and running. After the libvirt updating to a new version which this patch works, the VMs can't be started after shutdown. I have a question to confirm. Is this a regression or taken as an improvement ? (I'm inclined to accept it.)
On Fri, Aug 15, 2014 at 05:59:38PM +0800, Wang Rui wrote:
On 2014/8/14 20:44, Erik Skultety wrote:
When editing guest's XML (on QEMU), it was possible to add multiple listen elements into graphics parent element. However QEMU does not support listening on multiple addresses. Configuration is tested for multiple 'listen address' and if positive, an error is raised. --- src/qemu/qemu_process.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9e6a9ae..1810e6c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3934,6 +3934,14 @@ int qemuProcessStart(virConnectPtr conn, } graphics->listens[0].fromConfig = true; } + /* multiple listen addresses are unsupported configuration in qemu + */ + else if (graphics->nListens > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("QEMU does not support multiple listen " + "addresses for a domain.")); + goto cleanup; + } } }
Think about the libvirt updating scenario.
VMs with such XMLs are already defined and running. After the libvirt updating to a new version which this patch works, the VMs can't be started after shutdown.
I have a question to confirm. Is this a regression or taken as an improvement ? (I'm inclined to accept it.)
Such VMs already had an incorrect configuration, so refusing to start them after upgrade is acceptable policy. We frequently make our error checking stricter with new versions. Regards, 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 (4)
-
Daniel P. Berrange -
Erik Skultety -
Ján Tomko -
Wang Rui