[libvirt] [PATCH] qemu: remove dead assignment
Detected by clang. * src/qemu/qemu_migration.c (qemuMigrationToFile): Nothing later uses is_reg. --- src/qemu/qemu_migration.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7f4b111..6c5bf66 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1311,7 +1311,6 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, if (virSecurityManagerSetFDLabel(driver->securityManager, vm, compressor ? pipeFD[1] : fd) < 0) goto cleanup; - is_reg = true; bypassSecurityDriver = true; } else { /* Phooey - we have to fall back on exec migration, where qemu -- 1.7.4.4
On 05/03/2011 05:08 PM, Eric Blake wrote:
Detected by clang.
* src/qemu/qemu_migration.c (qemuMigrationToFile): Nothing later uses is_reg. --- src/qemu/qemu_migration.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7f4b111..6c5bf66 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1311,7 +1311,6 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, if (virSecurityManagerSetFDLabel(driver->securityManager, vm, compressor ? pipeFD[1] : fd)< 0) goto cleanup; - is_reg = true; bypassSecurityDriver = true; } else { /* Phooey - we have to fall back on exec migration, where qemu
ACK, but I wonder if this code used to live in a function where is_reg *was* used below, was moved into this helper function, and now the function that's calling it is doing the wrong thing afterwards because it's expecting is_reg to be set to true (and it's not because it was passed by value rather than reference). It *seems* not (based on the two places qemuMigrationToFile() is called), but just the fact that this was there in the first place raised my eyebrow so I thought I should mention it...
On 05/03/2011 10:47 PM, Laine Stump wrote:
+++ b/src/qemu/qemu_migration.c @@ -1311,7 +1311,6 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, if (virSecurityManagerSetFDLabel(driver->securityManager, vm, compressor ? pipeFD[1] : fd)< 0) goto cleanup; - is_reg = true; bypassSecurityDriver = true; } else { /* Phooey - we have to fall back on exec migration, where qemu
ACK, but I wonder if this code used to live in a function where is_reg *was* used below, was moved into this helper function, and now the function that's calling it is doing the wrong thing afterwards because it's expecting is_reg to be set to true (and it's not because it was passed by value rather than reference).
Seeing as how I introduced the bug in the first place, here's more details :) commit 6034ddd moved the code into qemu_migration.c; there, a single read of !is_reg was the condition used to gate whether an attempt was made to do cgroup ACL labelling on a non-regular file. Then the next commit, 34fa0de, sunk the original !is_reg read into an else clause, while adding an if clause for the case when fd: migration is possible. That was the commit that introduced the dead store to is_reg in the if clause. It was leftovers from when I rebased my series to move the code into qemu_migration.c in the first place; in my original proposal, at https://www.redhat.com/archives/libvir-list/2011-March/msg00435.html (or maybe in earlier non-posted trials), I had been using the setting of is_reg to true as a key to skip later cgroup cleanup within the consolidated migration helper code. Either way, the caller, in qemudDomainSaveFlag in qemu_driver.c, really does not want to see any internal changes to is_reg, because that gates whether it does an unlink() on failure. I've pushed it as is, and don't see the need for any future changes. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Laine Stump