Re: [libvirt] [PATCH v2] Remove qemuDriverLock from almost everywhere

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; With the majority of fields in the virQEMUDriverPtr struct now immutable or self-locking, there is no need for practically any methods to be using the QEMU driver lock. Only a handful of helper APIs in qemu_conf.c now need it --- src/qemu/THREADS.txt | 194 +++-------------------- src/qemu/qemu_conf.c | 50 ++++-- src/qemu/qemu_domain.c | 213 +++++-------------------- src/qemu/qemu_domain.h | 40 +---- src/qemu/qemu_driver.c | 386 +++++++++++----------------------------------- src/qemu/qemu_hotplug.c | 118 +++++++------- src/qemu/qemu_migration.c | 66 ++++---- src/qemu/qemu_process.c | 223 +++++++++----------------- src/qemu/qemu_process.h | 3 +- 9 files changed, 360 insertions(+), 933 deletions(-) diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt index c3bad21..785be99 100644 --- a/src/qemu/THREADS.txt +++ b/src/qemu/THREADS.txt @@ -14,35 +14,24 @@ Basic locking primitives There are a number of locks on various objects - * struct qemud_driver: RWLock + * virQEMUDriverPtr - This is the top level lock on the entire driver. Every API call in - the QEMU driver is blocked while this is held, though some internal - callbacks may still run asynchronously. This lock must never be held - for anything which sleeps/waits (i.e. monitor commands) + The qemu_conf.h file has inline comments describing the locking + needs for each field. Any field marked immutable, self-locking + can be accessed without the driver lock. For other fields there + are typically helper APIs in qemu_conf.c that provide serialized + access to the data. No code outside qemu_conf.c should ever + acquire this lock

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4917721..ec4c8f8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -983,9 +938,17 @@ qemuDomainObjAbortAsyncJob(virDomainObjPtr obj) priv->job.asyncAbort = true; } +/* + * obj must be locked before calling + * + * To be called immediately before any QEMU monitor API call + * Must have already either called qemuDomainObjBeginJob() and checked + * that the VM is still active; may not be used for nested async jobs. + * + * To be followed with qemuDomainObjExitMonitor() once complete + */