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

Re: [libvirt] [PATCH] qemu: remove dead assignment



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

Attachment: signature.asc
Description: OpenPGP digital signature


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