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

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.