[libvirt] [PATCH] qemu: update disk chain in vm->newDef when block job compeleted

[Peter Krempa]

On 11/10/14 05:15, weiwei li wrote:
> A bug will be hitted with libvirt+qemu+kvm when doing follow steps:
> 1.initial status :vm = base+z(z is a snapshot)
> 2. call snapshotCreateXML( ): vm =  base+z +z'
> 3. call blockRebase( ): vm = base+z''(z rebase to z')
> 4.(after rebasing work done) vm shutdown and then start
> 
> an error occured like this:
> "unable to set user and group to '0:0' on '
> /path/to/disk': No such file or directory
> 
> A second start may sucess.
> 
> The error message reported by virSecurityDACSetSecurityDiskLabel
> when the disk chain is misused(that is base+z +z', after rebase,
> but z' was deleted yet).virSecurityDACSetSecurityDiskLabel used
> an old disk chain after rebasing.
> In qemuProcessHandleBlockJob when rebase complete, the disk chain
> in vm->def is updated but vm->newDef is not(when vm->newDef is not
> null). In shutdown( actrully in qemuProcessStop ), if vm->newDef
> is not null , vm->def will be replaced  by vm->newDef, which
> contains a wrong disk chain!!! Then the next start will failed with
> above message.
> To fix it, update disk chain in vm->newDef in qemuProcessHandleBlockJob.
> 
> Signed-off-by: Weiwei Li <nuonuoli at tencent.com
> <mailto:nuonuoli at tencent.com>>
> ---
>  src/qemu/qemu_process.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)

Since commit:

commit fe7ef7b112b3b4d6f9c9edf499a79683fb0b7edb
Author:     Peter Krempa <pkrempa at redhat.com>
AuthorDate: Thu Sep 25 17:30:28 2014 +0200
Commit:     Peter Krempa <pkrempa at redhat.com>
CommitDate: Fri Sep 26 09:35:33 2014 +0200

    qemu: Always re-detect backing chain
    
    Since 363e9a68 we track backing chain metadata when creating snapshots
    the right way even for the inactive configuration. As we did not yet
    update other code paths that modify the backing chain (blockpull) the
    newDef backing chain gets out of sync.
    
    After stopping of a VM the new definition gets copied to the next start
    one. The new VM then has incorrect backing chain info. This patch
    switches the backing chain detector to always purge the existing backing
    chain and forces re-detection to avoid this issue until we'll have full
    backing chain tracking support.
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1144922

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 11145d1..76fccce 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2540,7 +2540,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
             virFileExists(virDomainDiskGetSource(disk)))
             continue;

-        if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) >= 0)
+        if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0)
             continue;

         if (disk->startupPolicy &&

thus your symptoms shouldn't show up. Could you please verify that the situation you've described happens 




> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 24e5f0f..0491249 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1033,6 +1033,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon
> ATTRIBUTE_UNUSED,
>      virDomainDiskDefPtr disk;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      virDomainDiskDefPtr persistDisk = NULL;
> +    virDomainDiskDefPtr newDisk = NULL;
>      bool save = false;
>  
>      virObjectLock(vm);
> @@ -1101,6 +1102,32 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon
> ATTRIBUTE_UNUSED,
>              disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
>              ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk,
>                                                        true, true));
> +            /* Chang vm->newDef disk chain  */
> +            if (!persistDisk && vm->newDef){
> +                int indx = virDomainDiskIndexByName(vm->newDef,
> +                                                disk->dst,            
>        
> +                                                false);                
>        
> +                virStorageSourcePtr copy = NULL;
> +
> +                if (indx >=0){  
> +                    newDisk = vm->newDef->disks[indx];
> +                    copy = virStorageSourceCopy(disk->src, false);
> +                    if (virStorageSourceInitChainElement(copy,
> +                                                         newDisk->src,
>                  
> +                                                         false) < 0) {
>                  
> +                            VIR_WARN("Unable to update newDef definition "
> +                                     "on vm %s after block job",    
> +                                     vm->def->name);                
> +                            virStorageSourceFree(copy);    
> +                            copy = NULL;                  
> +                            newDisk = NULL;                
> +                    }
> +                }
> +                if (copy){
> +                    virStorageSourceFree(newDisk->src);
> +                    newDisk->src = copy;          
> +                }
> +            }
>          } else if (disk->mirror &&
>                     (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY ||
>                      type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) {
> -- 
> 1.7.1
> 
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141202/35cf6ad9/attachment-0001.sig>