[PATCH] virutil: Introduce virGetHostnameSimple()

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

After 759b4d1b0fe5f we are getting hostname in virLogOnceInit().
Problem with this approach is in the NSS module because the
module calls some internal APIs which occasionally want to log
something. This results in virLogInitialize() to be called which
in turn ends up calling virGetHostnameQuiet() and effectively the
control gets to NSS plugin again which calls some internal APIs
which occasionally want to log something. You can see the
deadlock now.

One way out of this is to call only gethostname() and not whole
virGetHostnameQuiet() machinery.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/libvirt_private.syms |  1 +
 src/util/virlog.c        |  2 +-
 src/util/virutil.c       | 46 ++++++++++++++++++++++++++++++++++------------
 src/util/virutil.h       |  1 +
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3b14d7d15..3ef55f809 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3016,6 +3016,7 @@ virGetGroupList;
 virGetGroupName;
 virGetHostname;
 virGetHostnameQuiet;
+virGetHostnameSimple;
 virGetListenFDs;
 virGetSelfLastChanged;
 virGetSystemPageSize;
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 8f1e4800d..fc854ffeb 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -276,7 +276,7 @@ virLogOnceInit(void)
      * it might not be possible to load NSS modules via getaddrinfo()
      * (e.g. at container startup the host filesystem will not be
      * accessible anymore. */
-    virLogHostname = virGetHostnameQuiet();
+    virLogHostname = virGetHostnameSimple();
 
     virLogUnlock();
     return 0;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index cd6fbf2f3..22adecd53 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -684,26 +684,23 @@ static char *
 virGetHostnameImpl(bool quiet)
 {
     int r;
-    char hostname[HOST_NAME_MAX+1], *result = NULL;
+    char *result;
     struct addrinfo hints, *info;
 
-    r = gethostname(hostname, sizeof(hostname));
-    if (r == -1) {
+    if (!(result = virGetHostnameSimple())) {
         if (!quiet)
             virReportSystemError(errno,
                                  "%s", _("failed to determine host name"));
         return NULL;
     }
-    NUL_TERMINATE(hostname);
 
-    if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) {
+    if (STRPREFIX(result, "localhost") || strchr(result, '.')) {
         /* in this case, gethostname returned localhost (meaning we can't
          * do any further canonicalization), or it returned an FQDN (and
          * we don't need to do any further canonicalization).  Return the
          * string as-is; it's up to callers to check whether "localhost"
          * is allowed.
          */
-        ignore_value(VIR_STRDUP_QUIET(result, hostname));
         goto cleanup;
     }
 
@@ -714,12 +711,11 @@ virGetHostnameImpl(bool quiet)
     memset(&hints, 0, sizeof(hints));
     hints.ai_flags = AI_CANONNAME|AI_CANONIDN;
     hints.ai_family = AF_UNSPEC;
-    r = getaddrinfo(hostname, NULL, &hints, &info);
+    r = getaddrinfo(result, NULL, &hints, &info);
     if (r != 0) {
         if (!quiet)
             VIR_WARN("getaddrinfo failed for '%s': %s",
-                     hostname, gai_strerror(r));
-        ignore_value(VIR_STRDUP_QUIET(result, hostname));
+                     result, gai_strerror(r));
         goto cleanup;
     }
 
@@ -727,15 +723,16 @@ virGetHostnameImpl(bool quiet)
     sa_assert(info);
 
     if (info->ai_canonname == NULL ||
-        STRPREFIX(info->ai_canonname, "localhost"))
+        STRPREFIX(info->ai_canonname, "localhost")) {
         /* in this case, we tried to canonicalize and we ended up back with
          * localhost.  Ignore the canonicalized name and just return the
          * original hostname
          */
-        ignore_value(VIR_STRDUP_QUIET(result, hostname));
-    else
+    } else {
         /* Caller frees this string. */
+        VIR_FREE(result);
         ignore_value(VIR_STRDUP_QUIET(result, info->ai_canonname));
+    }
 
     freeaddrinfo(info);
 
@@ -760,6 +757,31 @@ virGetHostnameQuiet(void)
 }
 
 
+/**
+ * virGetHostnameSimple:
+ *
+ * Plain wrapper over gethostname(). The difference to
+ * virGetHostname() is that this function doesn't try to
+ * canonicalize the hostname.
+ *
+ * Returns: hostname string (caller must free),
+ *          NULL on error.
+ */
+char *
+virGetHostnameSimple(void)
+{
+    char hostname[HOST_NAME_MAX+1];
+    char *ret;
+
+    if (gethostname(hostname, sizeof(hostname)) == -1)
+        return NULL;
+
+    NUL_TERMINATE(hostname);
+    ignore_value(VIR_STRDUP_QUIET(ret, hostname));
+    return ret;
+}
+
+
 char *
 virGetUserDirectory(void)
 {
diff --git a/src/util/virutil.h b/src/util/virutil.h
index be0f6b0ea..57148374b 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -136,6 +136,7 @@ static inline int pthread_sigmask(int how,
 
 char *virGetHostname(void);
 char *virGetHostnameQuiet(void);
+char *virGetHostnameSimple(void);
 
 char *virGetUserDirectory(void);
 char *virGetUserDirectoryByUID(uid_t uid);
-- 
2.13.6

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list