Since a9e97e0c we don't require driver->config, or virQEMUDriverGetConfig() to be called with locked QEMU driver. In fact, calling it with the driver locked would cause deadlock. Hence, the annotation to the struct field is not right. --- src/qemu/qemu_conf.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 8229cfc..fd5cc57 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -160,8 +160,7 @@ struct _virQEMUDriverConfig { struct _virQEMUDriver { virMutex lock; - /* Require lock to get reference on 'config', - * then lockless thereafter */ + /* Immutable pointer, self-locking APIs */ virQEMUDriverConfigPtr config;

On 17.07.2013 11:46, Daniel P. Berrange wrote: > On Tue, Jul 16, 2013 at 08:25:25PM +0200, Michal Privoznik wrote: >> Since a9e97e0c we don't require driver->config, or >> virQEMUDriverGetConfig() to be called with locked QEMU driver. In fact, >> calling it with the driver locked would cause deadlock. Hence, the >> annotation to the struct field is not right. >> --- >> src/qemu/qemu_conf.h | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h >> index 8229cfc..fd5cc57 100644 >> --- a/src/qemu/qemu_conf.h >> +++ b/src/qemu/qemu_conf.h >> @@ -160,8 +160,7 @@ struct _virQEMUDriverConfig { >> struct _virQEMUDriver { >> virMutex lock; >> >> - /* Require lock to get reference on 'config', >> - * then lockless thereafter */ >> + /* Immutable pointer, self-locking APIs */ >> virQEMUDriverConfigPtr config; > > Actually, it is 'immutable pointer, immutable+lockless data', since > we don't require any locks to be held in order to access fields in > the config object. > > Regards, > Daniel > Consider this squashed in: diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 37d2ba6..ca8a39e 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -357,7 +357,7 @@ static int lxcContainerSetID(virDomainDefPtr def) VIR_DEBUG("Set UID/GID to 0/0"); if (def->idmap.nuidmap && - virSetUIDGID(0, 0, groups, ngroups) < 0) { + virSetUIDGID(0, 0, NULL, 0) < 0) { virReportSystemError(errno, "%s", _("setuid or setgid failed")); return -1;