[libvirt] [PATCH v3] nwfilter: extend nwfilter reload support

[Stefan Berger]

This time with a proper title.

v3:
     - Fixed an indentation problem

     - added bool parameter to function terminating the IP address 
learner threads to determine whether future threads may still run 
(needed in case of driver reload) or all must terminate (need in case of 
libvirtd termination)

v2:

     - Fixes to the nwfilter driver reload function that also needs a 
valid virConnectPtr.

In this patch I am extending and fixing the nwfilter module's reload 
support to stop all ongoing threads (for learning IP addresses of 
interfaces) and rebuild the filtering rules of all interfaces of all VMs 
when libvirt is started. Now libvirtd rebuilds the filters upon the 
SIGHUP signal and libvirtd restart.

About the patch: The nwfilter functions require a virConnectPtr. 
Therefore I am opening a connection in qemudStartup, which later on 
needs to be closed outside where the driver lock is held since otherwise 
it ends up in a deadlock due to virConnectClose() trying to lock the 
driver as well.

I have tested this now for a while with several machines running and 
needing the IP address learner thread(s). The rebuilding of the firewall 
rules seems to work fine following libvirtd restart or a SIGHUP. Also 
the termination of libvirtd worked fine.


Signed-off-by: Stefan Berger<stefanb at us.ibm.com>

---
  src/nwfilter/nwfilter_driver.c      |   21 +++++++++++---
  src/nwfilter/nwfilter_learnipaddr.c |   16 ++++++++---
  src/nwfilter/nwfilter_learnipaddr.h |    1
  src/qemu/qemu_driver.c              |   52 
+++++++++++++++++++++++++++++++++---
  4 files changed, 77 insertions(+), 13 deletions(-)

Index: libvirt-acl/src/nwfilter/nwfilter_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_driver.c
@@ -143,15 +143,26 @@ conf_init_err:
   */
  static int
  nwfilterDriverReload(void) {
+    virConnectPtr conn;
+
      if (!driverState) {
          return -1;
      }

-    nwfilterDriverLock(driverState);
-    virNWFilterPoolLoadAllConfigs(NULL,
- &driverState->pools,
-                                  driverState->configDir);
-    nwfilterDriverUnlock(driverState);
+    conn = virConnectOpen("qemu:///system");
+
+    if (conn) {
+        /* shut down all threads -- qemud for example will restart them */
+        virNWFilterLearnThreadsTerminate(true);
+
+        nwfilterDriverLock(driverState);
+        virNWFilterPoolLoadAllConfigs(conn,
+ &driverState->pools,
+                                      driverState->configDir);
+        nwfilterDriverUnlock(driverState);
+
+        virConnectClose(conn);
+    }

      return 0;
  }
Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c
+++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
@@ -857,6 +857,17 @@ virNWFilterLearnInit(void) {
  }


+void
+virNWFilterLearnThreadsTerminate(bool allowNewThreads) {
+    threadsTerminate = true;
+
+    while (virHashSize(pendingLearnReq) != 0)
+        usleep((PKT_TIMEOUT_MS * 1000) / 3);
+
+    if (allowNewThreads)
+        threadsTerminate = false;
+}
+
  /**
   * virNWFilterLearnShutdown
   * Shutdown of this layer
@@ -864,10 +875,7 @@ virNWFilterLearnInit(void) {
  void
  virNWFilterLearnShutdown(void) {

-    threadsTerminate = true;
-
-    while (virHashSize(pendingLearnReq) != 0)
-        usleep((PKT_TIMEOUT_MS * 1000) / 3);
+    virNWFilterLearnThreadsTerminate(false);

      virHashFree(pendingLearnReq, freeLearnReqEntry);
      pendingLearnReq = NULL;
Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.h
+++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h
@@ -71,5 +71,6 @@ void virNWFilterUnlockIface(const char *

  int virNWFilterLearnInit(void);
  void virNWFilterLearnShutdown(void);
+void virNWFilterLearnThreadsTerminate(bool allowNewThreads);

  #endif /* __NWFILTER_LEARNIPADDR_H */
