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

Stefan Berger stefanb at linux.vnet.ibm.com
Mon Jun 14 11:49:50 UTC 2010


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 at 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;
  };





More information about the libvir-list mailing list