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? Michal
