Hello! I am trying to fix the loadable modules support by removing all direct calls from qemu_driver.c and qemu_conf.c into nwfilter functions. The below patch extends the nwfilter driver interface with 2 more private functions to instantiate and tear down the filters. I call them 'private' because no client should ever be calling them via RPC and be able to tear down the filters for example -- they should only ever be callable from within libvirtd. I extended the qemudShutdownVMDaemon function with the virConnectPtr type of parameter so that I have the pointer to conn->nwfilterDriver to access the private nwfilter driver functions. The problem with that is that the conn pointer is not always available to be passed (qemuReconnectDomain, qemuHandleMonitorEOF), particularly in tear-down functions, and so I have to pass a NULL pointer, which then prevents the calling of the private interface function to tear down the filters.Does anyone have a suggestion on how I would best change the interface with the nwfilter driver to fix this particular problem? Stefan Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- src/esx/esx_nwfilter_driver.c | 10 ++++++++ src/nwfilter/nwfilter_driver.c | 15 ++++++++++++ src/nwfilter/nwfilter_gentech_driver.h | 26 ++++++++++++++++----- src/qemu/qemu_conf.c | 10 ++++---- src/qemu/qemu_driver.c | 40 ++++++++++++++++++--------------- 5 files changed, 72 insertions(+), 29 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -410,6 +410,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, @@ -421,6 +434,8 @@ static virNWFilterDriver nwfilterDriver .defineXML = nwfilterDefine, .undefine = nwfilterUndefine, .getXMLDesc = nwfilterDumpXML, + .privateInstantiateFilter = nwfilterInstantiateFilter, + .privateTeardownFilter = nwfilterTeardownFilter, }; Index: libvirt-acl/src/esx/esx_nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/esx/esx_nwfilter_driver.c +++ libvirt-acl/src/esx/esx_nwfilter_driver.c @@ -75,6 +75,8 @@ static virNWFilterDriver esxNWFilterDriv NULL, /* defineXML */ NULL, /* undefine */ NULL, /* getXMLDesc */ + NULL, /* privateInstantiateFilter */ + NULL, /* privateTeardownFilter */ }; 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 @@ -23,6 +23,8 @@ #ifndef __NWFILTER_GENTECH_DRIVER_H # define __NWFILTER_GENTECH_DRIVER_H +# include "datatypes.h" + virNWFilterTechDriverPtr virNWFilterTechDriverForName(const char *name); int virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res, @@ -37,8 +39,18 @@ enum instCase { }; +static inline +int virNWFilterInstNWFilter(virConnectPtr conn, + const virDomainNetDefPtr net) +{ + if ((conn) && (conn->nwfilterDriver)) + return conn->nwfilterDriver->privateInstantiateFilter(conn, net); + return 0; +} + int virNWFilterInstantiateFilter(virConnectPtr conn, const virDomainNetDefPtr net); + int virNWFilterUpdateInstantiateFilter(virConnectPtr conn, const virDomainNetDefPtr net, bool *skipIface); @@ -70,18 +82,20 @@ void virNWFilterDomainFWUpdateCB(void *p /* tear down an interface's filter before tearing down the interface */ static inline void -virNWFilterTearNWFilter(virDomainNetDefPtr net) { - if ((net->filter) && (net->ifname)) - virNWFilterTeardownFilter(net); +virNWFilterTearNWFilter(virConnectPtr conn, virDomainNetDefPtr net) { + if ((conn) && (conn->nwfilterDriver) && (net->filter) && (net->ifname)) + conn->nwfilterDriver->privateTeardownFilter(net); } static inline void -virNWFilterTearVMNWFilters(virDomainObjPtr vm) { +virNWFilterTearVMNWFilters(virConnectPtr conn, virDomainObjPtr vm) { int i; - for (i = 0; i < vm->def->nnets; i++) - virNWFilterTearNWFilter(vm->def->nets[i]); + if (conn) { + for (i = 0; i < vm->def->nnets; i++) + virNWFilterTearNWFilter(conn, 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 @@ -1555,7 +1555,7 @@ qemudPhysIfaceConnect(virConnectPtr conn if (rc >= 0) { if ((net->filter) && (net->ifname)) { - err = virNWFilterInstantiateFilter(conn, net); + err = virNWFilterInstNWFilter(conn, net); if (err) { close(rc); rc = -1; @@ -1688,7 +1688,7 @@ qemudNetworkIfaceConnect(virConnectPtr c if (tapfd >= 0) { if ((net->filter) && (net->ifname)) { - err = virNWFilterInstantiateFilter(conn, net); + err = virNWFilterInstNWFilter(conn, net); if (err) { close(tapfd); tapfd = -1; @@ -4207,7 +4207,7 @@ int qemudBuildCommandLine(virConnectPtr goto error; if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - virNWFilterTearNWFilter(net); + virNWFilterTearNWFilter(conn, net); close(tapfd); goto no_memory; } @@ -4226,7 +4226,7 @@ int qemudBuildCommandLine(virConnectPtr goto error; if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - virNWFilterTearNWFilter(net); + virNWFilterTearNWFilter(conn, net); close(tapfd); goto no_memory; } @@ -4766,7 +4766,7 @@ int qemudBuildCommandLine(virConnectPtr virReportOOMError(); error: for (i = 0; i <= last_good_net; i++) - virNWFilterTearNWFilter(def->nets[i]); + virNWFilterTearNWFilter(conn, def->nets[i]); if (vmfds && *vmfds) { for (i = 0; i < *nvmfds; i++) Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -158,7 +158,8 @@ static int qemudStartVMDaemon(virConnect int stdin_fd, const char *stdin_path); -static void qemudShutdownVMDaemon(struct qemud_driver *driver, +static void qemudShutdownVMDaemon(virConnectPtr conn, + struct qemud_driver *driver, virDomainObjPtr vm, int migrated); @@ -735,7 +736,7 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon VIR_DOMAIN_EVENT_STOPPED_FAILED : VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(NULL, driver, vm, 0); if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); else @@ -1283,7 +1284,7 @@ error: /* We can't get the monitor back, so must kill the VM * to remove danger of it ending up running twice if * user tries to start it again later */ - qemudShutdownVMDaemon(driver, obj, 0); + qemudShutdownVMDaemon(NULL, driver, obj, 0); if (!obj->persistent) virDomainRemoveInactive(&driver->domains, obj); else @@ -3576,7 +3577,7 @@ static int qemudStartVMDaemon(virConnect VIR_FREE(progenv); if (ret == -1) /* The VM failed to start; tear filters before taps */ - virNWFilterTearVMNWFilters(vm); + virNWFilterTearVMNWFilters(conn, vm); if (vmfds) { for (i = 0 ; i < nvmfds ; i++) { @@ -3642,7 +3643,7 @@ cleanup: /* We jump here if we failed to start the VM for any reason, or * if we failed to initialize the now running VM. kill it off and * pretend we never started it */ - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(conn, driver, vm, 0); if (logfile != -1) close(logfile); @@ -3651,7 +3652,8 @@ cleanup: } -static void qemudShutdownVMDaemon(struct qemud_driver *driver, +static void qemudShutdownVMDaemon(virConnectPtr conn, + struct qemud_driver *driver, virDomainObjPtr vm, int migrated) { int ret; @@ -3668,7 +3670,7 @@ static void qemudShutdownVMDaemon(struct * reporting so we don't squash a legit error. */ orig_err = virSaveLastError(); - virNWFilterTearVMNWFilters(vm); + virNWFilterTearVMNWFilters(conn, vm); if (driver->macFilter) { def = vm->def; @@ -4465,7 +4467,7 @@ static int qemudDomainDestroy(virDomainP goto endjob; } - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(dom->conn, driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); @@ -5211,7 +5213,7 @@ static int qemudDomainSaveFlag(virDomain ret = 0; /* Shut it down */ - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(dom->conn, driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); @@ -5528,7 +5530,7 @@ static int qemudDomainCoreDump(virDomain endjob: if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(dom->conn, driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); @@ -7640,7 +7642,7 @@ cleanup: VIR_WARN0("Unable to release PCI address on NIC"); if (ret != 0) - virNWFilterTearNWFilter(net); + virNWFilterTearNWFilter(conn, net); VIR_FREE(nicstr); VIR_FREE(netstr); @@ -8534,7 +8536,8 @@ cleanup: } static int -qemudDomainDetachNetDevice(struct qemud_driver *driver, +qemudDomainDetachNetDevice(virConnectPtr conn, + struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev, unsigned long long qemuCmdFlags) @@ -8609,7 +8612,7 @@ qemudDomainDetachNetDevice(struct qemud_ } qemuDomainObjExitMonitorWithDriver(driver, vm); - virNWFilterTearNWFilter(detach); + virNWFilterTearNWFilter(conn, detach); #if WITH_MACVTAP if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) { @@ -8921,7 +8924,8 @@ static int qemudDomainDetachDevice(virDo _("This type of disk cannot be hot unplugged")); } } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { - ret = qemudDomainDetachNetDevice(driver, vm, dev, qemuCmdFlags); + ret = qemudDomainDetachNetDevice(dom->conn, driver, vm, dev, + qemuCmdFlags); } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { ret = qemudDomainDetachPciControllerDevice(driver, vm, dev, @@ -10215,7 +10219,7 @@ qemudDomainMigratePrepareTunnel(virConne qemust = qemuStreamMigOpen(st, unixfile); if (qemust == NULL) { - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(dconn, driver, vm, 0); if (!vm->persistent) { if (qemuDomainObjEndJob(vm) > 0) virDomainRemoveInactive(&driver->domains, vm); @@ -10940,7 +10944,7 @@ qemudDomainMigratePerform (virDomainPtr } /* Clean up the source domain. */ - qemudShutdownVMDaemon(driver, vm, 1); + qemudShutdownVMDaemon(dom->conn, driver, vm, 1); resume = 0; event = virDomainEventNewFromObj(vm, @@ -11100,7 +11104,7 @@ qemudDomainMigrateFinish2 (virConnectPtr goto endjob; } } else { - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(dconn, driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); @@ -11946,7 +11950,7 @@ static int qemuDomainRevertToSnapshot(vi */ if (virDomainObjIsActive(vm)) { - qemudShutdownVMDaemon(driver, vm, 0); + qemudShutdownVMDaemon(snapshot->domain->conn, driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); Index: libvirt-acl/src/driver.h =================================================================== --- libvirt-acl.orig/src/driver.h +++ libvirt-acl/src/driver.h @@ -1098,6 +1098,14 @@ typedef char * (*virDrvNWFilterGetXMLDesc) (virNWFilterPtr pool, unsigned int flags); +# include "conf/domain_conf.h" + +typedef int + (*virDrvNWFilterInstantiateFilter) (virConnectPtr conn, + const virDomainNetDefPtr net); + +typedef void + (*virDrvNWFilterTeardownFilter) (virDomainNetDefPtr net); typedef struct _virNWFilterDriver virNWFilterDriver; typedef virNWFilterDriver *virNWFilterDriverPtr; @@ -1124,6 +1132,8 @@ struct _virNWFilterDriver { virDrvNWFilterDefineXML defineXML; virDrvNWFilterUndefine undefine; virDrvNWFilterGetXMLDesc getXMLDesc; + virDrvNWFilterInstantiateFilter privateInstantiateFilter; + virDrvNWFilterTeardownFilter privateTeardownFilter; };