[libvirt] [PATCH v2 06/11] qemu: Grab modify job for changing domain XML
Michal Privoznik
mprivozn at redhat.com
Wed Jun 5 09:35:19 UTC 2019
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 at redhat.com>
>> ---
>> 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
More information about the libvir-list
mailing list