[libvirt] [PATCH 0/2] fix migration related graphics listen code bugs
Pavel Hrdina (2): conf: store "autoGenerated" for graphics listen in status XML conf: properly skip graphics listen element in migratable XML src/conf/domain_conf.c | 18 +++++++++++++++++- src/qemu/qemu_domain.c | 3 ++- 2 files changed, 19 insertions(+), 2 deletions(-) -- 2.11.1
When libvirtd is started we call qemuDomainRecheckInternalPaths to detect whether a domain has VNC socket path generated by libvirt based on option from qemu.conf. However if we are parsing status XML for running domain the existing socket path can be generated also if the config XML uses the new <listen type='socket'/> element without specifying any socket. The current code doesn't make difference how the socket was generated and always marks it as "fromConfig". We need to store the "autoGenerated" value in the status XML in order to preserve that information. The difference between "fromConfig" and "autoGenerated" is important for migration, because if the socket is based on "fromConfig" we don't print it into the migratable XML and we assume that user has properly configured qemu.conf on both hosts. However if the socket is based on "autoGenerated" it means that a new feature was used and therefore we need to leave the socket in migratable XML to make sure that if this feature is not supported on destination the migration will fail. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++++++- src/qemu/qemu_domain.c | 3 ++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 97d42fe993..db1890f8a7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11543,6 +11543,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, char *network = virXMLPropString(node, "network"); char *socketPath = virXMLPropString(node, "socket"); char *fromConfig = virXMLPropString(node, "fromConfig"); + char *autoGenerated = virXMLPropString(node, "autoGenerated"); char *addressCompat = NULL; char *socketCompat = NULL; const char *graphicsType = virDomainGraphicsTypeToString(graphics->type); @@ -11662,6 +11663,17 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, def->fromConfig = tmp != 0; } + if (autoGenerated && + flags & VIR_DOMAIN_DEF_PARSE_STATUS) { + if (virStrToLong_i(autoGenerated, NULL, 10, &tmp) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid autoGenerated value: %s"), + autoGenerated); + goto error; + } + def->autoGenerated = tmp != 0; + } + ret = 0; error: if (ret < 0) @@ -11671,6 +11683,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, VIR_FREE(network); VIR_FREE(socketPath); VIR_FREE(fromConfig); + VIR_FREE(autoGenerated); VIR_FREE(addressCompat); VIR_FREE(socketCompat); return ret; @@ -22894,8 +22907,10 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " socket='%s'", def->socket); } - if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) { virBufferAsprintf(buf, " fromConfig='%d'", def->fromConfig); + virBufferAsprintf(buf, " autoGenerated='%d'", def->autoGenerated); + } virBufferAddLit(buf, "/>\n"); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c187214dc3..2bd6affe33 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2541,11 +2541,12 @@ qemuDomainRecheckInternalPaths(virDomainDefPtr def, /* This will happen only if we parse XML from old libvirts where * unix socket was available only for VNC graphics. In this * particular case we should follow the behavior and if we remove - * the auto-generated socket base on config option from qemu.conf + * the auto-generated socket based on config option from qemu.conf * we need to change the listen type to address. */ if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && glisten->socket && + !glisten->autoGenerated && STRPREFIX(glisten->socket, cfg->libDir)) { if (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) { VIR_FREE(glisten->socket); -- 2.11.1
On Mon, Feb 27, 2017 at 17:31:05 +0100, Pavel Hrdina wrote:
When libvirtd is started we call qemuDomainRecheckInternalPaths to detect whether a domain has VNC socket path generated by libvirt based on option from qemu.conf. However if we are parsing status XML for running domain the existing socket path can be generated also if the config XML uses the new <listen type='socket'/> element without specifying any socket.
The current code doesn't make difference how the socket was generated and always marks it as "fromConfig". We need to store the "autoGenerated" value in the status XML in order to preserve that information.
The difference between "fromConfig" and "autoGenerated" is important for migration, because if the socket is based on "fromConfig" we don't print it into the migratable XML and we assume that user has properly configured qemu.conf on both hosts. However if the socket is based on "autoGenerated" it means that a new feature was used and therefore we need to leave the socket in migratable XML to make sure that if this feature is not supported on destination the migration will fail.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++++++- src/qemu/qemu_domain.c | 3 ++- 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 97d42fe993..db1890f8a7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11543,6 +11543,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, char *network = virXMLPropString(node, "network"); char *socketPath = virXMLPropString(node, "socket"); char *fromConfig = virXMLPropString(node, "fromConfig"); + char *autoGenerated = virXMLPropString(node, "autoGenerated"); char *addressCompat = NULL; char *socketCompat = NULL; const char *graphicsType = virDomainGraphicsTypeToString(graphics->type); @@ -11662,6 +11663,17 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, def->fromConfig = tmp != 0; }
+ if (autoGenerated && + flags & VIR_DOMAIN_DEF_PARSE_STATUS) { + if (virStrToLong_i(autoGenerated, NULL, 10, &tmp) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid autoGenerated value: %s"), + autoGenerated); + goto error; + } + def->autoGenerated = tmp != 0; + } +
autoGenerated is bool and should be stored as such in the XML. I don't think we need to copy the bad design from the past even if the result is going to be inconsistent since this only affects our status XML. Jirka
We should skip <listen type='socket'/> only if the 'socket' path is specified because if there is no 'socket' path we need to keep that element in migratable XML. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1366088 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db1890f8a7..4c72cf0573 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23132,6 +23132,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, * thus we can generate it in the migrateble XML. */ if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET && + def->listens[i].socket && !def->listens[i].autoGenerated) continue; -- 2.11.1
participants (2)
-
Jiri Denemark -
Pavel Hrdina