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

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

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.

> Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list