[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH v2] 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.

Signed-off-by: Cole Robinson <crobinso redhat com>
---
 daemon/libvirtd.c       |    7 +++----
 src/lxc/lxc_driver.c    |   16 +---------------
 src/qemu/qemu_driver.c  |   22 ++--------------------
 src/test/test_driver.c  |   16 +---------------
 src/uml/uml_driver.c    |   17 +----------------
 src/util/util.c         |   18 +++++++++++++++---
 src/util/util.h         |    2 +-
 src/vbox/vbox_tmpl.c    |   16 +---------------
 src/xen/xen_driver.c    |   20 +-------------------
 src/xen/xend_internal.c |    6 ++----
 tools/virsh.c           |    2 +-
 11 files changed, 29 insertions(+), 113 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 0615cd2..322a320 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -993,11 +993,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 47e59f6..05fc48d 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2027,20 +2027,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 */
@@ -2258,7 +2244,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 437a1b4..0470315 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2610,21 +2610,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;
@@ -6248,11 +6233,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
@@ -7084,7 +7066,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 7e19072..31b5ad3 100644
--- 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 fac3c4c..2455b92 100644
--- 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 08070da..853d3a0 100644
--- 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 3ef26e6..f4e395e 100644
--- 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 aecda23..c6305ac 100644
--- 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 f2744b0..6dc4ac0 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -478,24 +478,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)
 {
@@ -1659,7 +1641,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 d3ab019..9080754 100644
--- 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 6b93405..3c668da 100644
--- 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;
     }
-- 
1.6.5.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]