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. Nikolay