[libvirt] [PATCH 2/3] Don't pass virConnectPtr in nwfilter 'struct domUpdateCBStruct'
Laine Stump
laine at laine.org
Thu Oct 3 14:13:32 UTC 2013
On 10/03/2013 09:06 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
>
> 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 at redhat.com>
> ---
> 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);
> }
More information about the libvir-list
mailing list