[libvirt] PATCH: 12/25: Remove global state from Inotify driver
Daniel P. Berrange
berrange at redhat.com
Tue Jan 13 17:43:38 UTC 2009
The 'inotify' driver currently keeps all its state in a global static
variables. Not so great since we access this from all sorts of places
and its hard to guarentee thread safety. Also, we're using per-connection
FD watches to update this state, so if multiple Xen connections were
active, they'd all be updating the same globl state. So we move the
state into the virConnect object. This will increase memory usage if
a single process has multiple Xen connections open though, but not
by very much
xen_inotify.c | 105 +++++++++++++++++++++++++++++++---------------------------
xen_unified.h | 7 +++
xs_internal.c | 33 ++++++++----------
3 files changed, 79 insertions(+), 66 deletions(-)
Daniel
diff --git a/src/xen_inotify.c b/src/xen_inotify.c
--- a/src/xen_inotify.c
+++ b/src/xen_inotify.c
@@ -48,9 +48,6 @@
__FUNCTION__, __LINE__, fmt)
#define LIBVIRTD_DOMAINS_DIR "/var/lib/xend/domains"
-static const char *configDir = NULL;
-static int useXenConfigCache = 0;
-static xenUnifiedDomainInfoListPtr configInfoList = NULL;
struct xenUnifiedDriver xenInotifyDriver = {
xenInotifyOpen, /* open */
@@ -121,6 +118,7 @@ xenInotifyXendDomainsDirLookup(virConnec
virDomainPtr dom;
const char *uuid_str;
unsigned char rawuuid[VIR_UUID_BUFLEN];
+ xenUnifiedPrivatePtr priv = conn->privateData;
/* xend is managing domains. we will get
* a filename in the manner:
@@ -142,15 +140,15 @@ xenInotifyXendDomainsDirLookup(virConnec
/* If we are here, the domain has gone away.
search for, and create a domain from the stored
list info */
- for (i=0; i<configInfoList->count; i++) {
- if (!memcmp(uuid, configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN)) {
- *name = strdup(configInfoList->doms[i]->name);
+ for (i = 0 ; i < priv->configInfoList->count ; i++) {
+ if (!memcmp(uuid, priv->configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN)) {
+ *name = strdup(priv->configInfoList->doms[i]->name);
if (!*name) {
virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
_("finding dom for %s"), uuid_str);
return -1;
}
- memcpy(uuid, configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN);
+ memcpy(uuid, priv->configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN);
DEBUG0("Found dom on list");
return 0;
}
@@ -172,7 +170,8 @@ xenInotifyDomainLookup(virConnectPtr con
xenInotifyDomainLookup(virConnectPtr conn,
const char *filename,
char **name, unsigned char *uuid) {
- if (useXenConfigCache)
+ xenUnifiedPrivatePtr priv = conn->privateData;
+ if (priv->useXenConfigCache)
return xenInotifyXenCacheLookup(filename, name, uuid);
else
return xenInotifyXendDomainsDirLookup(conn, filename, name, uuid);
@@ -195,8 +194,9 @@ xenInotifyDomainEventFromFile(virConnect
}
static int
-xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn ATTRIBUTE_UNUSED,
+xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn,
const char *fname) {
+ xenUnifiedPrivatePtr priv = conn->privateData;
const char *uuidstr = fname + strlen(LIBVIRTD_DOMAINS_DIR) + 1;
unsigned char uuid[VIR_UUID_BUFLEN];
int i;
@@ -208,22 +208,22 @@ xenInotifyXendDomainsDirRemoveEntry(virC
}
/* match and remove on uuid */
- for (i=0; i<configInfoList->count; i++) {
- if (!memcmp(uuid, configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN)) {
- VIR_FREE(configInfoList->doms[i]->name);
- VIR_FREE(configInfoList->doms[i]);
+ for (i = 0 ; i < priv->configInfoList->count ; i++) {
+ if (!memcmp(uuid, priv->configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN)) {
+ VIR_FREE(priv->configInfoList->doms[i]->name);
+ VIR_FREE(priv->configInfoList->doms[i]);
- if (i < (configInfoList->count - 1))
- memmove(configInfoList->doms + i,
- configInfoList->doms + i + 1,
- sizeof(*(configInfoList->doms)) *
- (configInfoList->count - (i + 1)));
+ if (i < (priv->configInfoList->count - 1))
+ memmove(priv->configInfoList->doms + i,
+ priv->configInfoList->doms + i + 1,
+ sizeof(*(priv->configInfoList->doms)) *
+ (priv->configInfoList->count - (i + 1)));
- if (VIR_REALLOC_N(configInfoList->doms,
- configInfoList->count - 1) < 0) {
+ if (VIR_REALLOC_N(priv->configInfoList->doms,
+ priv->configInfoList->count - 1) < 0) {
; /* Failure to reduce memory allocation isn't fatal */
}
- configInfoList->count--;
+ priv->configInfoList->count--;
return 0;
}
}
@@ -235,13 +235,15 @@ xenInotifyXendDomainsDirAddEntry(virConn
const char *fname) {
char *name = NULL;
unsigned char uuid[VIR_UUID_BUFLEN];
+ xenUnifiedPrivatePtr priv = conn->privateData;
+
if (xenInotifyDomainLookup(conn, fname, &name, uuid) < 0) {
virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Error looking up domain"));
return -1;
}
- if( xenUnifiedAddDomainInfo(configInfoList,
+ if (xenUnifiedAddDomainInfo(priv->configInfoList,
-1, name, uuid) < 0) {
virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Error adding file to config cache"));
@@ -255,15 +257,19 @@ static int
static int
xenInotifyRemoveDomainConfigInfo(virConnectPtr conn,
const char *fname) {
- return useXenConfigCache ? xenXMConfigCacheRemoveFile(conn, fname) :
- xenInotifyXendDomainsDirRemoveEntry(conn, fname);
+ xenUnifiedPrivatePtr priv = conn->privateData;
+ return priv->useXenConfigCache ?
+ xenXMConfigCacheRemoveFile(conn, fname) :
+ xenInotifyXendDomainsDirRemoveEntry(conn, fname);
}
static int
xenInotifyAddDomainConfigInfo(virConnectPtr conn,
const char *fname) {
- return useXenConfigCache ? xenXMConfigCacheAddFile(conn, fname) :
- xenInotifyXendDomainsDirAddEntry(conn, fname);
+ xenUnifiedPrivatePtr priv = conn->privateData;
+ return priv->useXenConfigCache ?
+ xenXMConfigCacheAddFile(conn, fname) :
+ xenInotifyXendDomainsDirAddEntry(conn, fname);
}
static void
@@ -277,7 +283,7 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUS
struct inotify_event *e;
int got;
char *tmp, *name;
- virConnectPtr conn = (virConnectPtr) data;
+ virConnectPtr conn = data;
xenUnifiedPrivatePtr priv = NULL;
DEBUG0("got inotify event");
@@ -315,7 +321,8 @@ reread:
name = (char *)&(e->name);
- snprintf(fname, 1024, "%s/%s", configDir, name);
+ snprintf(fname, 1024, "%s/%s",
+ priv->configDir, name);
if (e->mask & (IN_DELETE | IN_MOVED_FROM)) {
virDomainEventPtr event =
@@ -378,24 +385,24 @@ xenInotifyOpen(virConnectPtr conn ATTRIB
if(priv->xendConfigVersion <= 2) {
/* /etc/xen */
- configDir = xenXMGetConfigDir();
- useXenConfigCache = 1;
+ priv->configDir = xenXMGetConfigDir();
+ priv->useXenConfigCache = 1;
} else {
/* /var/lib/xend/domains/<uuid>/config.sxp */
- configDir = LIBVIRTD_DOMAINS_DIR;
- useXenConfigCache = 0;
+ priv->configDir = LIBVIRTD_DOMAINS_DIR;
+ priv->useXenConfigCache = 0;
- if ( VIR_ALLOC(configInfoList ) < 0) {
+ if (VIR_ALLOC(priv->configInfoList) < 0) {
virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
- "%s", _("failed to allocate configInfoList"));
+ "%s", _("failed to allocate configInfoList"));
return -1;
}
/* populate initial list */
- if (!(dh = opendir(configDir))) {
+ if (!(dh = opendir(priv->configDir))) {
virReportSystemError(NULL, errno,
_("cannot open directory: %s"),
- configDir);
+ priv->configDir);
return -1;
}
while ((ent = readdir(dh))) {
@@ -403,9 +410,10 @@ xenInotifyOpen(virConnectPtr conn ATTRIB
continue;
/* Build the full file path */
- if ((strlen(configDir) + 1 + strlen(ent->d_name) + 1) > PATH_MAX)
+ if ((strlen(priv->configDir) + 1 +
+ strlen(ent->d_name) + 1) > PATH_MAX)
continue;
- strcpy(path, configDir);
+ strcpy(path, priv->configDir);
strcat(path, "/");
strcat(path, ent->d_name);
@@ -419,24 +427,25 @@ xenInotifyOpen(virConnectPtr conn ATTRIB
}
if ((priv->inotifyFD = inotify_init()) < 0) {
- virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
- "%s", _("initializing inotify"));
+ virReportSystemError(NULL, errno,
+ "%s", _("initializing inotify"));
return -1;
}
- DEBUG("Adding a watch on %s", configDir);
+ DEBUG("Adding a watch on %s", priv->configDir);
if (inotify_add_watch(priv->inotifyFD,
- configDir,
+ priv->configDir,
IN_CREATE |
IN_CLOSE_WRITE | IN_DELETE |
IN_MOVED_TO | IN_MOVED_FROM) < 0) {
- virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
- _("adding watch on %s"), _(configDir));
+ virReportSystemError(NULL, errno,
+ _("adding watch on %s"),
+ priv->configDir);
return -1;
}
DEBUG0("Building initial config cache");
- if (useXenConfigCache &&
+ if (priv->useXenConfigCache &&
xenXMConfigCacheRefresh (conn) < 0) {
DEBUG("Failed to enable XM config cache %s", conn->err.message);
return -1;
@@ -464,10 +473,10 @@ int
int
xenInotifyClose(virConnectPtr conn)
{
- xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
+ xenUnifiedPrivatePtr priv = conn->privateData;
- if(configInfoList)
- xenUnifiedDomainInfoListFree(configInfoList);
+ if (priv->configInfoList)
+ xenUnifiedDomainInfoListFree(priv->configInfoList);
if (priv->inotifyWatch != -1)
virEventRemoveHandle(priv->inotifyWatch);
diff --git a/src/xen_unified.h b/src/xen_unified.h
--- a/src/xen_unified.h
+++ b/src/xen_unified.h
@@ -156,6 +156,9 @@ struct _xenUnifiedPrivate {
/* A list of xenstore watches */
xenStoreWatchListPtr xsWatchList;
int xsWatch;
+ /* A list of active domain name/uuids */
+ xenUnifiedDomainInfoListPtr activeDomainList;
+
/* An list of callbacks */
virDomainEventCallbackListPtr domainEventCallbacks;
@@ -164,6 +167,10 @@ struct _xenUnifiedPrivate {
/* The inotify fd */
int inotifyFD;
int inotifyWatch;
+
+ const char *configDir;
+ int useXenConfigCache ;
+ xenUnifiedDomainInfoListPtr configInfoList;
#endif
};
diff --git a/src/xs_internal.c b/src/xs_internal.c
--- a/src/xs_internal.c
+++ b/src/xs_internal.c
@@ -46,9 +46,6 @@
#endif
#ifndef PROXY
-/* A list of active domain name/uuids */
-static xenUnifiedDomainInfoListPtr activeDomainList = NULL;
-
static char *xenStoreDomainGetOSType(virDomainPtr domain);
struct xenUnifiedDriver xenStoreDriver = {
@@ -312,7 +309,7 @@ xenStoreOpen(virConnectPtr conn,
#ifndef PROXY
/* Init activeDomainList */
- if ( VIR_ALLOC(activeDomainList) < 0) {
+ if (VIR_ALLOC(priv->activeDomainList) < 0) {
virXenStoreError(NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("failed to allocate activeDomainList"));
return -1;
@@ -389,7 +386,7 @@ xenStoreClose(virConnectPtr conn)
xenStoreWatchListFree(priv->xsWatchList);
#ifndef PROXY
- xenUnifiedDomainInfoListFree(activeDomainList);
+ xenUnifiedDomainInfoListFree(priv->activeDomainList);
#endif
if (priv->xshandle == NULL)
return(-1);
@@ -1188,7 +1185,7 @@ int xenStoreDomainIntroduced(virConnectP
int *new_domids;
int nread;
- xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) opaque;
+ xenUnifiedPrivatePtr priv = opaque;
retry:
new_domain_cnt = xenStoreNumOfDomains(conn);
@@ -1207,8 +1204,8 @@ retry:
missing = 0;
for (i=0 ; i < new_domain_cnt ; i++) {
found = 0;
- for (j = 0 ; j < activeDomainList->count ; j++) {
- if (activeDomainList->doms[j]->id == new_domids[i]) {
+ for (j = 0 ; j < priv->activeDomainList->count ; j++) {
+ if (priv->activeDomainList->doms[j]->id == new_domids[i]) {
found = 1;
break;
}
@@ -1236,7 +1233,7 @@ retry:
xenUnifiedDomainEventDispatch(priv, event);
/* Add to the list */
- xenUnifiedAddDomainInfo(activeDomainList,
+ xenUnifiedAddDomainInfo(priv->activeDomainList,
new_domids[i], name, uuid);
VIR_FREE(name);
@@ -1265,7 +1262,7 @@ int xenStoreDomainReleased(virConnectPtr
xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) opaque;
- if(!activeDomainList->count) return 0;
+ if(!priv->activeDomainList->count) return 0;
retry:
new_domain_cnt = xenStoreNumOfDomains(conn);
@@ -1283,10 +1280,10 @@ retry:
}
removed = 0;
- for (j=0 ; j < activeDomainList->count ; j++) {
+ for (j=0 ; j < priv->activeDomainList->count ; j++) {
found = 0;
for (i=0 ; i < new_domain_cnt ; i++) {
- if (activeDomainList->doms[j]->id == new_domids[i]) {
+ if (priv->activeDomainList->doms[j]->id == new_domids[i]) {
found = 1;
break;
}
@@ -1295,18 +1292,18 @@ retry:
if (!found) {
virDomainEventPtr event =
virDomainEventNew(-1,
- activeDomainList->doms[j]->name,
- activeDomainList->doms[j]->uuid,
+ priv->activeDomainList->doms[j]->name,
+ priv->activeDomainList->doms[j]->uuid,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
if (event)
xenUnifiedDomainEventDispatch(priv, event);
/* Remove from the list */
- xenUnifiedRemoveDomainInfo(activeDomainList,
- activeDomainList->doms[j]->id,
- activeDomainList->doms[j]->name,
- activeDomainList->doms[j]->uuid);
+ xenUnifiedRemoveDomainInfo(priv->activeDomainList,
+ priv->activeDomainList->doms[j]->id,
+ priv->activeDomainList->doms[j]->name,
+ priv->activeDomainList->doms[j]->uuid);
removed = 1;
}
--
|: 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 :|
More information about the libvir-list
mailing list