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 <jinyan12@huawei.com>
---
  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.