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

[Nikolay Shirokovskiy]

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