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

Re: [libvirt] [PATCH] Xen Events Updated



On Thu, Nov 20, 2008 at 03:49:07PM -0500, Ben Guthro wrote:
> New xen events patch attached. This removes a couple unnecessary 
> changes from my prior patch, but remains functionally the same as
> the last version.
> 
> This will emit the following events for Xen:
> 
> STARTED
> STOPPED
> ADDED
> REMOVED

I hit a few problems with the old Xen 3.0.3 codepath for /etc/xen files
in this patch. What follows is the patch I needed to make it work
reliably on RHEL-5 Xen.

The changes are

 - Remove hardcoded qemu:///system to let libvirt probe HV
 - Add a cast to workaround lack of const-ness in RHEL5 python
 - Add an explicit xenXMConfigRemoveFile() to deal with files
   going away
 - Remove use of IN_MODIFY - it fired when the config was still
   incompletely written resulting in wierd errors
 - Add use of IN_CLOSE_WRITE so we're notified when the file is
   finished writing
 - Ignoring IN_CREATE if stat() shows zero size, because this fires
   the moment the name is allocated in the FS, but before content
   is created. We can't ignore it completely, because its needed
   if someone creates the file via hard-linking, when the entire
   content appears attomically & no IN_CLOSED_WRITE is fired.
 - Allocate capabilities info before initializing inotify driver
   because loading XM config files /etc/xen requires this
 - Fix removal of file handles for inotify & xenstore, since we
   need to remove based on watch number, not FD number

Regards,
Daniel

diff -r fe87b41b48e3 examples/domain-events/events-c/event-test.c
--- a/examples/domain-events/events-c/event-test.c	Fri Nov 21 08:02:15 2008 -0500
+++ b/examples/domain-events/events-c/event-test.c	Fri Nov 21 08:40:58 2008 -0500
@@ -308,7 +308,7 @@ int main(int argc, char **argv)
                           myEventRemoveTimeoutFunc);
 
     virConnectPtr dconn = NULL;
