[libvirt] [PATCH] qemu: Only restore security label when saving is successfull.

[Eric Blake]

On 03/27/2011 10:16 PM, Osier Yang wrote:
> 于 2011年03月25日 23:54, Eric Blake 写道:
>> On 03/25/2011 02:54 AM, Osier Yang wrote:
>>> "qemudDomainSaveFlag" trys to restore security label even if
>>> the saving fails, a useless warning will be thowed then, e.g.
>>> if "doStopVcpus" fails.
>>>
>>> * src/qemu/qemu_driver.c
>>> ---

>>>       virCgroupPtr cgroup = NULL;
>>> +    bool saved = false;
>>
>> You don't need this if we can key off of some other condition.
>>
>>>

> No, that's not the original patch meant, and this attached patch
> doesn't fix the problem.
> 
>     /* Pause */
>     if (vm->state == VIR_DOMAIN_RUNNING) {
>         header.was_running = 1;
>         if (qemuProcessStopCPUs(driver, vm) < 0)
>             goto endjob;
> 
> The original patch meant:
> 
> In case of "qemuProcessStopCPUs" failure, (in RHEL, it's
> "doStopVcpus"), which means VM is still not NULL, and
> "bypassSecurityDriver" is never changed before, (it's initialized
> as 0 at the beginning), so, when it jumps to "endjob", it
> must try to restore the label for the saving path, however,
> it even didn't try to save (even no label setting before), as
> a result, it will always warn "No such file or directory, bla bla".
> 
> So changing "bypassSecurityDriver" after the first restoring
> attempt doesn't work.

Aha - I see what you mean.  We had _two_ logic bugs, and I only fixed
the second one (we restored the label twice on a late error), whereas
you were trying to fix the first (we restore the label even if we never
set it in the first place on an early error).

Meanwhile, my fd: migration already touched this file, so your patch no
longer applies.  I think I fixed that incidentally by moving the
labeling into qemu_migration.c (that is, commit 6034ddd55 should fix the
issue you were first testing).  Now, the bypassSecurityDriver in
qemu_driver.c doesn't affect any cleanup paths early or late, and in the
new qemuMigrationToFile, bypassSecurityDriver is coupled with the new
restoreLabel boolean which is set only when the labeling took place, so
the cleanup is properly gated.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110328/1aba4d49/attachment-0001.sig>