[libvirt] [PATCH v2] nwfilter: fix loadable module support

[Daniel P. Berrange]

On Tue, Jun 15, 2010 at 08:08:46AM -0400, Stefan Berger wrote:
> Following Daniel Berrange's suggestion of introducing another driver 
> interface, I now wrote the below patch where the nwfilter driver 
> registers the functions to instantiate and teardown the nwfilters with a 
> function in conf/domain_nwfilter.c called virDomainConfNWFilterRegister. 
> Previous helper functions that were called from qemu_driver.c and 
> qemu_conf.c were move into conf/domain_nwfilter.h with slight renaming 
> done for consistency. Those functions now call the function expored by 
> domain_nwfilter.c, which in turn call the functions of the new driver 
> interface, if available.
> 
> Signed-off-by: Stefan Berger <stefanb at us.ibm.com>

Structurally this basically looks good to me.

> Index: libvirt-acl/src/conf/domain_nwfilter.h
> ===================================================================
> --- /dev/null
> +++ libvirt-acl/src/conf/domain_nwfilter.h
> @@ -0,0 +1,68 @@
> +/*
> + * domain_nwfilter.h:
> + *
> + * Copyright (C) 2010 IBM Corporation
> + * Copyright (C) 2010 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> + *
> + * Author: Stefan Berger <stefanb at us.ibm.com>
> + */
> +#ifndef DOMAIN_NWFILTER_H
> +# define DOMAIN_NWFILTER_H
> +
> +typedef int (*virDomainConfInstantiateNWFilter)(virConnectPtr conn,
> +                                                virDomainNetDefPtr net);
> +typedef void (*virDomainConfTeardownNWFilter)(virDomainNetDefPtr net);
> +
> +typedef struct {
> +    virDomainConfInstantiateNWFilter instantiateFilter;
> +    virDomainConfTeardownNWFilter    teardownFilter;
> +} virDomainConfNWFilterDriver;
> +typedef virDomainConfNWFilterDriver *virDomainConfNWFilterDriverPtr;
> +
> +void virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver);
> +
> +int virDomainConfNWFilterInstantiate(virConnectPtr conn,
> +                                     virDomainNetDefPtr net);
> +void virDomainConfNWFilterTeardown(virDomainNetDefPtr net);
> +
> +
> +/* helper functions */
> +
> +static inline
> +int virNWFilterInstantiateNWFilter(virConnectPtr conn,
> +                                   const virDomainNetDefPtr net)
> +{
> +    return virDomainConfNWFilterInstantiate(conn, net);
> +}
> +
> +/* tear down an interface's filter before tearing down the interface */
> +static inline void
> +virNWFilterTearNWFilter(virDomainNetDefPtr net) {
> +    if ((net->filter) && (net->ifname))
> +        virDomainConfNWFilterTeardown(net);
> +}
> +
> +
> +static inline void
> +virNWFilterTearVMNWFilters(virDomainObjPtr vm) {
> +    int i;
> +
> +    for (i = 0; i < vm->def->nnets; i++)
> +        virNWFilterTearNWFilter(vm->def->nets[i]);
> +}

I'd prefer it if we didn't add these inline functions. It is simple
enough to just update the QEMU driver to call the new function
names directly, instead of having the compat shim layer. Using the
functions with new naming convention in QEMU driver code will make
it more obvious to the person reading QEMU code that functions
are calling into this code, rather than directly to nwfilter code.


Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|