Index: libvirt-acl/src/qemu/qemu_driver.c
===================================================================
--- libvirt-acl.orig/src/qemu/qemu_driver.c
+++ libvirt-acl/src/qemu/qemu_driver.c
@@ -170,6 +170,9 @@ static int qemuDetectVcpuPIDs(struct qem
  static int qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
                                         virDomainDefPtr def);

+static int qemudVMFiltersInstantiate(virConnectPtr conn,
+                                     virDomainDefPtr def);
+
  static struct qemud_driver *qemu_driver = NULL;


@@ -1423,6 +1426,10 @@ error:
      return ret;
  }

+struct virReconnectDomainData {
+    virConnectPtr conn;
+    struct qemud_driver *driver;
+};
  /*
   * Open an existing VM's monitor, re-detect VCPU threads
   * and re-reserve the security labels in use
@@ -1431,9 +1438,11 @@ static void
  qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, 
void *opaque)
  {
      virDomainObjPtr obj = payload;
-    struct qemud_driver *driver = opaque;
+    struct virReconnectDomainData *data = opaque;
+    struct qemud_driver *driver = data->driver;
      qemuDomainObjPrivatePtr priv;
      unsigned long long qemuCmdFlags;
+    virConnectPtr conn = data->conn;

      virDomainObjLock(obj);

@@ -1467,6 +1476,9 @@ qemuReconnectDomain(void *payload, const
                                                             obj) < 0)
          goto error;

+    if (qemudVMFiltersInstantiate(conn, obj->def))
+        goto error;
+
      if (obj->def->id >= driver->nextvmid)
          driver->nextvmid = obj->def->id + 1;

@@ -1491,9 +1503,10 @@ error:
   * about.
   */
  static void
-qemuReconnectDomains(struct qemud_driver *driver)
+qemuReconnectDomains(virConnectPtr conn, struct qemud_driver *driver)
  {
-    virHashForEach(driver->domains.objs, qemuReconnectDomain, driver);
+    struct virReconnectDomainData data = {conn, driver};
+    virHashForEach(driver->domains.objs, qemuReconnectDomain, &data);
  }


@@ -1691,6 +1704,7 @@ qemudStartup(int privileged) {
      char *base = NULL;
      char driverConf[PATH_MAX];
      int rc;
+    virConnectPtr conn = NULL;

      if (VIR_ALLOC(qemu_driver) < 0)
          return -1;
@@ -1912,7 +1926,11 @@ qemudStartup(int privileged) {
                                  1, NULL, NULL) < 0)
          goto error;

-    qemuReconnectDomains(qemu_driver);
+    conn = virConnectOpen(qemu_driver->privileged ?
+                          "qemu:///system" :
+                          "qemu:///session");
+
+    qemuReconnectDomains(conn, qemu_driver);

      /* Then inactive persistent configs */
      if (virDomainLoadAllConfigs(qemu_driver->caps,
@@ -1930,6 +1948,8 @@ qemudStartup(int privileged) {

      qemudAutostartConfigs(qemu_driver);

+    if (conn)
+        virConnectClose(conn);

      return 0;

@@ -1938,6 +1958,8 @@ out_of_memory:
  error:
      if (qemu_driver)
          qemuDriverUnlock(qemu_driver);
+    if (conn)
+        virConnectClose(conn);
      VIR_FREE(base);
      qemudShutdown();
      return -1;
@@ -12738,6 +12760,28 @@ qemudVMFilterRebuild(virConnectPtr conn
      return 0;
  }

+static int
+qemudVMFiltersInstantiate(virConnectPtr conn,
+                          virDomainDefPtr def)
+{
+    int err = 0;
+    int i;
+
+    if (!conn)
+        return 1;
+
+    for (i = 0 ; i < def->nnets ; i++) {
+        virDomainNetDefPtr net = def->nets[i];
+        if ((net->filter) && (net->ifname)) {
+           if (virDomainConfNWFilterInstantiate(conn, net)) {
+                err = 1;
+                break;
+            }
+        }
+    }
+
+    return err;
+}

  static virNWFilterCallbackDriver qemuCallbackDriver = {
      .name = "QEMU",