[libvirt] [PATCH v2] nwfilter: fix loadable module support
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@us.ibm.com> --- src/Makefile.am | 3 - src/conf/domain_nwfilter.c | 51 ++++++++++++++++++++++++ src/conf/domain_nwfilter.h | 68 +++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 5 ++ src/nwfilter/nwfilter_driver.c | 22 ++++++++++ src/nwfilter/nwfilter_gentech_driver.h | 17 -------- src/qemu/qemu_conf.c | 11 ++--- src/qemu/qemu_driver.c | 2 8 files changed, 155 insertions(+), 24 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h @@ -67,21 +67,4 @@ void virNWFilterDomainFWUpdateCB(void *p const char *name ATTRIBUTE_UNUSED, void *data); - -/* tear down an interface's filter before tearing down the interface */ -static inline void -virNWFilterTearNWFilter(virDomainNetDefPtr net) { - if ((net->filter) && (net->ifname)) - virNWFilterTeardownFilter(net); -} - - -static inline void -virNWFilterTearVMNWFilters(virDomainObjPtr vm) { - int i; - - for (i = 0; i < vm->def->nnets; i++) - virNWFilterTearNWFilter(vm->def->nets[i]); -} - #endif Index: libvirt-acl/src/qemu/qemu_conf.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.c +++ libvirt-acl/src/qemu/qemu_conf.c @@ -54,7 +54,7 @@ #include "network.h" #include "macvtap.h" #include "cpu/cpu.h" -#include "nwfilter/nwfilter_gentech_driver.h" +#include "domain_nwfilter.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1514,9 +1514,10 @@ int qemudExtractVersion(struct qemud_dri /** * qemudPhysIfaceConnect: * @conn: pointer to virConnect object + * @driver: pointer to the qemud_driver * @net: pointer to he VM's interface description with direct device type - * @linkdev: The name of the physical interface to link the macvtap to - * @brmode: The mode to put the macvtap device into + * @qemuCmdFlags: flags for qemu + * @vmuuid: The UUID of the VM (needed by 802.1Qbh) * * Returns a filedescriptor on success or -1 in case of error. */ @@ -1555,7 +1556,7 @@ qemudPhysIfaceConnect(virConnectPtr conn if (rc >= 0) { if ((net->filter) && (net->ifname)) { - err = virNWFilterInstantiateFilter(conn, net); + err = virNWFilterInstantiateNWFilter(conn, net); if (err) { close(rc); rc = -1; @@ -1688,7 +1689,7 @@ qemudNetworkIfaceConnect(virConnectPtr c if (tapfd >= 0) { if ((net->filter) && (net->ifname)) { - err = virNWFilterInstantiateFilter(conn, net); + err = virNWFilterInstantiateNWFilter(conn, net); if (err) { close(tapfd); tapfd = -1; Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -81,7 +81,7 @@ #include "xml.h" #include "cpu/cpu.h" #include "macvtap.h" -#include "nwfilter/nwfilter_gentech_driver.h" +#include "domain_nwfilter.h" #include "hooks.h" #include "storage_file.h" Index: libvirt-acl/src/Makefile.am =================================================================== --- libvirt-acl.orig/src/Makefile.am +++ libvirt-acl/src/Makefile.am @@ -97,7 +97,8 @@ DRIVER_SOURCES = \ # Domain driver generic impl APIs DOMAIN_CONF_SOURCES = \ conf/capabilities.c conf/capabilities.h \ - conf/domain_conf.c conf/domain_conf.h + conf/domain_conf.c conf/domain_conf.h \ + conf/domain_nwfilter.c conf/domain_nwfilter.h DOMAIN_EVENT_SOURCES = \ conf/domain_event.c conf/domain_event.h 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@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]); +} + +#endif /* DOMAIN_NWFILTER_H */ Index: libvirt-acl/src/conf/domain_nwfilter.c =================================================================== --- /dev/null +++ libvirt-acl/src/conf/domain_nwfilter.c @@ -0,0 +1,51 @@ +/* + * domain_nwfilter.c: + * + * Copyright (C) 2010 IBM Corporation + * + * 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@us.ibm.com> + */ + +#include <config.h> + +#include "internal.h" + +#include "datatypes.h" +#include "domain_conf.h" +#include "domain_nwfilter.h" + +static virDomainConfNWFilterDriverPtr nwfilterDriver; + +void +virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver) { + nwfilterDriver = driver; +} + +int +virDomainConfNWFilterInstantiate(virConnectPtr conn, + virDomainNetDefPtr net) { + if (nwfilterDriver) + return nwfilterDriver->instantiateFilter(conn, net); + /* driver module not available -- don't indicate failure */ + return 0; +} + +void +virDomainConfNWFilterTeardown(virDomainNetDefPtr net) { + if (nwfilterDriver) + nwfilterDriver->teardownFilter(net); +} Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -264,6 +264,11 @@ virDomainEventDispatchDefaultFunc; virDomainEventDispatch; virDomainEventQueueDispatch; +# domain_nwfilter.h +virDomainConfNWFilterRegister; +virDomainConfNWFilterInstantiate; +virDomainConfNWFilterTeardown; + # ebtables.h ebtablesAddForwardAllowIn; Index: libvirt-acl/src/nwfilter/nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -33,6 +33,7 @@ #include "datatypes.h" #include "memory.h" #include "domain_conf.h" +#include "domain_nwfilter.h" #include "nwfilter_driver.h" #include "nwfilter_gentech_driver.h" @@ -410,6 +411,19 @@ cleanup: } +static int +nwfilterInstantiateFilter(virConnectPtr conn, + virDomainNetDefPtr net) { + return virNWFilterInstantiateFilter(conn, net); +} + + +static void +nwfilterTeardownFilter(virDomainNetDefPtr net) { + virNWFilterTeardownFilter(net); +} + + static virNWFilterDriver nwfilterDriver = { .name = "nwfilter", .open = nwfilterOpen, @@ -432,8 +446,16 @@ static virStateDriver stateDriver = { .active = nwfilterDriverActive, }; + +static virDomainConfNWFilterDriver domainNWFilterDriver = { + .instantiateFilter = nwfilterInstantiateFilter, + .teardownFilter = nwfilterTeardownFilter, +}; + + int nwfilterRegister(void) { virRegisterNWFilterDriver(&nwfilterDriver); virRegisterStateDriver(&stateDriver); + virDomainConfNWFilterRegister(&domainNWFilterDriver); return 0; }
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@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@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 :|
On 06/16/2010 08:02 AM, Daniel P. Berrange wrote:
+/* 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.
The reason why I previously had introduced them (I only moved the latter 2) is that these functions get called several times (2, 5 and 2 times respectively) and I wanted to avoid replication of for example the tests (if-statement) in front of them. The virNWFilterTearVMNWFilters() calls the inline function virNWFilterTearNWFilter() while iterating over all the nets, so that also makes the code easier to read if one only has to call one function. Stefan
Regards, Daniel
participants (2)
-
Daniel P. Berrange -
Stefan Berger