On 05.06.2019 12:09, Michal Privoznik wrote:
> In some cases, caller of qemuDomainObjListAdd() tries to acquire
> MODIFY job after the call. Let's adjust qemuDomainObjListAdd() so
> that it will keep the job set upon return (if requested by
> caller).
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_domain.c | 17 ++++++++++++++---
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_driver.c | 24 +++++++++++-------------
> src/qemu/qemu_migration.c | 2 +-
> 4 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f6b677c69e..b0b3fa5fd8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7061,6 +7061,7 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = {
> * @def: domain definition
> * @oldDef: previous domain definition
> * @live: whether @def is live definition
> + * @keepJob: whether to leave MODIFY job set on returned object
> * @flags: an bitwise-OR of virDomainObjListAdd flags
> *
> * Add a domain onto the list of domain object and sets its
> @@ -7070,6 +7071,10 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = {
> * In addition to that, if definition of an existing domain is
> * changed a MODIFY job is acquired prior to that.
> *
> + * If @keepJob is true, then the MODIFY job is not ended upon
> + * successful return from this function. This might be handy if
> + * caller would try to acquire the job anyway.
> + *
> * Returns: domain object pointer on success,
> * NULL otherwise.
> */
> @@ -7078,9 +7083,11 @@ qemuDomainObjListAdd(virQEMUDriverPtr driver,
> virDomainDefPtr def,
> virDomainDefPtr *oldDef,
> bool live,
> + bool keepJob,
> unsigned int flags)
> {
> virDomainObjPtr vm = NULL;
> + bool defSet = false;
>
> if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, flags)))
> return NULL;
> @@ -7091,7 +7098,9 @@ qemuDomainObjListAdd(virQEMUDriverPtr driver,
> * just set the definition without acquiring job. */
> if (!vm->def) {
> virDomainObjAssignDef(vm, def, live, oldDef);
> - VIR_RETURN_PTR(vm);
> + defSet = true;
> + if (!keepJob)
> + VIR_RETURN_PTR(vm);
> }
Just realized. If we got in this branch and have @keepJob = true
and later fail to aqcuire job and remove vm entirely then we
free @def which is unexpected by callers.
Nikolay
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
