[PATCH] Fix xm_internal configCache

["Richard W.M. Jones" <rjones@xxxxxxxxxx>]

This patch:

(1) Fixes inactive domains over remote.

The real cause of this error turned out to be that after configCache 
hash is created, the last refresh date wasn't reset, so a subsequent 
call to xenXMConfigCacheRefresh might not actually repopulate the cache. 
 This only affects the remote case, because only in the remote case do 
we have a long running process (libvirtd) which closes and reopens 
xm_internal.c, so this code had never been exercised before.

It also explains why the bug was annoyingly intermittent.  If you wait 
>= 10 seconds during testing, then the cache does get refilled, so the 
bug is hidden.

(2) Adds error messages to xenXMConfigCacheRefresh.

(3) As Daniel Veillard suggested, change the obscure !nconnections-- 
condition to two explicit statements.

(4) In xenXMOpen, change the condition from nconnections == 0 to 
configCache == NULL, to remove the implicit dependency between the two 
variables.

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903

? qemud/libvirtd.init
Index: src/xm_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xm_internal.c,v
retrieving revision 1.33
diff -u -p -r1.33 xm_internal.c
--- src/xm_internal.c	26 Jun 2007 22:33:22 -0000	1.33
+++ src/xm_internal.c	4 Jul 2007 12:05:13 -0000
@@ -27,6 +27,8 @@
 #include <time.h>
 #include <sys/stat.h>
 #include <limits.h>
+#include <string.h>
+#include <errno.h>
 
 #include <unistd.h>
 #include <stdint.h>
@@ -326,13 +328,14 @@ static int xenXMConfigReaper(const void 
    environment variable) and process any domain configs. It
    has rate-limited so never rescans more frequently than
    once every X seconds */
-static int xenXMConfigCacheRefresh(void) {
+static int xenXMConfigCacheRefresh (virConnectPtr conn) {
     DIR *dh;
     struct dirent *ent;
     time_t now = time(NULL);
     int ret = -1;
 
     if (now == ((time_t)-1)) {
+        xenXMError (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno));
         return (-1);
     }
 
@@ -344,6 +347,7 @@ static int xenXMConfigCacheRefresh(void)
 
     /* Process the files in the config dir */
     if (!(dh = opendir(configDir))) {
+        xenXMError (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno));
         return (-1);
     }
 
@@ -418,6 +422,7 @@ static int xenXMConfigCacheRefresh(void)
         } else { /* Completely new entry */
             newborn = 1;
             if (!(entry = malloc(sizeof(xenXMConfCache)))) {
+                xenXMError (conn, VIR_ERR_NO_MEMORY, strerror (errno));
                 goto cleanup;
             }
             memcpy(entry->filename, path, PATH_MAX);
@@ -439,6 +444,7 @@ static int xenXMConfigCacheRefresh(void)
                 virHashRemoveEntry(configCache, path, NULL);
             }
             free(entry);
+            xenXMError (conn, VIR_ERR_INTERNAL_ERROR, "xenXMConfigCacheRefresh: name");
             goto cleanup;
         }
 
@@ -448,6 +454,7 @@ static int xenXMConfigCacheRefresh(void)
             if (virHashAddEntry(configCache, entry->filename, entry) < 0) {
                 virConfFree(entry->conf);
                 free(entry);
+                xenXMError (conn, VIR_ERR_INTERNAL_ERROR, "xenXMConfigCacheRefresh: virHashAddEntry");
                 goto cleanup;
             }
         }
@@ -489,7 +496,7 @@ int
 xenXMOpen (virConnectPtr conn ATTRIBUTE_UNUSED,
            const char *name ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED)
 {
-    if (nconnections == 0) {
+    if (configCache == NULL) {
         configCache = virHashCreate(50);
         if (!configCache)
             return (-1);
@@ -499,6 +506,10 @@ xenXMOpen (virConnectPtr conn ATTRIBUTE_
             configCache = NULL;
             return (-1);
         }
+        /* Force the cache to be reloaded next time that
+         * xenXMConfigCacheRefresh is called.
+         */
+        lastRefresh = 0;
     }
     nconnections++;
 
@@ -510,7 +521,8 @@ xenXMOpen (virConnectPtr conn ATTRIBUTE_
  * last connection
  */
 int xenXMClose(virConnectPtr conn ATTRIBUTE_UNUSED) {
-    if (!nconnections--) {
+    nconnections--;
+    if (nconnections == 0) {
         virHashFree(nameConfigMap, NULL);
         nameConfigMap = NULL;
         virHashFree(configCache, xenXMConfigFree);
@@ -1211,7 +1223,7 @@ virDomainPtr xenXMDomainLookupByName(vir
         return (NULL);
     }
 
-    if (xenXMConfigCacheRefresh() < 0)
+    if (xenXMConfigCacheRefresh (conn) < 0)
         return (NULL);
 
     if (!(filename = virHashLookup(nameConfigMap, domname)))
@@ -1274,7 +1286,7 @@ virDomainPtr xenXMDomainLookupByUUID(vir
         return (NULL);
     }
 
-    if (xenXMConfigCacheRefresh() < 0)
+    if (xenXMConfigCacheRefresh (conn) < 0)
         return (NULL);
 
     if (!(entry = virHashSearch(configCache, xenXMDomainSearchForUUID, (const void *)uuid))) {
@@ -2115,7 +2127,7 @@ virDomainPtr xenXMDomainDefineXML(virCon
     if (conn->flags & VIR_CONNECT_RO)
         return (NULL);
 
-    if (xenXMConfigCacheRefresh() < 0)
+    if (xenXMConfigCacheRefresh (conn) < 0)
         return (NULL);
 
     if (!(conf = xenXMParseXMLToConfig(conn, xml)))
@@ -2296,7 +2308,7 @@ int xenXMListDefinedDomains(virConnectPt
         return (-1);
     }
 
-    if (xenXMConfigCacheRefresh() < 0)
+    if (xenXMConfigCacheRefresh (conn) < 0)
         return (-1);
 
     if (maxnames > virHashSize(configCache))
@@ -2321,7 +2333,7 @@ int xenXMNumOfDefinedDomains(virConnectP
         return (-1);
     }
 
-    if (xenXMConfigCacheRefresh() < 0)
+    if (xenXMConfigCacheRefresh (conn) < 0)
         return (-1);
 
     return virHashSize(nameConfigMap);

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

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