[libvirt] [PATCH 04/11] qemu_security: Kill code duplication

[Michal Privoznik]

On 02/08/2017 03:55 PM, Martin Kletzander wrote:
> On Wed, Feb 08, 2017 at 03:49:47PM +0100, Peter Krempa wrote:
>> On Wed, Feb 08, 2017 at 15:33:48 +0100, Michal Privoznik wrote:
>>> On 02/08/2017 02:59 PM, Martin Kletzander wrote:
>>> > On Wed, Feb 08, 2017 at 02:37:48PM +0100, Michal Privoznik wrote:
>>> >> On 02/08/2017 01:43 PM, Peter Krempa wrote:
>>> >>> On Wed, Feb 08, 2017 at 13:37:48 +0100, Michal Privoznik wrote:
>>> >>>> On 02/08/2017 01:23 PM, Peter Krempa wrote:
>>
>> [...]
>>
>>>
>>> This doesn't solve the syntax-check problem. But whatever, I will go
>>> with this and just drop the syntax-check patch. We have plenty of
>>> arguments for using macros here but since some don't like it I'm not
>>> going to push it. Lets just hope that we will take care to use
>>> qemuSecurity* wrappers instead of calling virSecurity APIs directly.
>>> Honestly, I don't care that much.
>>
>> You can do a macro:
>>
>> #define QEMU_SECURITY_WRAPPED(func) func
>>
>> And then use it in the functions that wrap the function like:
>>
>> int
>> qemuSecurityRestoreDiskLabel(virQEMUDriverPtr driver,
>>                             virDomainObjPtr vm,
>>                             virDomainDiskDefPtr disk)
>> {
>>    int ret = -1;
>>
>>    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
>>        virSecurityManagerTransactionStart(driver->securityManager) < 0)
>>        goto cleanup;
>>
>>    if
>> (QEMU_SECURITY_WRAPPED(virSecurityManagerRestoreDiskLabel)(driver->securityManager,
>>
>>                                                                  vm->def,
>>                                                                  disk)
>> < 0)
>>        goto cleanup;
>>
>>    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
>>        virSecurityManagerTransactionCommit(driver->securityManager,
>>                                            vm->pid) < 0)
>>        goto cleanup;
>>
>>    ret = 0;
>> cleanup:
>>    virSecurityManagerTransactionAbort(driver->securityManager);
>>    return ret;
>> }
>>
>> But the conditions are to use a very distinctive name of the macro and
>> the macro actually not doing much so that the readability of the code is
>> preserved, while having an anchor point for automatically looking up the
>> function names that were wrapped.
>>
>> Yet another option would be to do:
>>
>> #define QEMU_SECURITY_WRAPPED
>>
>> void QEMU_SECURITY_WRAPPED
>> qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
>>                                                       virDomainObjPtr vm,
>>                                                       bool migrated);
>>
>> In that case you need some 'sed' magic to figure out the function name
>> though.
>>
> 
> Why not just take the wrappers around virSecurity code, put them into
> qemu_security or similar and allow virSecurity* functions only in that
> file?  Clearly that file will only be wrappers, so it should help a lot.

Because so far we don't have a qemuSecurity wrapper for every
virSecurity API. Not sure whether we will need a wrapper for all of them
(e.g. virSecurityManagerSetImageFDLabel)

Michal