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

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

于 2011年03月29日 03:07, 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.

I looked through commit 6034ddd55, it should fix the problem,
thanks for picking it up incidentally.


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