This patch provides for locking of the virDomainObjPtr instances on a per
object level, and defines locking semantics of the various API calls
that use these objects.
Since this locking only covers the individual objects, it is expected that
the driver provide a higher level of locking around arrays of objects.
I considering putting an explicit lock on the virDomainObjListPtr array,
but this adds more complexity in the drivers, and does not improve
safety or concurrency.
* Domain lookup methods
virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms,
int id);
virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms,
const unsigned char *uuid);
virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
const char *name);
The driver must hold its global lock to protect the virDomainObjListPtr
object. The returned virDomainObjPtr instance will be locked at which
point the driver can optionall release its global lock
* Creating virDomainObjPtr instances
virDomainObjPtr virDomainAssignDef(virConnectPtr conn,
virDomainObjListPtr doms,
const virDomainDefPtr def);
The driver must hold its global lock to protect the virDomainObjListPtr
object. The newly created or cached virDomainObjPtr instance that is
returned will be locked.
* Removing a virDomainObjPtr instsance:
void virDomainRemoveInactive(virDomainObjListPtr doms,
virDomainObjPtr dom);
The driver must hold its global lock to protect the virDomaiNObjListPtr
object. The virDomainObjPtr must be unlocked.
For all the other methods in domain_conf.h, which deal with the config
in the virDomainDefPtr instances, the caller must hold a lock over the
virDomainObjPtr instance which owns the virDomainDefPtr.
This may all sound fairly onerous / complex, but it is straightforward
to work with in the drivers, as the following patches will demonstrate
Daniel
diff --git a/src/domain_conf.c b/src/domain_conf.c
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -145,40 +145,70 @@ VIR_ENUM_IMPL(virDomainHostdevSubsys, VI
__virReportErrorHelper(conn, VIR_FROM_DOMAIN, code, __FILE__, \
__FUNCTION__, __LINE__, fmt)
+/**
+ * @virDomainFindByID:
+ *
+ * Caller must hold exclusive lock over 'doms'
+ *
+ * Returns a virDomainObjPtr locked for exclusive access, or NULL
+ */
virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms,
int id)
{
unsigned int i;
- for (i = 0 ; i < doms->count ; i++)
+ for (i = 0 ; i < doms->count ; i++) {
+ virDomainLock(doms->objs[i]);
if (virDomainIsActive(doms->objs[i]) &&
doms->objs[i]->def->id == id)
return doms->objs[i];
+ virDomainUnlock(doms->objs[i]);
+ }
return NULL;
}
+/**
+ * @virDomainFindByUUID:
+ *
+ * Caller must hold exclusive lock over 'doms'
+ *
+ * Returns a virDomainObjPtr locked for exclusive access, or NULL
+ */
virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms,
const unsigned char *uuid)
{
unsigned int i;
- for (i = 0 ; i < doms->count ; i++)
+ for (i = 0 ; i < doms->count ; i++) {
+ virDomainLock(doms->objs[i]);
if (!memcmp(doms->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
return doms->objs[i];
+ virDomainUnlock(doms->objs[i]);
+ }
return NULL;
}
+/**
+ * @virDomainFindByName:
+ *
+ * Caller must hold exclusive lock over 'doms'
+ *
+ * Returns a virDomainObjPtr locked for exclusive access, or NULL
+ */
virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
const char *name)
{
unsigned int i;
- for (i = 0 ; i < doms->count ; i++)
+ for (i = 0 ; i < doms->count ; i++) {
+ virDomainLock(doms->objs[i]);
if (STREQ(doms->objs[i]->def->name, name))
return doms->objs[i];
+ virDomainUnlock(doms->objs[i]);
+ }
return NULL;
}
@@ -406,6 +436,12 @@ void virDomainDefFree(virDomainDefPtr de
VIR_FREE(def);
}
+/**
+ * @virDomainObjListFree
+ *
+ * Caller should not hold lock on 'dom', but must
+ * also ensure no other thread can have an active lock
+ */
void virDomainObjFree(virDomainObjPtr dom)
{
if (!dom)
@@ -419,17 +455,35 @@ void virDomainObjFree(virDomainObjPtr do
VIR_FREE(dom);
}
-void virDomainObjListFree(virDomainObjListPtr vms)
+/**
+ * @virDomainObjListFree
+ *
+ * Caller must hold exclusive lock over 'doms'
+ */
+void virDomainObjListFree(virDomainObjListPtr doms)
{
unsigned int i;
- for (i = 0 ; i < vms->count ; i++)
- virDomainObjFree(vms->objs[i]);
+ for (i = 0 ; i < doms->count ; i++) {
+ /* Lock+unlock ensures no one is still using this dom */
+ virDomainLock(doms->objs[i]);
+ virDomainUnlock(doms->objs[i]);
- VIR_FREE(vms->objs);
- vms->count = 0;
+ virDomainObjFree(doms->objs[i]);
+ }
+
+ VIR_FREE(doms->objs);
+ doms->count = 0;
}
+
+/**
+ * @virDomainAssignDef
+ *
+ * Caller must hold exclusive lock over 'doms'.
+ *
+ * The return domain object will be locked
+ */
virDomainObjPtr virDomainAssignDef(virConnectPtr conn,
virDomainObjListPtr doms,
const virDomainDefPtr def)
@@ -456,6 +510,7 @@ virDomainObjPtr virDomainAssignDef(virCo
domain->state = VIR_DOMAIN_SHUTOFF;
domain->def = def;
+ pthread_mutex_init(&domain->lock, NULL);
if (VIR_REALLOC_N(doms->objs, doms->count + 1) < 0) {
virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL);
@@ -466,13 +521,26 @@ virDomainObjPtr virDomainAssignDef(virCo
doms->objs[doms->count] = domain;
doms->count++;
+ virDomainLock(domain);
+
return domain;
}
+
+/**
+ * @virDomainRemoveInactive:
+ *
+ * Caller must hold exclusive lock over 'doms', but
+ * 'dom' must be unlocked
+ */
void virDomainRemoveInactive(virDomainObjListPtr doms,
virDomainObjPtr dom)
{
unsigned int i;
+
+ /* Ensure no other thread holds a lock on dom */
+ virDomainLock(dom);
+ virDomainUnlock(dom);
for (i = 0 ; i < doms->count ; i++) {
if (doms->objs[i] == dom) {
@@ -3335,8 +3403,10 @@ int virDomainLoadAllConfigs(virConnectPt
configDir,
autostartDir,
entry->d_name);
- if (dom)
+ if (dom) {
dom->persistent = 1;
+ virDomainUnlock(dom);
+ }
}
closedir(dir);
diff --git a/src/domain_conf.h b/src/domain_conf.h
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -452,6 +452,8 @@ typedef struct _virDomainObj virDomainOb
typedef struct _virDomainObj virDomainObj;
typedef virDomainObj *virDomainObjPtr;
struct _virDomainObj {
+ pthread_mutex_t lock;
+
int stdin_fd;
int stdout_fd;
int stderr_fd;
@@ -481,6 +483,20 @@ virDomainIsActive(virDomainObjPtr dom)
virDomainIsActive(virDomainObjPtr dom)
{
return dom->def->id != -1;
+}
+
+
+static inline int
+virDomainLock(virDomainObjPtr dom)
+{
+ return pthread_mutex_lock(&dom->lock);
+}
+
+
+static inline int
+virDomainUnlock(virDomainObjPtr dom)
+{
+ return pthread_mutex_unlock(&dom->lock);
}
--
|: 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