Re: [Qemu-devel] [libvirt] [PATCH] snapshot: fix rollback failure in transaction mode

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

On 09/12/2012 09:22 AM, Guannan Ren wrote:
> After failure of qemu transaction command, the snapshot file still
> be there with non-zero in size. In order to unlink the file, the
> patch removes the file size checking.

Can you give some exact steps to reproduce this, so that I know who is
making the file have non-zero size?  I'm worried that unlinking a
non-empty file is discarding data, so the commit message needs a lot
more details about how we are proving that the only way the file can be
non-zero size is because qemu happened to put data into a previously
empty file prior to the failed 'transaction' attempt.

That is, after re-reading context code just now, I'm fairly confident
that this code can only be reached when qemu supports the 'transaction'
monitor command, and libvirt's --reuse-ext flag was not specified, and
therefore libvirt must have just created the file.  But I want that in
the commit message rather than having to re-read the code every time I
visit this commit in future reads of the git log.  It may also be that
qemu has a bug in that the 'transaction' command is modifying files even
when it fails, so even while this works around the bug, I'm cc'ing Jeff
to see if qemu also needs a bug fix.

> ---
>  src/qemu/qemu_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 8e8e00c..1fedfb8 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10833,7 +10833,7 @@ qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver,
>      if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
>          VIR_WARN("Unable to release lock on %s", disk->src);
>      if (need_unlink && stat(disk->src, &st) == 0 &&
> -        st.st_size == 0 && S_ISREG(st.st_mode) && unlink(disk->src) < 0)
> +        S_ISREG(st.st_mode) && unlink(disk->src) < 0)
>          VIR_WARN("Unable to remove just-created %s", disk->src);
>  
>      /* Update vm in place to match changes.  */
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]