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

[Michal Privoznik]

On 01/09/2017 11:18 PM, John Ferlan wrote:
> 
> 
> 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 at redhat.com>
>> ---
>>  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.


Yes, it is no longer required. There is no fork() happening in
virSecurityManagerSetAllLabel() anymore.

Michal