Re: [libvirt] [PATCH v2 5/5] qemu: Use transactions from security driver

On 01/09/2017 07:58 AM, Michal Privoznik wrote: > So far if qemu is spawned under separate mount namespace in order > to relabel everything it needs an access to the security driver > is run in that namespace too. This has a very nasty down side - s/is/to/ > it is being run in a separate process, so any internal state > transition is NOT reflected in the dameon. This can lead to many s/dameon/daemon > sleepless nights. Therefore, use the transaction APIs so that > libvirt developers can sleep tight again. Having trouble sleeping lately? ;-) > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/qemu/qemu_security.c | 100 ++++++++++++++--------------------------------- > 1 file changed, 30 insertions(+), 70 deletions(-) > > diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c > index 9ab91e9f2..544feeb4a 100644 > --- a/src/qemu/qemu_security.c > +++ b/src/qemu/qemu_security.c > @@ -40,66 +40,31 @@ struct qemuSecuritySetRestoreAllLabelData { > }; > > > -static int > -qemuSecuritySetRestoreAllLabelHelper(pid_t pid, > - void *opaque) > -{ > - struct qemuSecuritySetRestoreAllLabelData *data = opaque; > - > - virSecurityManagerPostFork(data->driver->securityManager); > - > - if (data->set) { > - VIR_DEBUG("Setting up security labels inside namespace pid=%lld", > - (long long) pid); > - if (virSecurityManagerSetAllLabel(data->driver->securityManager, > - data->vm->def, > - data->stdin_path) < 0) > - return -1; > - } else { > - VIR_DEBUG("Restoring security labels inside namespace pid=%lld", > - (long long) pid); > - if (virSecurityManagerRestoreAllLabel(data->driver->securityManager, > - data->vm->def, > - data->migrated) < 0) > - return -1; > - } > - > - return 0; > -} > - > - > int > qemuSecuritySetAllLabel(virQEMUDriverPtr driver, > virDomainObjPtr vm, > const char *stdin_path) > { > - struct qemuSecuritySetRestoreAllLabelData data; > + int ret = -1; > > - memset(&data, 0, sizeof(data)); > + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) && > + virSecurityManagerTransactionStart(driver->securityManager) < 0) > + goto cleanup; > > - data.set = true; > - data.driver = driver; > - data.vm = vm; > - data.stdin_path = stdin_path; > + if (virSecurityManagerSetAllLabel(driver->securityManager, > + vm->def, > + stdin_path) < 0) > + goto cleanup; > > - if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) { > - if (virSecurityManagerPreFork(driver->securityManager) < 0) > - return -1; Both paths have removed the PreFork/PostFork processing... Is that then no longer required? This is/was the only PreFork caller I think.