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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list