Re: [PATCH] snapshot: improve qemu handling of reused snapshot targets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/20/2012 03:29 PM, Eric Blake wrote:
> The oVirt developers have stated that the real reasons they want
> to have qemu reuse existing volumes when creating a snapshot are:
> 1. the management framework is set up so that creation has to be
> done from a central node for proper resource tracking, and having
> libvirt and/or qemu create things violates the framework, and
> 2. qemu defaults to creating snapshots with an absolute path to
> the backing file, but oVirt wants to manage a backing chain that
> uses just relative names, to allow for easier migration of a chain
> across storage locations.
> 

> 
>      /* create the stub file and set selinux labels; manipulate disk in
>       * place, in a way that can be reverted on failure. */
> -    fd = qemuOpenFile(driver, source, O_WRONLY | O_TRUNC | O_CREAT,
> +    fd = qemuOpenFile(driver, source,
> +                      O_WRONLY | (reuse ? 0 : O_TRUNC | O_CREAT),
>                        &need_unlink, NULL);
>      if (fd < 0)
>          goto cleanup;

I just realized this is a pointless open in the reuse case, since the
very next statement closes the fd (in other words, we are using open for
the side-effect of creation, but reuse says no creation is necessary).
I plan on squashing this:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index ca81e75..4286b2a 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -9884,12 +9884,13 @@ qemuDomainSnapshotCreateSingleDiskActive(struct
qemud_driver *driver,

     /* create the stub file and set selinux labels; manipulate disk in
      * place, in a way that can be reverted on failure. */
-    fd = qemuOpenFile(driver, source,
-                      O_WRONLY | (reuse ? 0 : O_TRUNC | O_CREAT),
-                      &need_unlink, NULL);
-    if (fd < 0)
-        goto cleanup;
-    VIR_FORCE_CLOSE(fd);
+    if (!reuse) {
+        fd = qemuOpenFile(driver, source, O_WRONLY | O_TRUNC | O_CREAT,
+                          &need_unlink, NULL);
+        if (fd < 0)
+            goto cleanup;
+        VIR_FORCE_CLOSE(fd);
+    }

     origsrc = disk->src;
     disk->src = source;

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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]