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 <[email protected]> > --- > 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?
> if (qemuDomainObjExitMonitor(driver, vm) < 0)
> goto endjob;
> if (r < 0) {
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
