Re: [libvirt] [PATCH v3 05/13] Make virDomainObjList self-locking via virObjectLockable

On Mon, Feb 04, 2013 at 05:22:59PM +0100, Jiri Denemark wrote: > On Fri, Feb 01, 2013 at 11:18:27 +0000, Daniel P. Berrange wrote: > > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > > > 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

> > @@ -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. Yes, that is correct. I've made that change Given that all other comments are just moving chunks to previous patches, do you want to see a v2 ?