-    dconn = virConnectOpen (argv[1] ? argv[1] : "qemu:///system");
+    dconn = virConnectOpen (argv[1] ? argv[1] : NULL);
     if (!dconn) {
         printf("error opening\n");
         return -1;
diff -r fe87b41b48e3 include/libvirt/virterror.h
--- a/include/libvirt/virterror.h	Fri Nov 21 08:02:15 2008 -0500
+++ b/include/libvirt/virterror.h	Fri Nov 21 08:02:47 2008 -0500
@@ -60,6 +60,7 @@ typedef enum {
     VIR_FROM_DOMAIN,    /* Error from domain config */
     VIR_FROM_UML,       /* Error at the UML driver */
     VIR_FROM_NODEDEV, /* Error from node device monitor */
+    VIR_FROM_XEN_INOTIFY, /* Error from xen inotify layer */
 } virErrorDomain;
 
 
diff -r fe87b41b48e3 python/libvir.c
--- a/python/libvir.c	Fri Nov 21 08:02:15 2008 -0500
+++ b/python/libvir.c	Fri Nov 21 08:36:40 2008 -0500
@@ -1564,7 +1564,8 @@ getLibvirtModuleObject (void) {
         return libvirt_module;
 
     // PyImport_ImportModule returns a new reference
-    libvirt_module = PyImport_ImportModule("libvirt");
+    /* Bogus (char *) cast for RHEL-5 python API brokenness */
+    libvirt_module = PyImport_ImportModule((char *)"libvirt");
     if(!libvirt_module) {
 #if DEBUG_ERROR
         printf("%s Error importing libvirt module\n", __FUNCTION__);
diff -r fe87b41b48e3 src/util.c
--- a/src/util.c	Fri Nov 21 08:02:15 2008 -0500
+++ b/src/util.c	Fri Nov 21 11:07:03 2008 -0500
@@ -613,6 +613,10 @@ virRun(virConnectPtr conn,
     VIR_FREE(outbuf);
     VIR_FREE(errbuf);
     VIR_FREE(argv_str);
+    if (outfd != -1)
+        close(outfd);
+    if (errfd != -1)
+        close(errfd);
     return ret;
 }
 
diff -r fe87b41b48e3 src/xen_inotify.c
--- a/src/xen_inotify.c	Fri Nov 21 08:02:15 2008 -0500
+++ b/src/xen_inotify.c	Fri Nov 21 11:07:22 2008 -0500
@@ -49,12 +49,6 @@ static const char *configDir        = NU
 static const char *configDir        = NULL;
 static int  useXenConfigCache = 0;
 static xenUnifiedDomainInfoListPtr configInfoList = NULL;
-
-/* declared in xm_internal.c */
-virHashTablePtr xenXMGetConfigCache(void);
-char *xenXMGetConfigDir(void);
-int xenXMConfigCacheRefresh (virConnectPtr conn);
-int xenXMConfigCacheAddRemoveFile(virConnectPtr conn, const char *filename);
 
 struct xenUnifiedDriver xenInotifyDriver = {
     xenInotifyOpen, /* open */
@@ -121,7 +115,7 @@ xenInotifyXendDomainsDirLookup(virConnec
     int i;
     virDomainPtr dom;
     const char *uuid_str;
-    const unsigned char uuid[VIR_UUID_BUFLEN];
+    unsigned char uuid[VIR_UUID_BUFLEN];
 
     /* xend is managing domains. we will get
     * a filename in the manner:
@@ -145,7 +139,7 @@ xenInotifyXendDomainsDirLookup(virConnec
         for (i=0; i<configInfoList->count; i++) {
             if (STREQ(uuid_str, configInfoList->doms[i]->uuid)) {
                 if(!(dom = virGetDomain(conn, configInfoList->doms[i]->name,
-                                        configInfoList->doms[i]->uuid))) {
+                                        (unsigned char *)configInfoList->doms[i]->uuid))) {
                     virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
                                        "finding dom for %s", uuid_str);
                     return NULL;
@@ -238,14 +232,14 @@ static int
 static int
 xenInotifyRemoveDomainConfigInfo(virConnectPtr conn,
                                  const char *fname) {
-    return useXenConfigCache ? xenXMConfigCacheAddRemoveFile(conn, fname) :
+    return useXenConfigCache ? xenXMConfigCacheRemoveFile(conn, fname) :
                                xenInotifyXendDomainsDirRemoveEntry(conn, fname);
 }
 
 static int
 xenInotifyAddDomainConfigInfo(virConnectPtr conn,
                               const char *fname) {
-    return useXenConfigCache ? xenXMConfigCacheAddRemoveFile(conn, fname) :
+    return useXenConfigCache ? xenXMConfigCacheAddFile(conn, fname) :
                                xenInotifyXendDomainsDirAddEntry(conn, fname);
 }
 
@@ -318,7 +312,7 @@ reread:
                                    "%s", _("Error adding file to config cache"));
                 return;
             }
-        } else if (e->mask & ( IN_MODIFY | IN_CREATE) ) {
+        } else if (e->mask & ( IN_CREATE | IN_CLOSE_WRITE) ) {
             if (xenInotifyAddDomainConfigInfo(conn, fname) < 0 ) {
                 virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
                                    "%s", _("Error adding file to config cache"));
@@ -409,7 +403,8 @@ xenInotifyOpen(virConnectPtr conn ATTRIB
     DEBUG("Adding a watch on %s", configDir);
     if (inotify_add_watch(priv->inotifyFD,
                           configDir,
-                          IN_CREATE | IN_MODIFY | IN_DELETE) < 0) {
+                          IN_CREATE |
+                          IN_CLOSE_WRITE | IN_DELETE) < 0) {
         virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
                            "adding watch on %s", _(configDir));
         return -1;
@@ -417,15 +412,16 @@ xenInotifyOpen(virConnectPtr conn ATTRIB
 
     DEBUG0("Building initial config cache");
     if (useXenConfigCache &&
-        xenXMConfigCacheRefresh (conn) < 0)
-        return -1;
-
+        xenXMConfigCacheRefresh (conn) < 0) {
+        DEBUG("Failed to enable XM config cache %s", conn->err.message);
+        return -1;
+    }
+
+    DEBUG0("Registering with event loop");
     /* Add the handle for monitoring */
-    if (virEventAddHandle(priv->inotifyFD, VIR_EVENT_HANDLE_READABLE,
-                          xenInotifyEvent, conn, NULL) < 0) {
-        virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
-                           "%s", _("Failed to add inotify event handle"));
-        return -1;
+    if ((priv->inotifyWatch = virEventAddHandle(priv->inotifyFD, VIR_EVENT_HANDLE_READABLE,
+                                                xenInotifyEvent, conn, NULL)) < 0) {
+        DEBUG0("Failed to add inotify handle, disabling events");
     }
 
     conn->refs++;
@@ -448,7 +444,8 @@ xenInotifyClose(virConnectPtr conn)
     if(configInfoList)
         xenUnifiedDomainInfoListFree(configInfoList);
 
-    virEventRemoveHandle(priv->inotifyFD);
+    if (priv->inotifyWatch != -1)
+        virEventRemoveHandle(priv->inotifyWatch);
     close(priv->inotifyFD);
     virUnrefConnect(conn);
 
diff -r fe87b41b48e3 src/xen_unified.c
--- a/src/xen_unified.c	Fri Nov 21 08:02:15 2008 -0500
+++ b/src/xen_unified.c	Fri Nov 21 09:25:22 2008 -0500
@@ -291,6 +291,11 @@ xenUnifiedOpen (virConnectPtr conn, virC
             DEBUG0("Activated hypervisor sub-driver");
             priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET] = 1;
         }
+    }
+
+    if (!(priv->caps = xenHypervisorMakeCapabilities(conn))) {
+        DEBUG0("Failed to make capabilities");
+        goto fail;
     }
 
     /* XenD is required to suceed if root.
@@ -351,11 +356,6 @@ xenUnifiedOpen (virConnectPtr conn, virC
     }
 #endif
 
-    if (!(priv->caps = xenHypervisorMakeCapabilities(conn))) {
-        DEBUG0("Failed to make capabilities");
-        goto fail;
-    }
-
     return VIR_DRV_OPEN_SUCCESS;
 
 fail:
@@ -1324,6 +1324,11 @@ xenUnifiedDomainEventRegister (virConnec
                                void (*freefunc)(void *))
 {
     GET_PRIVATE (conn);
+    if (priv->xsWatch == -1) {
+        xenUnifiedError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+        return -1;
+    }
+
     conn->refs++;
     return virDomainEventCallbackListAdd(conn, priv->domainEventCallbacks,
                                          callback, opaque, freefunc);
@@ -1335,6 +1340,11 @@ xenUnifiedDomainEventDeregister (virConn
 {
     int ret;
     GET_PRIVATE (conn);
+    if (priv->xsWatch == -1) {
+        xenUnifiedError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+        return -1;
+    }
+
     ret = virDomainEventCallbackListRemove(conn, priv->domainEventCallbacks,
                                             callback);
     virUnrefConnect(conn);
diff -r fe87b41b48e3 src/xen_unified.h
--- a/src/xen_unified.h	Fri Nov 21 08:02:15 2008 -0500
+++ b/src/xen_unified.h	Fri Nov 21 09:16:19 2008 -0500
@@ -155,6 +155,7 @@ struct _xenUnifiedPrivate {
 
     /* A list of xenstore watches */
     xenStoreWatchListPtr xsWatchList;
+    int xsWatch;
 
     /* An list of callbacks */
     virDomainEventCallbackListPtr domainEventCallbacks;
@@ -162,6 +163,7 @@ struct _xenUnifiedPrivate {
 #if WITH_XEN_INOTIFY
     /* The inotify fd */
     int inotifyFD;
+    int inotifyWatch;
 #endif
 };
 
diff -r fe87b41b48e3 src/xm_internal.c
--- a/src/xm_internal.c	Fri Nov 21 08:02:15 2008 -0500
+++ b/src/xm_internal.c	Fri Nov 21 10:51:01 2008 -0500
@@ -45,6 +45,8 @@
 #include "uuid.h"
 #include "util.h"
 #include "memory.h"
+#include "logging.h"
+
 
 /* The true Xen limit varies but so far is always way
    less than 1024, which is the Linux kernel limit according
@@ -65,10 +67,6 @@ char * xenXMAutoAssignMac(void);
 char * xenXMAutoAssignMac(void);
 static int xenXMDomainAttachDevice(virDomainPtr domain, const char *xml);
 static int xenXMDomainDetachDevice(virDomainPtr domain, const char *xml);
-virHashTablePtr xenXMGetConfigCache (void);
-char *xenXMGetConfigDir (void);
-int xenXMConfigCacheRefresh (virConnectPtr conn);
-int xenXMConfigCacheAddRemoveFile(virConnectPtr conn, const char *filename);
 
 #define XM_REFRESH_INTERVAL 10
 
@@ -377,15 +375,45 @@ xenXMConfigSaveFile(virConnectPtr conn, 
 }
 
 int
-xenXMConfigCacheAddRemoveFile(virConnectPtr conn, const char *filename)
+xenXMConfigCacheRemoveFile(virConnectPtr conn ATTRIBUTE_UNUSED,
+                           const char *filename)
+{
+    xenXMConfCachePtr entry;
+
+    entry = virHashLookup(configCache, filename);
+    if (!entry) {
+        DEBUG("No config entry for %s", filename);
+        return 0;
+    }
+
+    virHashRemoveEntry(nameConfigMap, entry->def->name, NULL);
+    virHashRemoveEntry(configCache, filename, xenXMConfigFree);
+    DEBUG("Removed %s %s", entry->def->name, filename);
+    return 0;
+}
+
+
+int
+xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
 {
     xenXMConfCachePtr entry;
     struct stat st;
     int newborn = 0;
     time_t now = time(NULL);
 
+    DEBUG("Adding file %s", filename);
+
     /* Get modified time */
     if ((stat(filename, &st) < 0)) {
+        xenXMError (conn, VIR_ERR_INTERNAL_ERROR,
+                    "cannot stat %s: %s", filename, strerror(errno));
+        return -1;
+    }
+
+    /* Ignore zero length files, because inotify fires before
+       any content has actually been created */
+    if (st.st_size == 0) {
+        DEBUG("Ignoring zero length file %s", filename);
         return -1;
     }
 
@@ -421,6 +449,7 @@ xenXMConfigCacheAddRemoveFile(virConnect
     entry->refreshedAt = now;
 
     if (!(entry->def = xenXMConfigReadFile(conn, entry->filename))) {
+        DEBUG("Failed to read %s", entry->filename);
         if (!newborn)
             virHashRemoveEntry(configCache, filename, NULL);
         VIR_FREE(entry);
@@ -449,6 +478,7 @@ xenXMConfigCacheAddRemoveFile(virConnect
             VIR_FREE(entry);
         }
     }
+    DEBUG("Added config %s %s", entry->def->name, filename);
 
     return 0;
 }
@@ -526,8 +556,8 @@ int xenXMConfigCacheRefresh (virConnectP
 
         /* If we already have a matching entry and it is not
            modified, then carry on to next one*/
-        if (xenXMConfigCacheAddRemoveFile(conn, path) < 0) {
-            goto cleanup;
+        if (xenXMConfigCacheAddFile(conn, path) < 0) {
+            /* Ignoring errors, since alot of stuff goes wrong in /etc/xen */
         }
     }
 
@@ -538,7 +568,6 @@ int xenXMConfigCacheRefresh (virConnectP
     virHashRemoveSet(configCache, xenXMConfigReaper, xenXMConfigFree, (const void*) &now);
     ret = 0;
 
- cleanup:
     if (dh)
         closedir(dh);
 
diff -r fe87b41b48e3 src/xm_internal.h
--- a/src/xm_internal.h	Fri Nov 21 08:02:15 2008 -0500
+++ b/src/xm_internal.h	Fri Nov 21 10:32:03 2008 -0500
@@ -32,6 +32,12 @@ extern struct xenUnifiedDriver xenXMDriv
 extern struct xenUnifiedDriver xenXMDriver;
 int xenXMInit (void);
 
+virHashTablePtr xenXMGetConfigCache(void);
+char *xenXMGetConfigDir(void);
+int xenXMConfigCacheRefresh (virConnectPtr conn);
+int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename);
+int xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename);
+
 int xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags);
 int xenXMClose(virConnectPtr conn);
 const char *xenXMGetType(virConnectPtr conn);
diff -r fe87b41b48e3 src/xs_internal.c
--- a/src/xs_internal.c	Fri Nov 21 08:02:15 2008 -0500
+++ b/src/xs_internal.c	Fri Nov 21 09:25:57 2008 -0500
@@ -44,10 +44,10 @@
 #error "unsupported platform"
 #endif
 
+#ifndef PROXY
 /* A list of active domain name/uuids */
 static xenUnifiedDomainInfoListPtr activeDomainList = NULL;
 
-#ifndef PROXY
 static char *xenStoreDomainGetOSType(virDomainPtr domain);
 
 struct xenUnifiedDriver xenStoreDriver = {
@@ -345,16 +345,12 @@ xenStoreOpen(virConnectPtr conn,
     }
 
     /* Add an event handle */
-    if (virEventAddHandle(xs_fileno(priv->xshandle),
-                          VIR_EVENT_HANDLE_READABLE,
-                          xenStoreWatchEvent,
-                          conn,
-                          NULL) < 0) {
-        virXenStoreError(NULL, VIR_ERR_INTERNAL_ERROR,
-                                 "%s", _("failed to handle"));
-
-        return -1;
-    }
+    if ((priv->xsWatch = virEventAddHandle(xs_fileno(priv->xshandle),
+                                           VIR_EVENT_HANDLE_READABLE,
+                                           xenStoreWatchEvent,
+                                           conn,
+                                           NULL)) < 0)
+        DEBUG0("Failed to add event handle, disabling events\n");
 
 #endif //PROXY
     return 0;
@@ -371,7 +367,6 @@ int
 int
 xenStoreClose(virConnectPtr conn)
 {
-    int fd;
     xenUnifiedPrivatePtr priv;
 
     if (conn == NULL) {
@@ -398,9 +393,8 @@ xenStoreClose(virConnectPtr conn)
     if (priv->xshandle == NULL)
         return(-1);
 
-    fd = xs_fileno(priv->xshandle);
-    virEventRemoveHandle(fd);
-    close(fd);
+    if (priv->xsWatch != -1)
+        virEventRemoveHandle(priv->xsWatch);
     xs_daemon_close(priv->xshandle);
     priv->xshandle = NULL;
 



-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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