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 <jinyan12@xxxxxxxxxx> --- 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.
