[libvirt] [PATCH 3/3] qemu: Split if condition in qemuDomainSnapshotUndoSingleDiskActive

Michal Privoznik mprivozn at redhat.com
Fri Apr 13 09:12:54 UTC 2012


Since compilers are trying to optimize code they are allowed to
reorder evaluation of conditions in if statement (okay, not in all
cases, but they can in this one). Therefore if we do:
    if (stat(file, &st) == 0 && unlink(file) < 0)
after compiler chews this it may get feeling that swapping order
is a good idea. However, we obviously don't want to call stat()
on just unlink()-ed file.
---
 src/qemu/qemu_driver.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 65ed290..037d45c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9998,9 +9998,14 @@ qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver,
         VIR_WARN("Unable to restore security label on %s", disk->src);
     if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
         VIR_WARN("Unable to release lock on %s", disk->src);
+
+    /* Deliberately do not join these two ifs. Compiler may mix up
+     * the order of evaluation so unlink() may proceed stat()
+     * which is not what we want */
     if (need_unlink && stat(disk->src, &st) == 0 &&
-        st.st_size == 0 && S_ISREG(st.st_mode) && unlink(disk->src) < 0)
-        VIR_WARN("Unable to remove just-created %s", disk->src);
+        st.st_size == 0 && S_ISREG(st.st_mode))
+        if (unlink(disk->src) < 0)
+            VIR_WARN("Unable to remove just-created %s", disk->src);
 
     /* Update vm in place to match changes.  */
     VIR_FREE(disk->src);
-- 
1.7.8.5




More information about the libvir-list mailing list