[libvirt] [PATCH 2/5] qemu: snapshot: Fix job handling when creating snapshots

[Eric Blake]

On 09/03/2014 07:53 AM, Peter Krempa wrote:
> Creating snapshots modifies the domain state. Currently we wouldn't
> enter the job for certain operations although they would modify the
> state. Refactor job handling so that everything is covered by an async
> job.
> ---
>  src/qemu/qemu_driver.c | 176 ++++++++++++++++++++++---------------------------
>  1 file changed, 78 insertions(+), 98 deletions(-)
> 

> @@ -13466,10 +13430,19 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>          goto cleanup;
>      }
> 
> +    /* We are going to modify the domain below. Internal snapshots would use
> +     * a regular job, so we need to set the job mask to disallow query as
> +     * 'savevm' blocks the monitor. External snapshot will then modify the
> +     * job mas appropriately. */

s/mas/mask/


> + endjob:
> +    if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
> +        if (qemuDomainSnapshotWriteMetadata(vm, snap,
> +                                            cfg->snapshotDir) < 0) {
> +            /* if writing of metadata fails, error out rather than trying
> +             * to silently carry on  without completing the snapshot */

While reindenting this, you could also fix the accidental double space
that I stuck in there.

> +
> +    if (!qemuDomainObjEndAsyncJob(driver, vm)) {
> +        /* Only possible if a transient vm quit while our locks were down,
> +         * in which case we don't want to save snapshot metadata. */

This comment is now out of date, because you placed it below the point
where you already saved the metadata.

But overall, looks like the right thing to do - you widened the overall
job to cover everything, rather than grabbing the job inside helper
functions.  ACK with the nits cleaned up.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

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