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

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



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


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