[libvirt] [PATCH] Xen Events Updated
Ben Guthro
bguthro at virtualiron.com
Fri Nov 21 18:06:08 UTC 2008
I tested these changes, and seem to be having some issues opening the xen driver in a post 3.0.3 codepath:
DEBUG: xen_unified.c: xenUnifiedOpen (Trying hypervisor sub-driver)
DEBUG: xen_unified.c: xenUnifiedOpen (Activated hypervisor sub-driver)
libvir: Xen Daemon error : internal error failed to create a socket
libvir: Xen error : out of memory
DEBUG: xen_unified.c: xenUnifiedOpen (Failed to make capabilities)
DEBUG: xen_unified.c: xenUnifiedOpen (Failed to activate a mandatory sub-driver)
DEBUG: libvirt.c: do_open (driver 1 Xen returned ERROR)
I haven't chased this down fully yet, but something is interfering...
Daniel P. Berrange wrote on 11/21/2008 11:13 AM:
> 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;
>
>
>
>
More information about the libvir-list
mailing list