On Thu, Sep 24, 2015 at 15:27:43 +0200, Peter Krempa wrote:
On Wed, Sep 23, 2015 at 22:48:14 +0200, Jiri Denemark wrote:
As of commit 6992994, we set graphics/@listen attribute according to the first listen child element even if that element is of type='network'. This was done for backward compatibility with applications which only support the original listen attribute. However, by doing so we broke migration to older libvirt which tried to check that the listen attribute matches one of the listen child elements but which did not take type='network' elements into account.
We are not concerned about compatibility with old applications when formatting domain XML for migration for two reasons. The XML is consumed only by libvirtd and the IP address associated with type='network' listen address on the source host is just useless on the destination host. Thus, we can safely avoid propagating the type='network' IP address to graphics/@listen attribute when creating migratable XML.
https://bugzilla.redhat.com/show_bug.cgi?id=1265111
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c890977..033ae46 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21020,19 +21020,17 @@ virDomainGraphicsDefFormat(virBufferPtr buf, * <graphics>. This is done to improve backward compatibility. */ for (i = 0; i < def->nListens; i++) { - virDomainGraphicsListenType listenType; - if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && def->listens[i].fromConfig) continue; - listenType = virDomainGraphicsListenGetType(def, i);
- if (listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS || - (listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK && - !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) { - if ((listenAddr = virDomainGraphicsListenGetAddress(def, i))) - break; - } + if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK && + flags & (VIR_DOMAIN_DEF_FORMAT_INACTIVE | + VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))
So basically the above line is the only functional change in the whole refactor.
+ continue; + + if ((listenAddr = virDomainGraphicsListenGetAddress(def, i))) + break; }
virBufferAsprintf(buf, "<graphics type='%s'", type);
ACK, but the change wasn't easy to interpret correctly.
Yeah, sorry about it. The condition was not exactly easy to grasp and I didn't want to make it even worse. So the change itself is a bit hidden but I think the result is much more clear about what is happening here. Pushed, Jirka