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

[Peter Krempa]

On 12/02/14 17:05, Peter Krempa wrote:
> 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 
> 

Oops I've hit send before finishing the message.

I want to ask whether this still reproduces with current upstream as the
commit mentioned above should avoid issue like you've described.

Additionally the patch does not apply cleanly now, so please repost a
 rebased version.

Thanks.

Peter


-------------- 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/d07821d9/attachment-0001.sig>