xen: fix race in refresh of config cache

The xenXMConfigCacheRefresh method scans /etc/xen and loads
all config files it finds. It then scans its internal hash
table and purges any (previously) loaded config files whose
refresh timestamp does not match the timestamp recorded at
the start of xenXMConfigCacheRefresh(). There is unfortunately
a subtle flaw in this, because if loading the config files
takes longer than 1 second, some of the config files will
have a refresh timestamp that is 1 or more seconds different
(newer) than is checked for. So we immediately purge a bunch
of valid config files we just loaded.

To avoid this flaw, we must pass the timestamp we record at
the start of xenXMConfigCacheRefresh() into the
xenXMConfigCacheAddFile() method, instead of letting the
latter call time(NULL) again.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index cd1e2dfba97655d0720edacff970e0bd1724b60b..d81a35d427783801e2fc259eaaca3d9730cd83c9 100644 (file)
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -217,11 +217,11 @@ xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, const char *fname)
 }
 
 static int
-xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname)
+xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname, time_t now)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
     return priv->useXenConfigCache ?
-        xenXMConfigCacheAddFile(conn, fname) :
+        xenXMConfigCacheAddFile(conn, fname, now) :
         xenInotifyXendDomainsDirAddEntry(conn, fname);
 }
 
@@ -238,6 +238,7 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUSED,
     char *tmp, *name;
     virConnectPtr conn = data;
     xenUnifiedPrivatePtr priv = NULL;
+    time_t now = time(NULL);
 
     VIR_DEBUG("got inotify event");
 
@@ -300,7 +301,7 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUSED,
             }
         } else if (e->mask & (IN_CREATE | IN_CLOSE_WRITE | IN_MOVED_TO)) {
             virObjectEventPtr event;
-            if (xenInotifyAddDomainConfigInfo(conn, fname) < 0) {
+            if (xenInotifyAddDomainConfigInfo(conn, fname, now) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                "%s", _("Error adding file to config cache"));
                 goto cleanup;
@@ -344,6 +345,7 @@ xenInotifyOpen(virConnectPtr conn,
     char *path;
     xenUnifiedPrivatePtr priv = conn->privateData;
     int direrr;
+    time_t now = time(NULL);
 
     virCheckFlags(VIR_CONNECT_RO, -1);
 
@@ -374,7 +376,7 @@ xenInotifyOpen(virConnectPtr conn,
                 return -1;
             }
 
-            if (xenInotifyAddDomainConfigInfo(conn, path) < 0) {
+            if (xenInotifyAddDomainConfigInfo(conn, path, now) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                "%s", _("Error adding file to config list"));
                 closedir(dh);

diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 59b1cd4afac2b54f9df4f1cd6eb074de7adfe3ba..07b9eb46e6223e9b7a9cebf3cc4045debc895770 100644 (file)
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -191,15 +191,14 @@ xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename)
  * calling this function
  */
 int
-xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
+xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename, time_t now)
 {
     xenUnifiedPrivatePtr priv = conn->privateData;
     xenXMConfCachePtr entry;
     struct stat st;
     int newborn = 0;
-    time_t now = time(NULL);
 
-    VIR_DEBUG("Adding file %s", filename);
+    VIR_DEBUG("Adding file %s %lld", filename, (long long)now);
 
     /* Get modified time */
     if ((stat(filename, &st) < 0)) {
@@ -373,7 +372,7 @@ xenXMConfigCacheRefresh(virConnectPtr conn)
 
         /* If we already have a matching entry and it is not
            modified, then carry on to next one*/
-        if (xenXMConfigCacheAddFile(conn, path) < 0) {
+        if (xenXMConfigCacheAddFile(conn, path, now) < 0) {
             /* Ignoring errors, since a lot of stuff goes wrong in /etc/xen */
         }
 

diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h
index 25b4fd565166d5e15c371aeeb59f5fa9788d5b27..0246895d89580a3d6a6a8a117bcff0021e05b12b 100644 (file)
--- a/src/xen/xm_internal.h
+++ b/src/xen/xm_internal.h
@@ -31,7 +31,7 @@
 # include "domain_conf.h"
 
 int xenXMConfigCacheRefresh (virConnectPtr conn);
-int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename);
+int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename, time_t now);
 int xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename);
 
 int xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags);