On 6/5/19 11:32 AM, Peter Krempa wrote:
On Wed, Jun 05, 2019 at 11:09:14 +0200, Michal Privoznik wrote:
Changing domain definition must be guarded with acquiring modify
job. The problem is if there is a thread executing say
qemuDomainSetMemoryStatsPeriod() which is an API that acquires
modify job and then possibly unlock the domain object and locks
it again. However, becasue virDomainObjAssignDef() does not take
a job (only object lock) it may have changed the domain
definition while the other thread unlocked the domain object in
order to talk on the monitor. For instance:
Thread1:
1) lookup domain object and lock it
2) acquire job
3) get pointers to live and persistent defs
4) unlock the domain object
5) talk to qemu on the monitor
Thread2 (Execute qemuDomainDefineXMLFlags):
1) lookup domain object and lock it
2) virDomainObjAssignDef() which overwrites persistent def and
frees the old one
3) unlock domain object
Thread1:
6) lock the domain object again
7) try to access the persistent def via pointer stored in 3)
Now, this will crash because the pointer from step 3) points to a
memory that was freed.
However, if Thread2 waited and acquired modify job (just like
Thread1) then these two would serialize and no harm would be
done.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/qemu/qemu_domain.c | 55 +++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 6 +++++
src/qemu/qemu_driver.c | 27 ++++++-------------
src/qemu/qemu_migration.c | 5 +---
4 files changed, 70 insertions(+), 23 deletions(-)
[...]
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8bbac339e0..fa93a686b7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1701,12 +1701,9 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn,
if (virDomainCreateXMLEnsureACL(conn, def) < 0)
goto cleanup;
- if (!(vm = virDomainObjListAdd(driver->domains, def,
- driver->xmlopt,
- VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE)))
+ if (!(vm = qemuDomainObjListAdd(driver, def, NULL, true,
+ VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE)))
goto cleanup;
-
- virDomainObjAssignDef(vm, def, true, NULL);
def = NULL;
if (qemuProcessBeginJob(driver, vm, VIR_DOMAIN_JOB_OPERATION_START,
@@ -2405,6 +2402,7 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period,
qemuDomainObjEnterMonitor(driver, vm);
r = qemuMonitorSetMemoryStatsPeriod(priv->mon, def->memballoon, period);
+ sleep(60);
Debugging leftovers?
Oh, right O:-)
Consider removed.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
