Improve error reporting for virConnectGetHostname calls

All drivers have copy + pasted inadequate error reporting which wraps
util.c:virGetHostname. Move all error reporting to this function, and improve
what we report.

Changes from v1:
  Drop the driver wrappers around virGetHostname. This means we still need
  to keep the new conn argument to virGetHostname, but I think it's worth
  it.

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index b5a62cce324995edf4539b554d3621eb41796c7e..daf06bc0bad56825ace622cb6632b981a0f4841f 100644 (file)
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -972,11 +972,10 @@ static int qemudNetworkInit(struct qemud_server *server) {
         if (!mdns_name) {
             char groupname[64], *localhost, *tmp;
             /* Extract the host part of the potentially FQDN */
-            localhost = virGetHostname();
-            if (localhost == NULL) {
-                virReportOOMError(NULL);
+            localhost = virGetHostname(NULL);
+            if (localhost == NULL)
                 goto cleanup;
-            }
+
             if ((tmp = strchr(localhost, '.')))
                 *tmp = '\0';
             snprintf(groupname, sizeof(groupname)-1, "Virtualization Host %s", localhost);

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 1e984c5aa82d6c24dec4235500bbbd94f0d445e7..9ab94bf221107df56711d986c752aff0651b2461 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2030,20 +2030,6 @@ cleanup:
     return ret;
 }
 
-static char *lxcGetHostname (virConnectPtr conn)
-{
-    char *result;
-
-    result = virGetHostname();
-    if (result == NULL) {
-        virReportSystemError (conn, errno,
-                              "%s", _("failed to determine host name"));
-        return NULL;
-    }
-    /* Caller frees this string. */
-    return result;
-}
-
 static int lxcFreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm)
 {
     int timeout = 1000; /* In milliseconds */
@@ -2261,7 +2247,7 @@ static virDriver lxcDriver = {
     NULL, /* supports_feature */
     NULL, /* type */
     lxcVersion, /* version */
-    lxcGetHostname, /* getHostname */
+    virGetHostname, /* getHostname */
     NULL, /* getMaxVcpus */
     nodeGetInfo, /* nodeGetInfo */
     lxcGetCapabilities, /* getCapabilities */

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b8b5502f2c90f6f0cfccb8c981616129849b9b6..344308251f012dba87678591e68c38f50ec16e7c 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2600,21 +2600,6 @@ cleanup:
     return ret;
 }
 
-static char *
-qemudGetHostname (virConnectPtr conn)
-{
-    char *result;
-
-    result = virGetHostname();
-    if (result == NULL) {
-        virReportSystemError (conn, errno,
-                              "%s", _("failed to determine host name"));
-        return NULL;
-    }
-    /* Caller frees this string. */
-    return result;
-}
-
 static int qemudListDomains(virConnectPtr conn, int *ids, int nids) {
     struct qemud_driver *driver = conn->privateData;
     int n;
@@ -6238,11 +6223,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
         if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0;
 
         /* Get hostname */
-        if ((hostname = virGetHostname()) == NULL) {
-            virReportSystemError (dconn, errno,
-                                  "%s", _("failed to determine host name"));
+        if ((hostname = virGetHostname(dconn)) == NULL)
             goto cleanup;
-        }
 
         /* XXX this really should have been a properly well-formed
          * URI, but we can't add in tcp:// now without breaking
@@ -7074,7 +7056,7 @@ static virDriver qemuDriver = {
     qemudSupportsFeature, /* supports_feature */
     qemudGetType, /* type */
     qemudGetVersion, /* version */
-    qemudGetHostname, /* getHostname */
+    virGetHostname, /* getHostname */
     qemudGetMaxVCPUs, /* getMaxVcpus */
     nodeGetInfo, /* nodeGetInfo */
     qemudGetCapabilities, /* getCapabilities */

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 7e190722c1a4bcdfe172ccffbe78e7b80dcc3880..31b5ad3e27f538b49713361e38cfaac5551396d5 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1023,20 +1023,6 @@ static int testGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED,
     return (0);
 }
 
-static char *testGetHostname (virConnectPtr conn)
-{
-    char *result;
-
-    result = virGetHostname();
-    if (result == NULL) {
-        virReportSystemError(conn, errno,
-                             "%s", _("cannot lookup hostname"));
-        return NULL;
-    }
-    /* Caller frees this string. */
-    return result;
-}
-
 static int testGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED,
                            const char *type ATTRIBUTE_UNUSED)
 {
@@ -4674,7 +4660,7 @@ static virDriver testDriver = {
     NULL, /* supports_feature */
     NULL, /* type */
     testGetVersion, /* version */
-    testGetHostname, /* getHostname */
+    virGetHostname, /* getHostname */
     testGetMaxVCPUs, /* getMaxVcpus */
     testNodeGetInfo, /* nodeGetInfo */
     testGetCapabilities, /* getCapabilities */

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 5c6dbff9a77d9ce5ce4f029776f03315d8b96844..212cd8fe10a64b7c4600a401367f165a8c7750d7 100644 (file)
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1146,21 +1146,6 @@ cleanup:
     return ret;
 }
 
-static char *
-umlGetHostname (virConnectPtr conn)
-{
-    char *result;
-
-    result = virGetHostname();
-    if (result == NULL) {
-        virReportSystemError(conn, errno,
-                             "%s", _("cannot lookup hostname"));
-        return NULL;
-    }
-    /* Caller frees this string. */
-    return result;
-}
-
 static int umlListDomains(virConnectPtr conn, int *ids, int nids) {
     struct uml_driver *driver = conn->privateData;
     int n;
@@ -1790,7 +1775,7 @@ static virDriver umlDriver = {
     NULL, /* supports_feature */
     umlGetType, /* type */
     umlGetVersion, /* version */
-    umlGetHostname, /* getHostname */
+    virGetHostname, /* getHostname */
     NULL, /* getMaxVcpus */
     nodeGetInfo, /* nodeGetInfo */
     umlGetCapabilities, /* getCapabilities */

diff --git a/src/util/util.c b/src/util/util.c
index 08070da3738373f940571c6cb25641a913e6585c..853d3a0a942cd85ddca1c959831c87e4f704d762 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1805,30 +1805,42 @@ int virDiskNameToIndex(const char *name) {
 #define AI_CANONIDN 0
 #endif
 
-char *virGetHostname(void)
+char *virGetHostname(virConnectPtr conn)
 {
     int r;
     char hostname[HOST_NAME_MAX+1], *result;
     struct addrinfo hints, *info;
 
     r = gethostname (hostname, sizeof(hostname));
-    if (r == -1)
+    if (r == -1) {
+        virReportSystemError (conn, errno,
+                              "%s", _("failed to determine host name"));
         return NULL;
+    }
     NUL_TERMINATE(hostname);
 
     memset(&hints, 0, sizeof(hints));
     hints.ai_flags = AI_CANONNAME|AI_CANONIDN;
     hints.ai_family = AF_UNSPEC;
     r = getaddrinfo(hostname, NULL, &hints, &info);
-    if (r != 0)
+    if (r != 0) {
+        ReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                    _("getaddrinfo failed for '%s': %s"),
+                    hostname, gai_strerror(r));
         return NULL;
+    }
     if (info->ai_canonname == NULL) {
+        ReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                    "%s", _("could not determine canonical host name"));
         freeaddrinfo(info);
         return NULL;
     }
 
     /* Caller frees this string. */
     result = strdup (info->ai_canonname);
+    if (!result)
+        virReportOOMError(conn);
+
     freeaddrinfo(info);
     return result;
 }

diff --git a/src/util/util.h b/src/util/util.h
index 3ef26e63f4d60fb90eb8e8e3d9c80e7565b2221f..f4e395ec4e7b438d900960b522aa1bdb7cc1e143 100644 (file)
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -222,7 +222,7 @@ static inline int getuid (void) { return 0; }
 static inline int getgid (void) { return 0; }
 #endif
 
-char *virGetHostname(void);
+char *virGetHostname(virConnectPtr conn);
 
 int virKillProcess(pid_t pid, int sig);
 

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index aecda23bbb512e6a4027b0bdc269abd1853987f7..c6305aca68d3cf86f77886eb63d529262547c3b7 100644 (file)
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -591,20 +591,6 @@ static int vboxGetVersion(virConnectPtr conn, unsigned long *version) {
     return 0;
 }
 
-static char *vboxGetHostname(virConnectPtr conn) {
-    char *hostname;
-
-    /* the return string should be freed by caller */
-    hostname = virGetHostname();
-    if (hostname == NULL) {
-        vboxError(conn, VIR_ERR_INTERNAL_ERROR,"%s",
-                  "failed to determine host name");
-        return NULL;
-    }
-
-    return hostname;
-}
-
 static int vboxGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) {
     vboxGlobalData *data = conn->privateData;
     PRUint32 maxCPUCount = 0;
@@ -6402,7 +6388,7 @@ virDriver NAME(Driver) = {
     NULL, /* supports_feature */
     NULL, /* type */
     vboxGetVersion, /* version */
-    vboxGetHostname, /* getHostname */
+    virGetHostname, /* getHostname */
     vboxGetMaxVcpus, /* getMaxVcpus */
     nodeGetInfo, /* nodeGetInfo */
     vboxGetCapabilities, /* getCapabilities */

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 41f340ced75539cea17a0d22f2399d0bd1cb1784..479db103bbc2380e0e7332202cacf1fb52c6fed7 100644 (file)
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -479,24 +479,6 @@ xenUnifiedGetVersion (virConnectPtr conn, unsigned long *hvVer)
     return -1;
 }
 
-/* NB: Even if connected to the proxy, we're still on the
- * same machine.
- */
-static char *
-xenUnifiedGetHostname (virConnectPtr conn)
-{
-    char *result;
-
-    result = virGetHostname();
-    if (result == NULL) {
-        virReportSystemError(conn, errno,
-                             "%s", _("cannot lookup hostname"));
-        return NULL;
-    }
-    /* Caller frees this string. */
-    return result;
-}
-
 static int
 xenUnifiedGetMaxVcpus (virConnectPtr conn, const char *type)
 {
@@ -1660,7 +1642,7 @@ static virDriver xenUnifiedDriver = {
     xenUnifiedSupportsFeature, /* supports_feature */
     xenUnifiedType, /* type */
     xenUnifiedGetVersion, /* version */
-    xenUnifiedGetHostname, /* getHostname */
+    virGetHostname, /* getHostname */
     xenUnifiedGetMaxVcpus, /* getMaxVcpus */
     xenUnifiedNodeGetInfo, /* nodeGetInfo */
     xenUnifiedGetCapabilities, /* getCapabilities */

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index d3ab019c5e105cbddf2149e359ad2e87c4ceb12d..90807546ce02db625d463bb06205e3819065332b 100644 (file)
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4347,11 +4347,9 @@ xenDaemonDomainMigratePrepare (virConnectPtr dconn,
      * deallocates this string.
      */
     if (uri_in == NULL) {
-        *uri_out = virGetHostname();
-        if (*uri_out == NULL) {
-            virReportOOMError(dconn);
+        *uri_out = virGetHostname(dconn);
+        if (*uri_out == NULL)
             return -1;
-        }
     }
 
     return 0;

diff --git a/tools/virsh.c b/tools/virsh.c
index 5ddbcb9ca2ad4af1e7ccaffd65589006a965e26e..f8e6ce4d7abe239188d9d5c7c86bf3de1045e129 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -524,7 +524,7 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom)
     char *thatHost = NULL;
     char *thisHost = NULL;
 
-    if (!(thisHost = virGetHostname())) {
+    if (!(thisHost = virGetHostname(ctl->conn))) {
         vshError(ctl, "%s", _("Failed to get local hostname"));
         goto cleanup;
     }