On 2020/11/13 2:54, Michal Privoznik wrote:
On 11/12/20 3:07 PM, Jin Yan wrote:
When migration fails, qemuMigrationDstPrepareAny will call qemuProcessStop
to restore labels only after all labels are successfully set. If some labels
fail to set, the labels that have been set will not be restored.

Signed-off-by: Jin Yan <[email protected]>
---
   src/qemu/qemu_security.c | 12 ++++++++++--
   1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 3bda96272c..0cb90c840a 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -51,16 +51,24 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
                                         incomingPath,
                                         priv->chardevStdioLogd,
                                         migrated) < 0)
-        goto cleanup;
+        goto restorelabel;
if (virSecurityManagerTransactionCommit(driver->securityManager,
                                               pid, priv->rememberOwner) < 0)
-        goto cleanup;
+        goto restorelabel;
ret = 0;
+
    cleanup:
       virSecurityManagerTransactionAbort(driver->securityManager);
       return ret;
+
+ restorelabel:
+    virSecurityManagerRestoreAllLabel(driver->securityManager,
+                                      vm->def,
+                                      migrated,
+                                      priv->chardevStdioLogd);
+    goto cleanup;
   }
I don't think this is correct. Firstly, this restores labels for ALL
paths, and not just the failed ones (which messes up seclabel
remembering and its refcounting), but more importantly:

1) rollback within one secdriver is handled in .transactionCommit
callback, well virSecurity*TransactionRun() actually,

2) rollback for other secdrivers after one failed is handled in
virSecurityStackSetAllLabel().


Is this not working properly? What version do you run?

Hi Michal,
I found this problem while performing migration, based on
    libvirt version: 6.2.0
    SELinux mode: permissive

Steps:
1. start a vm configured with pipe-type serial port.
    <serial type='pipe'>
      <source path='/tmp/test_pipe'/>
      <target type='system-serial' port='1'>
        <model name='pl011'/>
      </target>
    </serial>
2. migrate vm to Dst-side where no '/tmp/test_pipe' exits.
3. migration failed in Dst-side qemuProcessLaunch, and the path's label that
has been set is not restored ('/var/lib/libvirt/qemu/nvram/XXX.fd').

I have no idea why 2)rollback you mentioned didn't work.

Reply via email to