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.

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);

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 */

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 */

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 */

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 */

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;
 }

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);
 

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 */

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 */

src/xen/xend_internal.c

@@ -4347,12 +4347,10 @@ 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;
 }

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;
     }