On Fri, Feb 01, 2013 at 11:18:27 +0000, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Switch virDomainObjList to inherit from virObjectLockable and
> make all the APIs acquire/release the mutex when running. This
> makes virDomainObjList completely self-locking and no longer
> reliant on the hypervisor driver locks
> ---
> src/conf/domain_conf.c | 75 ++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 61 insertions(+), 14 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 79da5eb..a1e899f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -60,7 +60,7 @@ verify(VIR_DOMAIN_VIRT_LAST <= 32);
>
>
> struct _virDomainObjList {
> - virObject parent;
> + virObjectLockable parent;
>
> /* uuid string -> virDomainObj mapping
> * for O(1), lockless lookup-by-uuid */
> @@ -715,7 +715,7 @@ static int virDomainObjOnceInit(void)
> virDomainObjDispose)))
> return -1;
>
> - if (!(virDomainObjListClass = virClassNew(virClassForObject(),
> + if (!(virDomainObjListClass = virClassNew(virClassForObjectLockable(),
> "virDomainObjList",
> sizeof(virDomainObjList),
> virDomainObjListDispose)))
These two hunks should go to 3/13.
...
> @@ -1880,25 +1886,32 @@ void virDomainObjAssignDef(virDomainObjPtr domain,
> * current def is Live
> *
> */
> -virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
> - virCapsPtr caps,
> - const virDomainDefPtr def,
> - unsigned int flags,
> - virDomainDefPtr *oldDef)
> +static virDomainObjPtr
> +virDomainObjListAddLocked(virDomainObjListPtr doms,
> + virCapsPtr caps,
> + const virDomainDefPtr def,
> + unsigned int flags,
> + virDomainDefPtr *oldDef)
> {
> virDomainObjPtr vm;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> if (oldDef)
> *oldDef = false;
>
> + virUUIDFormat(def->uuid, uuidstr);
> +
> /* See if a VM with matching UUID already exists */
> - if ((vm = virDomainObjListFindByUUID(doms, def->uuid))) {
> + if ((vm = virHashLookup(doms->objs, uuidstr))) {
> + virObjectLock(vm);
> /* UUID matches, but if names don't match, refuse it */
> if (STRNEQ(vm->def->name, def->name)) {
> virUUIDFormat(vm->def->uuid, uuidstr);
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("domain '%s' is already defined with uuid %s"),
> vm->def->name, uuidstr);
> + virObjectUnlock(vm);
> + vm = NULL;
These two lines should go to 4/13.
> goto cleanup;
> }
>
> @@ -1908,6 +1921,8 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
> virReportError(VIR_ERR_OPERATION_INVALID,
> _("domain is already active as '%s'"),
> vm->def->name);
> + virObjectUnlock(vm);
> + vm = NULL;
> goto cleanup;
> }
> }
And this hunk should go to 4/13 too.
> @@ -1934,12 +1949,11 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms,
> }
> } else {
> /* UUID does not match, but if a name matches, refuse it */
> - if ((vm = virDomainObjListFindByName(doms, def->name))) {
> + if ((vm = virHashSearch(doms->objs, virDomainObjListSearchName, def->name))) {
I believe you wanted to add virObjectLock(vm) here rather than...
> virUUIDFormat(vm->def->uuid, uuidstr);
> virReportError(VIR_ERR_OPERATION_FAILED,
> _("domain '%s' already exists with uuid %s"),
> def->name, uuidstr);
> - virObjectUnlock(vm);
...removing this unlock here.
> vm = NULL;
> goto cleanup;
> }
...
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list