Re: [libvirt] [PATCH 0/9] Grab modify job for changing domain XML

On 30.05.2019 12:33, Michal Privoznik wrote: > This is an alternative proposal to: > > https://www.redhat.com/archives/libvir-list/2019-May/msg00830.html > > The problem I'm trying to fix is described here: > > https://www.redhat.com/archives/libvir-list/2019-May/msg00810.html > > Michal Prívozník (9): > virDomainObjListAddLocked: Drop useless @cleanup label > virDomainObjListAddObjLocked: Don't expect vm->def to be set > virDomainObjListAddLocked: Set vm->def only in success path > virDomainObjIsActive: Allow vm->def to be NULL > virDomainObjListAdd: Leave def assigning as an exercise for caller > qemu: Allow vm->def == NULL in job control APIs > qemu: Grab modify job for changing domain XML > lxc: Grab modify job for changing domain XML > libxl: Grab modify job for changing domain XML > > src/bhyve/bhyve_driver.c | 10 +++++--- > src/conf/domain_conf.h | 2 +- > src/conf/virdomainobjlist.c | 48 ++++++++++++++---------------------- > src/conf/virdomainobjlist.h | 3 +-- > src/libxl/libxl_domain.c | 3 ++- > src/libxl/libxl_driver.c | 48 ++++++++++++++++++++++++------------ > src/libxl/libxl_migration.c | 14 +++++------ > src/lxc/lxc_domain.c | 3 ++- > src/lxc/lxc_driver.c | 23 +++++++++++------ > src/openvz/openvz_conf.c | 12 ++++----- > src/openvz/openvz_driver.c | 17 +++++++------ > src/qemu/qemu_domain.c | 30 ++++++++++++++--------- > src/qemu/qemu_driver.c | 49 ++++++++++++++++++++++++++----------- > src/qemu/qemu_migration.c | 13 +++++++--- > src/test/test_driver.c | 21 +++++++++------- > src/vmware/vmware_conf.c | 4 +-- > src/vmware/vmware_driver.c | 9 +++---- > src/vz/vz_sdk.c | 4 ++- > 18 files changed, 183 insertions(+), 130 deletions(-) > In patches 8 and 9 there are same issues as in qemu part patches. In order to overcome NULL names in acquire job and remove inactive domain functions I suggest using next approach: if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, 0))) goto cleanup; if (!vm->def) { virDomainObjAssignDef(vm, def, false, NULL); def = NULL; } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) { qemuDomainRemoveInactive(driver, vm); goto cleanup; } if (def) { virDomainObjAssignDef(vm, def, false, &oldDef); def = NULL; } Then we don't need other patches that support case when vm->def == NULL. First assign can be moved to virDomainObjListAddLocked to the branch where new object is created although virDomainObjListAdd will have a bit weird semantics in this case.