[libvirt] [PATCH] Recheck disk backing chains after snapshot

[John Ferlan]

On 04/25/2014 09:13 AM, Jiri Denemark wrote:
> When a snapshot operation finishes we have to recheck the backing chain
> of all disks involved in the snapshot. And we need to do that even if
> the operation failed because some of the disks might have changed if
> QEMU did not support transactions.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
> 
> Notes:
>     - BlockRebase and BlockCommit already recheck the backing chain when we
>       get an event about a completed block job (in qemuProcessHandleBlockJob)
> 
>  src/qemu/qemu_driver.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 

FYI: This change fixes an issue that virt-test found over my
weekend run in the 'blockpull' test which was caused by 4/4 of
the addressing backing stores by index:

http://www.redhat.com/archives/libvir-list/2014-April/msg00823.html

Essentially the test sets things up via:

/usr/bin/virsh snapshot-create-as virt-tests-vm1 snapshot1 snap1-desc --disk-only --atomic --no-metadata vda,snapshot=external,file=/home/virt-test/tmp/jeos-20-64.snap1
/usr/bin/virsh snapshot-create-as virt-tests-vm1 snapshot2 snap2-desc --disk-only --atomic --no-metadata vda,snapshot=external,file=/home/virt-test/tmp/jeos-20-64.snap2
/usr/bin/virsh snapshot-create-as virt-tests-vm1 snapshot3 snap3-desc --disk-only --atomic --no-metadata vda,snapshot=external,file=/home/virt-test/tmp/jeos-20-64.snap3

Then does (in different passes):

/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --base /home/virt-test/tmp/jeos-20-64.snap2
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --base /home/virt-test/tmp/jeos-20-64.snap1
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --timeout 1
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --timeout 1 --base /home/virt-test/tmp/jeos-20-64.snap2
/usr/bin/virsh blockpull virt-tests-vm1 vda --wait --verbose --timeout 1 --base /home/virt-test/tmp/jeos-20-64.snap1

Without this patch applied, the blockpull's w/ --base supplied fail
with either:

could not find image '/home/virt-test/tmp/jeos-20-64.snap2' in chain for '/home/virt-test/tmp/jeos-20-64.snap3'

or

could not find image '/home/virt-test/tmp/jeos-20-64.snap1' in chain for '/home/virt-test/tmp/jeos-20-64.snap3'


John

FWIW: The weekend run I had w/ virt-test against recent upstream
changes was quite successful. There's still an issue or two to
resolve regarding how capacity is checked, but the recent storage 
changes haven't caused regressions.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 67ba487..492fcc1 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12942,6 +12942,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
>      bool persist = false;
>      bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    virErrorPtr orig_err = NULL;
>  
>      if (!virDomainObjIsActive(vm)) {
>          virReportError(VIR_ERR_OPERATION_INVALID,
> @@ -13033,6 +13034,17 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver,
>      }
>  
>   cleanup:
> +    /* recheck backing chains of all disks involved in the snapshot */
> +    orig_err = virSaveLastError();
> +    for (i = 0; i < snap->def->ndisks; i++) {
> +        if (snap->def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)
> +            continue;
> +        qemuDomainDetermineDiskChain(driver, vm, vm->def->disks[i], true);
> +    }
> +    if (orig_err) {
> +        virSetError(orig_err);
> +        virFreeError(orig_err);
> +    }
>  
>      if (ret == 0 || !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION)) {
>          if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0 ||
>