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(a)redhat.com&gt; --- 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