[PATCH 1/1] qemu_domain: fix vcpu hotunplug race by locking vcpupriv
multiple parallel timedout vcpu hotunplug requests coming from qemu handled asynchronously by libvirt can cause data corruption as they modify vcpu data during refresh by libvirt. Lock each vcpu before modification and then release the lock Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com> --- src/qemu/qemu_domain.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3469f0d40c59..15ec1b0a771a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10084,6 +10084,7 @@ qemuDomainRefreshVcpuInfo(virDomainObj *vm, for (i = 0; i < maxvcpus; i++) { vcpu = virDomainDefGetVcpu(vm->def, i); vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu); + virObjectLock(vcpupriv); if (validTIDs) vcpupriv->tid = info[i].tid; @@ -10113,6 +10114,7 @@ qemuDomainRefreshVcpuInfo(virDomainObj *vm, else vcpu->hotpluggable = VIR_TRISTATE_BOOL_NO; } + virObjectUnlock(vcpupriv); } ret = 0; -- 2.39.3
On Wed, Apr 24, 2024 at 14:31:10 +0530, Shaleen Bathla via Devel wrote:
multiple parallel timedout vcpu hotunplug requests coming from qemu handled asynchronously by libvirt can cause data corruption as they modify vcpu data during refresh by libvirt.
Could you please elaborate? which two code paths hit this? Could you please provide stack traces?
Lock each vcpu before modification and then release the lock
Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com> --- src/qemu/qemu_domain.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3469f0d40c59..15ec1b0a771a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10084,6 +10084,7 @@ qemuDomainRefreshVcpuInfo(virDomainObj *vm, for (i = 0; i < maxvcpus; i++) { vcpu = virDomainDefGetVcpu(vm->def, i); vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu); + virObjectLock(vcpupriv);
'qemuDomainVcpuPrivate' is a 'virObject' not a 'virObjectLockable' so this doesn't work. In fact it does nothing as virObjectLock is a no-op (apart from printing a warning of incorrect use) if the object is not lockable. The vcpu private data are supposed to be guarded by the lock on the domain object along with a modification job.
participants (2)
-
Peter Krempa -
Shaleen Bathla