Re: [PATCH 2/3] Don't pass virConnectPtr in nwfilter 'struct domUpdateCBStruct'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/03/2013 09:06 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> The nwfilter driver only needs a reference to its private
> state object, not a full virConnectPtr. Update the domUpdateCBStruct
> struct to have a 'void *opaque' field instead of a virConnectPtr.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  src/conf/nwfilter_conf.c               | 14 +++++++++++---
>  src/conf/nwfilter_conf.h               |  4 ++--
>  src/nwfilter/nwfilter_dhcpsnoop.c      | 12 ++++++------
>  src/nwfilter/nwfilter_driver.c         |  5 +++--
>  src/nwfilter/nwfilter_gentech_driver.c | 32 ++++++++++++++++----------------
>  src/nwfilter/nwfilter_gentech_driver.h | 10 +++++-----
>  src/nwfilter/nwfilter_learnipaddr.c    |  6 +++---
>  7 files changed, 46 insertions(+), 37 deletions(-)

I did a sanity compile / syntax-check, and manually looked through the
code and it all looks okay. So ACK based on that (I'm assuming that
you've actually fired it all up :-)

>
> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
> index c009921..9927f7e 100644
> --- a/src/conf/nwfilter_conf.c
> +++ b/src/conf/nwfilter_conf.c
> @@ -2850,6 +2850,7 @@ virNWFilterCallbackDriversUnlock(void)
>  
>  
>  static virDomainObjListIterator virNWFilterDomainFWUpdateCB;
> +static void *virNWFilterDomainFWUpdateOpaque;
>  
>  /**
>   * virNWFilterInstFiltersOnAllVMs:
> @@ -2861,7 +2862,7 @@ virNWFilterInstFiltersOnAllVMs(virConnectPtr conn)
>  {
>      size_t i;
>      struct domUpdateCBStruct cb = {
> -        .conn = conn,
> +        .opaque = virNWFilterDomainFWUpdateOpaque,
>          .step = STEP_APPLY_CURRENT,
>          .skipInterfaces = NULL, /* not needed */
>      };
> @@ -2880,7 +2881,7 @@ virNWFilterTriggerVMFilterRebuild(virConnectPtr conn)
>      size_t i;
>      int ret = 0;
>      struct domUpdateCBStruct cb = {
> -        .conn = conn,
> +        .opaque = virNWFilterDomainFWUpdateOpaque,
>          .step = STEP_APPLY_NEW,
>          .skipInterfaces = virHashCreate(0, NULL),
>      };
> @@ -3474,9 +3475,14 @@ char *virNWFilterConfigFile(const char *dir,
>  }
>  
>  
> -int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB)
> +int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB,
> +                             void *opaque)
>  {
> +    if (initialized)
> +        return -1;
> +
>      virNWFilterDomainFWUpdateCB = domUpdateCB;
> +    virNWFilterDomainFWUpdateOpaque = opaque;
>  
>      initialized = true;
>  
> @@ -3495,6 +3501,8 @@ void virNWFilterConfLayerShutdown(void)
>      virMutexDestroy(&updateMutex);
>  
>      initialized = false;
> +    virNWFilterDomainFWUpdateOpaque = NULL;
> +    virNWFilterDomainFWUpdateCB = NULL;
>  }
>  
>  
> diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
> index faa7527..e470615 100644
> --- a/src/conf/nwfilter_conf.h
> +++ b/src/conf/nwfilter_conf.h
> @@ -586,7 +586,7 @@ enum UpdateStep {
>  };
>  
>  struct domUpdateCBStruct {
> -    virConnectPtr conn;
> +    void *opaque;
>      enum UpdateStep step;
>      virHashTablePtr skipInterfaces;
>  };
> @@ -722,7 +722,7 @@ void virNWFilterObjUnlock(virNWFilterObjPtr obj);
>  void virNWFilterLockFilterUpdates(void);
>  void virNWFilterUnlockFilterUpdates(void);
>  
> -int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB);
> +int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, void *opaque);
>  void virNWFilterConfLayerShutdown(void);
>  
>  int virNWFilterInstFiltersOnAllVMs(virConnectPtr conn);
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
> index 3e9f046..2bc1686 100644
> --- a/src/nwfilter/nwfilter_dhcpsnoop.c
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -481,15 +481,15 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
>      /* instantiate the filters */
>  
>      if (req->ifname)
> -        rc = virNWFilterInstantiateFilterLate(NULL,
> +        rc = virNWFilterInstantiateFilterLate(req->driver,
> +                                              NULL,
>                                                req->ifname,
>                                                req->ifindex,
>                                                req->linkdev,
>                                                req->nettype,
>                                                &req->macaddr,
>                                                req->filtername,
> -                                              req->vars,
> -                                              req->driver);
> +                                              req->vars);
>  
>  exit_snooprequnlock:
>      virNWFilterSnoopReqUnlock(req);
> @@ -867,15 +867,15 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
>          goto skip_instantiate;
>  
>      if (ipAddrLeft) {
> -        ret = virNWFilterInstantiateFilterLate(NULL,
> +        ret = virNWFilterInstantiateFilterLate(req->driver,
> +                                               NULL,
>                                                 req->ifname,
>                                                 req->ifindex,
>                                                 req->linkdev,
>                                                 req->nettype,
>                                                 &req->macaddr,
>                                                 req->filtername,
> -                                               req->vars,
> -                                               req->driver);
> +                                               req->vars);
>      } else {
>          const virNWFilterVarValuePtr dhcpsrvrs =
>              virHashLookup(req->vars->hashTable, NWFILTER_VARNAME_DHCPSERVER);
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index c2afdfc..6e20e03 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -203,7 +203,8 @@ nwfilterStateInitialize(bool privileged,
>  
>      virNWFilterTechDriversInit(privileged);
>  
> -    if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB) < 0)
> +    if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB,
> +                                 driverState) < 0)
>          goto err_techdrivers_shutdown;
>  
>      /*
> @@ -681,7 +682,7 @@ nwfilterInstantiateFilter(virConnectPtr conn,
>                            const unsigned char *vmuuid,
>                            virDomainNetDefPtr net)
>  {
> -    return virNWFilterInstantiateFilter(conn, vmuuid, net);
> +    return virNWFilterInstantiateFilter(conn->nwfilterPrivateData, vmuuid, net);
>  }
>  
>  
> diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> index 382d73f..5961165 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/src/nwfilter/nwfilter_gentech_driver.c
> @@ -800,7 +800,8 @@ err_unresolvable_vars:
>   * Call this function while holding the NWFilter filter update lock
>   */
>  static int
> -__virNWFilterInstantiateFilter(const unsigned char *vmuuid,
> +__virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
> +                               const unsigned char *vmuuid,
>                                 bool teardownOld,
>                                 const char *ifname,
>                                 int ifindex,
> @@ -810,7 +811,6 @@ __virNWFilterInstantiateFilter(const unsigned char *vmuuid,
>                                 const char *filtername,
>                                 virNWFilterHashTablePtr filterparams,
>                                 enum instCase useNewFilter,
> -                               virNWFilterDriverStatePtr driver,
>                                 bool forceWithPendingReq,
>                                 bool *foundNewFilter)
>  {
> @@ -921,7 +921,7 @@ err_exit:
>  
>  
>  static int
> -_virNWFilterInstantiateFilter(virConnectPtr conn,
> +_virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
>                                const unsigned char *vmuuid,
>                                const virDomainNetDefPtr net,
>                                bool teardownOld,
> @@ -948,7 +948,8 @@ _virNWFilterInstantiateFilter(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> -    rc = __virNWFilterInstantiateFilter(vmuuid,
> +    rc = __virNWFilterInstantiateFilter(driver,
> +                                        vmuuid,
>                                          teardownOld,
>                                          net->ifname,
>                                          ifindex,
> @@ -958,7 +959,6 @@ _virNWFilterInstantiateFilter(virConnectPtr conn,
>                                          net->filter,
>                                          net->filterparams,
>                                          useNewFilter,
> -                                        conn->nwfilterPrivateData,
>                                          false,
>                                          foundNewFilter);
>  
> @@ -970,22 +970,23 @@ cleanup:
>  
>  
>  int
> -virNWFilterInstantiateFilterLate(const unsigned char *vmuuid,
> +virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
> +                                 const unsigned char *vmuuid,
>                                   const char *ifname,
>                                   int ifindex,
>                                   const char *linkdev,
>                                   enum virDomainNetType nettype,
>                                   const virMacAddrPtr macaddr,
>                                   const char *filtername,
> -                                 virNWFilterHashTablePtr filterparams,
> -                                 virNWFilterDriverStatePtr driver)
> +                                 virNWFilterHashTablePtr filterparams)
>  {
>      int rc;
>      bool foundNewFilter = false;
>  
>      virNWFilterLockFilterUpdates();
>  
> -    rc = __virNWFilterInstantiateFilter(vmuuid,
> +    rc = __virNWFilterInstantiateFilter(driver,
> +                                        vmuuid,
>                                          true,
>                                          ifname,
>                                          ifindex,
> @@ -995,7 +996,6 @@ virNWFilterInstantiateFilterLate(const unsigned char *vmuuid,
>                                          filtername,
>                                          filterparams,
>                                          INSTANTIATE_ALWAYS,
> -                                        driver,
>                                          true,
>                                          &foundNewFilter);
>      if (rc < 0) {
> @@ -1015,13 +1015,13 @@ virNWFilterInstantiateFilterLate(const unsigned char *vmuuid,
>  
>  
>  int
> -virNWFilterInstantiateFilter(virConnectPtr conn,
> +virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
>                               const unsigned char *vmuuid,
>                               const virDomainNetDefPtr net)
>  {
>      bool foundNewFilter = false;
>  
> -    return _virNWFilterInstantiateFilter(conn, vmuuid, net,
> +    return _virNWFilterInstantiateFilter(driver, vmuuid, net,
>                                           1,
>                                           INSTANTIATE_ALWAYS,
>                                           &foundNewFilter);
> @@ -1029,14 +1029,14 @@ virNWFilterInstantiateFilter(virConnectPtr conn,
>  
>  
>  int
> -virNWFilterUpdateInstantiateFilter(virConnectPtr conn,
> +virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver,
>                                     const unsigned char *vmuuid,
>                                     const virDomainNetDefPtr net,
>                                     bool *skipIface)
>  {
>      bool foundNewFilter = false;
>  
> -    int rc = _virNWFilterInstantiateFilter(conn, vmuuid, net,
> +    int rc = _virNWFilterInstantiateFilter(driver, vmuuid, net,
>                                             0,
>                                             INSTANTIATE_FOLLOW_NEWFILTER,
>                                             &foundNewFilter);
> @@ -1154,7 +1154,7 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj,
>              if ((net->filter) && (net->ifname)) {
>                  switch (cb->step) {
>                  case STEP_APPLY_NEW:
> -                    ret = virNWFilterUpdateInstantiateFilter(cb->conn,
> +                    ret = virNWFilterUpdateInstantiateFilter(cb->opaque,
>                                                               vm->uuid,
>                                                               net,
>                                                               &skipIface);
> @@ -1179,7 +1179,7 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj,
>                      break;
>  
>                  case STEP_APPLY_CURRENT:
> -                    ret = virNWFilterInstantiateFilter(cb->conn,
> +                    ret = virNWFilterInstantiateFilter(cb->opaque,
>                                                         vm->uuid,
>                                                         net);
>                      if (ret)
> diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h
> index 4b47b4a..8528e2a 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.h
> +++ b/src/nwfilter/nwfilter_gentech_driver.h
> @@ -39,23 +39,23 @@ enum instCase {
>  };
>  
>  
> -int virNWFilterInstantiateFilter(virConnectPtr conn,
> +int virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
>                                   const unsigned char *vmuuid,
>                                   const virDomainNetDefPtr net);
> -int virNWFilterUpdateInstantiateFilter(virConnectPtr conn,
> +int virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver,
>                                         const unsigned char *vmuuid,
>                                         const virDomainNetDefPtr net,
>                                         bool *skipIface);
>  
> -int virNWFilterInstantiateFilterLate(const unsigned char *vmuuid,
> +int virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
> +                                     const unsigned char *vmuuid,
>                                       const char *ifname,
>                                       int ifindex,
>                                       const char *linkdev,
>                                       enum virDomainNetType nettype,
>                                       const virMacAddrPtr macaddr,
>                                       const char *filtername,
> -                                     virNWFilterHashTablePtr filterparams,
> -                                     virNWFilterDriverStatePtr driver);
> +                                     virNWFilterHashTablePtr filterparams);
>  
>  int virNWFilterTeardownFilter(const virDomainNetDefPtr net);
>  
> diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
> index 7e67203..093158a 100644
> --- a/src/nwfilter/nwfilter_learnipaddr.c
> +++ b/src/nwfilter/nwfilter_learnipaddr.c
> @@ -612,15 +612,15 @@ learnIPAddressThread(void *arg)
>                            "cache for interface %s"), inetaddr, req->ifname);
>              }
>  
> -            ret = virNWFilterInstantiateFilterLate(NULL,
> +            ret = virNWFilterInstantiateFilterLate(req->driver,
> +                                                   NULL,
>                                                     req->ifname,
>                                                     req->ifindex,
>                                                     req->linkdev,
>                                                     req->nettype,
>                                                     &req->macaddr,
>                                                     req->filtername,
> -                                                   req->filterparams,
> -                                                   req->driver);
> +                                                   req->filterparams);
>              VIR_DEBUG("Result from applying firewall rules on "
>                        "%s with IP addr %s : %d\n", req->ifname, inetaddr, ret);
>          }

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]