[libvirt] [PATCH] Make an error message in PCI util code clearer.

Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/util/pci.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index aa8344e..83ed461 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -540,7 +540,7 @@ pciTrySecondaryBusReset(pciDevice *dev, */ if (pciRead(dev, 0, config_space, PCI_CONF_LEN) < 0) { pciReportError(VIR_ERR_NO_SUPPORT, - _("Failed to save PCI config space for %s"), + _("Failed to read PCI config space for %s"), dev->name); goto out; } @@ -586,7 +586,7 @@ pciTryPowerManagementReset(pciDevice *dev) /* Save and restore the device's config space. */ if (pciRead(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) { pciReportError(VIR_ERR_NO_SUPPORT, - _("Failed to save PCI config space for %s"), + _("Failed to read PCI config space for %s"), dev->name); return -1; } -- 1.6.6

Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/conf/domain_conf.c | 6 +++--- src/conf/domain_conf.h | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4b5510b..c13973b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5533,9 +5533,9 @@ char *virDomainDefFormat(virDomainDefPtr def, return NULL; } -char *virDomainObjFormat(virCapsPtr caps, - virDomainObjPtr obj, - int flags) +static char *virDomainObjFormat(virCapsPtr caps, + virDomainObjPtr obj, + int flags) { char *config_xml = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d3a0775..3c672c6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -800,9 +800,6 @@ int virDomainDefAddDiskControllers(virDomainDefPtr def); #endif char *virDomainDefFormat(virDomainDefPtr def, int flags); -char *virDomainObjFormat(virCapsPtr caps, - virDomainObjPtr obj, - int flags); int virDomainCpuSetParse(const char **str, char sep, -- 1.6.6

On Thu, Feb 18, 2010 at 10:38:36AM -0500, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/conf/domain_conf.c | 6 +++--- src/conf/domain_conf.h | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4b5510b..c13973b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5533,9 +5533,9 @@ char *virDomainDefFormat(virDomainDefPtr def, return NULL; }
-char *virDomainObjFormat(virCapsPtr caps, - virDomainObjPtr obj, - int flags) +static char *virDomainObjFormat(virCapsPtr caps, + virDomainObjPtr obj, + int flags) { char *config_xml = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d3a0775..3c672c6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -800,9 +800,6 @@ int virDomainDefAddDiskControllers(virDomainDefPtr def); #endif char *virDomainDefFormat(virDomainDefPtr def, int flags); -char *virDomainObjFormat(virCapsPtr caps, - virDomainObjPtr obj, - int flags);
int virDomainCpuSetParse(const char **str, char sep,
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

If the hostname as returned by "gethostname" resolves to "localhost" (as it does with the broken Fedora-12 installer), then live migration will fail because the source will try to migrate to itself. Detect this situation up-front and abort the live migration before we do any real work. Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 2 +- src/util/util.c | 37 +++++++++++++++++++++++++++++++++++-- src/util/util.h | 1 + 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e3806cd..69ad686 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -568,6 +568,7 @@ virExecDaemonize; virSetCloseExec; virSetNonBlock; virFormatMacAddr; +virGetHostnameLocalhost; virGetHostname; virParseMacAddr; virFileDeletePid; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d8ec04..2123880 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7748,7 +7748,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0; /* Get hostname */ - if ((hostname = virGetHostname(dconn)) == NULL) + if ((hostname = virGetHostnameLocalhost(0)) == NULL) goto cleanup; /* XXX this really should have been a properly well-formed diff --git a/src/util/util.c b/src/util/util.c index cdab300..72cc222 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2193,11 +2193,11 @@ char *virIndexToDiskName(int idx, const char *prefix) #define AI_CANONIDN 0 #endif -char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) +char *virGetHostnameLocalhost(int allow_localhost) { int r; char hostname[HOST_NAME_MAX+1], *result; - struct addrinfo hints, *info; + struct addrinfo hints, *info, *res; r = gethostname (hostname, sizeof(hostname)); if (r == -1) { @@ -2217,6 +2217,34 @@ char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) hostname, gai_strerror(r)); return NULL; } + + /* if we aren't allowing localhost, then we iterate through the + * list and make sure none of the IPv4 addresses are 127.0.0.1 and + * that none of the IPv6 addresses are ::1 + */ + if (!allow_localhost) { + res = info; + while (res) { + if (res->ai_family == AF_INET) { + if (htonl(((struct sockaddr_in *)res->ai_addr)->sin_addr.s_addr) == INADDR_LOOPBACK) { + virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", + _("canonical hostname pointed to localhost, but this is not allowed")); + freeaddrinfo(info); + return NULL; + } + } + else if (res->ai_family == AF_INET6) { + if (IN6_IS_ADDR_LOOPBACK(&((struct sockaddr_in6 *)res->ai_addr)->sin6_addr)) { + virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", + _("canonical hostname pointed to localhost, but this is not allowed")); + freeaddrinfo(info); + return NULL; + } + } + res = res->ai_next; + } + } + if (info->ai_canonname == NULL) { virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not determine canonical host name")); @@ -2233,6 +2261,11 @@ char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) return result; } +char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return virGetHostnameLocalhost(1); +} + /* send signal to a single process */ int virKillProcess(pid_t pid, int sig) { diff --git a/src/util/util.h b/src/util/util.h index 4207508..d024fe1 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -232,6 +232,7 @@ static inline int getuid (void) { return 0; } static inline int getgid (void) { return 0; } #endif +char *virGetHostnameLocalhost(int allow_localhost); char *virGetHostname(virConnectPtr conn); int virKillProcess(pid_t pid, int sig); -- 1.6.6

On Thu, Feb 18, 2010 at 10:38:37AM -0500, Chris Lalancette wrote:
If the hostname as returned by "gethostname" resolves to "localhost" (as it does with the broken Fedora-12 installer), then live migration will fail because the source will try to migrate to itself. Detect this situation up-front and abort the live migration before we do any real work.
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 2 +- src/util/util.c | 37 +++++++++++++++++++++++++++++++++++-- src/util/util.h | 1 + 4 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e3806cd..69ad686 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -568,6 +568,7 @@ virExecDaemonize; virSetCloseExec; virSetNonBlock; virFormatMacAddr; +virGetHostnameLocalhost; virGetHostname; virParseMacAddr; virFileDeletePid; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d8ec04..2123880 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7748,7 +7748,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0;
/* Get hostname */ - if ((hostname = virGetHostname(dconn)) == NULL) + if ((hostname = virGetHostnameLocalhost(0)) == NULL) goto cleanup;
/* XXX this really should have been a properly well-formed diff --git a/src/util/util.c b/src/util/util.c index cdab300..72cc222 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2193,11 +2193,11 @@ char *virIndexToDiskName(int idx, const char *prefix) #define AI_CANONIDN 0 #endif
-char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) +char *virGetHostnameLocalhost(int allow_localhost) { int r; char hostname[HOST_NAME_MAX+1], *result; - struct addrinfo hints, *info; + struct addrinfo hints, *info, *res;
r = gethostname (hostname, sizeof(hostname)); if (r == -1) { @@ -2217,6 +2217,34 @@ char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) hostname, gai_strerror(r)); return NULL; } + + /* if we aren't allowing localhost, then we iterate through the + * list and make sure none of the IPv4 addresses are 127.0.0.1 and + * that none of the IPv6 addresses are ::1 + */ + if (!allow_localhost) { + res = info; + while (res) { + if (res->ai_family == AF_INET) { + if (htonl(((struct sockaddr_in *)res->ai_addr)->sin_addr.s_addr) == INADDR_LOOPBACK) { + virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", + _("canonical hostname pointed to localhost, but this is not allowed")); + freeaddrinfo(info); + return NULL; + } + } + else if (res->ai_family == AF_INET6) { + if (IN6_IS_ADDR_LOOPBACK(&((struct sockaddr_in6 *)res->ai_addr)->sin6_addr)) { + virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", + _("canonical hostname pointed to localhost, but this is not allowed")); + freeaddrinfo(info); + return NULL; + } + } + res = res->ai_next; + } + } + if (info->ai_canonname == NULL) { virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not determine canonical host name")); @@ -2233,6 +2261,11 @@ char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) return result; }
+char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return virGetHostnameLocalhost(1); +} + /* send signal to a single process */ int virKillProcess(pid_t pid, int sig) { diff --git a/src/util/util.h b/src/util/util.h index 4207508..d024fe1 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -232,6 +232,7 @@ static inline int getuid (void) { return 0; } static inline int getgid (void) { return 0; } #endif
+char *virGetHostnameLocalhost(int allow_localhost); char *virGetHostname(virConnectPtr conn);
int virKillProcess(pid_t pid, int sig);
Yes that looks fine, ACK thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Feb 18, 2010 at 10:38:35AM -0500, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com> --- src/util/pci.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c index aa8344e..83ed461 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -540,7 +540,7 @@ pciTrySecondaryBusReset(pciDevice *dev, */ if (pciRead(dev, 0, config_space, PCI_CONF_LEN) < 0) { pciReportError(VIR_ERR_NO_SUPPORT, - _("Failed to save PCI config space for %s"), + _("Failed to read PCI config space for %s"), dev->name); goto out; } @@ -586,7 +586,7 @@ pciTryPowerManagementReset(pciDevice *dev) /* Save and restore the device's config space. */ if (pciRead(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) { pciReportError(VIR_ERR_NO_SUPPORT, - _("Failed to save PCI config space for %s"), + _("Failed to read PCI config space for %s"), dev->name); return -1; }
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Feb 18, 2010 at 10:38:35AM -0500, Chris Lalancette wrote:
Signed-off-by: Chris Lalancette <clalance@redhat.com>
Okay, I pushed those 3 patches, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Chris Lalancette
-
Daniel Veillard