[libvirt] [PATCH 09/14] nwfilter: convert the gentech driver code to use virNWFilterBinding

Jiri Denemark jdenemar at redhat.com
Mon Apr 30 15:05:40 UTC 2018


On Fri, Apr 27, 2018 at 16:25:08 +0100, Daniel P. Berrangé wrote:
> Use the virNWFilterBinding struct in the gentech driver code
> directly.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/nwfilter/nwfilter_dhcpsnoop.c      |  35 +++---
>  src/nwfilter/nwfilter_driver.c         |  21 +++-
>  src/nwfilter/nwfilter_gentech_driver.c | 211 +++++++++++++++++----------------
>  src/nwfilter/nwfilter_gentech_driver.h |  22 ++--
>  src/nwfilter/nwfilter_learnipaddr.c    |  16 +--
>  5 files changed, 168 insertions(+), 137 deletions(-)
> 
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
> index aec68ab847..dc4e3cb834 100644
> --- a/src/nwfilter/nwfilter_dhcpsnoop.c
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -486,15 +486,18 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
>  
>      /* instantiate the filters */
>  
> -    if (req->ifname)
> +    if (req->ifname) {
> +        virNWFilterBinding binding = {
> +            .portdevname = req->ifname,
> +            .linkdevname = req->linkdev,
> +            .mac = req->macaddr,
> +            .filter = req->filtername,
> +            .filterparams = req->vars,
> +        };
>          rc = virNWFilterInstantiateFilterLate(req->driver,
> -                                              NULL,
> -                                              req->ifname,
> -                                              req->ifindex,
> -                                              req->linkdev,
> -                                              &req->macaddr,
> -                                              req->filtername,
> -                                              req->vars);
> +                                              &binding,
> +                                              req->ifindex);
> +    }
>  
>   exit_snooprequnlock:
>      virNWFilterSnoopReqUnlock(req);
> @@ -873,14 +876,16 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
>          goto skip_instantiate;
>  
>      if (ipAddrLeft) {
> +        virNWFilterBinding binding = {
> +            .portdevname = req->ifname,
> +            .linkdevname = req->linkdev,
> +            .mac = req->macaddr,
> +            .filter = req->filtername,
> +            .filterparams = req->vars,
> +        };
>          ret = virNWFilterInstantiateFilterLate(req->driver,
> -                                               NULL,
> -                                               req->ifname,
> -                                               req->ifindex,
> -                                               req->linkdev,
> -                                               &req->macaddr,
> -                                               req->filtername,
> -                                               req->vars);
> +                                               &binding,
> +                                               req->ifindex);
>      } else {
>          virNWFilterVarValuePtr dhcpsrvrs =
>              virHashLookup(req->vars, NWFILTER_VARNAME_DHCPSERVER);
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index d17a8ec00b..a375e9bda8 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -642,19 +642,34 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter,
>  
>  
>  static int
> -nwfilterInstantiateFilter(const char *vmname ATTRIBUTE_UNUSED,
> +nwfilterInstantiateFilter(const char *vmname,
>                            const unsigned char *vmuuid,
>                            virDomainNetDefPtr net)
>  {
> -    return virNWFilterInstantiateFilter(driver, vmuuid, net);
> +    virNWFilterBindingPtr binding;
> +    int ret;
> +
> +    if (!(binding = virNWFilterBindingForNet(vmname, vmuuid, net)))
> +        return -1;
> +    ret = virNWFilterInstantiateFilter(driver, binding);
> +    virNWFilterBindingFree(binding);
> +    return ret;
>  }
>  
>  
>  static void
>  nwfilterTeardownFilter(virDomainNetDefPtr net)
>  {
> +    virNWFilterBinding binding = {
> +        .portdevname = net->ifname,
> +        .linkdevname = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT ?
> +                        net->data.direct.linkdev : NULL),
> +        .mac = net->mac,
> +        .filter = net->filter,
> +        .filterparams = net->filterparams,
> +    };

I think using virNWFilterBindingForNet or its variant which doesn't copy
the arguments would be a bit better than open coding it here. But for
consistency and safety reasons I'd prefer if we used
virNWFilterBindingForNet everywhere without introducing a non-copying
version.

>      if ((net->ifname) && (net->filter))
> -        virNWFilterTeardownFilter(net);
> +        virNWFilterTeardownFilter(&binding);
>  }
>  
>  
> diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> index af4411d4db..c755350586 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/src/nwfilter/nwfilter_gentech_driver.c
> @@ -577,12 +577,9 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
>  
>  /**
>   * virNWFilterDoInstantiate:
> - * @vmuuid: The UUID of the VM
>   * @techdriver: The driver to use for instantiation
> + * @binding: description of port to bind the filter to
>   * @filter: The filter to instantiate
> - * @ifname: The name of the interface to apply the rules to
> - * @vars: A map holding variable names and values used for instantiating
> - *  the filter and its subfilters.
>   * @forceWithPendingReq: Ignore the check whether a pending learn request
>   *  is active; 'true' only when the rules are applied late
>   *
> @@ -596,17 +593,13 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
>   * Call this function while holding the NWFilter filter update lock
>   */
>  static int
> -virNWFilterDoInstantiate(const unsigned char *vmuuid,
> -                         virNWFilterTechDriverPtr techdriver,
> +virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
> +                         virNWFilterBindingPtr binding,
>                           virNWFilterDefPtr filter,
> -                         const char *ifname,
>                           int ifindex,
> -                         const char *linkdev,
> -                         virHashTablePtr vars,
>                           enum instCase useNewFilter,
>                           bool *foundNewFilter,
>                           bool teardownOld,
> -                         const virMacAddr *macaddr,
>                           virNWFilterDriverStatePtr driver,
>                           bool forceWithPendingReq)
>  {
> @@ -628,14 +621,14 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid,
>      }
>  
>      rc = virNWFilterDetermineMissingVarsRec(filter,
> -                                            vars,
> +                                            binding->filterparams,
>                                              missing_vars,
>                                              useNewFilter,
>                                              driver);
>      if (rc < 0)
>          goto err_exit;
>  
> -    lv = virHashLookup(vars, NWFILTER_VARNAME_CTRL_IP_LEARNING);
> +    lv = virHashLookup(binding->filterparams, NWFILTER_VARNAME_CTRL_IP_LEARNING);
>      if (lv)
>          learning = virNWFilterVarValueGetNthValue(lv, 0);
>      else
> @@ -652,19 +645,20 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid,
>                  goto err_unresolvable_vars;
>              }
>              if (STRCASEEQ(learning, "dhcp")) {
> -                rc = virNWFilterDHCPSnoopReq(techdriver, ifname, linkdev,
> -                                             vmuuid, macaddr,
> -                                             filter->name, vars, driver);
> +                rc = virNWFilterDHCPSnoopReq(techdriver, binding->portdevname,
> +                                             binding->linkdevname,
> +                                             binding->owneruuid, &binding->mac,
> +                                             filter->name, binding->filterparams, driver);
>                  goto err_exit;
>              } else if (STRCASEEQ(learning, "any")) {
>                  if (!virNWFilterHasLearnReq(ifindex)) {
>                      rc = virNWFilterLearnIPAddress(techdriver,
> -                                                   ifname,
> +                                                   binding->portdevname,
>                                                     ifindex,
> -                                                   linkdev,
> -                                                   macaddr,
> +                                                   binding->linkdevname,
> +                                                   &binding->mac,
>                                                     filter->name,
> -                                                   vars, driver,
> +                                                   binding->filterparams, driver,
>                                                     DETECT_DHCP|DETECT_STATIC);
>                  }
>                  goto err_exit;
> @@ -688,7 +682,7 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid,
>  
>      rc = virNWFilterDefToInst(driver,
>                                filter,
> -                              vars,
> +                              binding->filterparams,
>                                useNewFilter, foundNewFilter,
>                                &inst);
>  
> @@ -705,22 +699,22 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid,
>      }
>  
>      if (instantiate) {
> -        if (virNWFilterLockIface(ifname) < 0)
> +        if (virNWFilterLockIface(binding->portdevname) < 0)
>              goto err_exit;
>  
> -        rc = techdriver->applyNewRules(ifname, inst.rules, inst.nrules);
> +        rc = techdriver->applyNewRules(binding->portdevname, inst.rules, inst.nrules);
>  
>          if (teardownOld && rc == 0)
> -            techdriver->tearOldRules(ifname);
> +            techdriver->tearOldRules(binding->portdevname);
>  
> -        if (rc == 0 && (virNetDevValidateConfig(ifname, NULL, ifindex) <= 0)) {
> +        if (rc == 0 && (virNetDevValidateConfig(binding->portdevname, NULL, ifindex) <= 0)) {
>              virResetLastError();
>              /* interface changed/disppeared */
> -            techdriver->allTeardown(ifname);
> +            techdriver->allTeardown(binding->portdevname);
>              rc = -1;
>          }
>  
> -        virNWFilterUnlockIface(ifname);
> +        virNWFilterUnlockIface(binding->portdevname);
>      }
>  
>   err_exit:
> @@ -749,14 +743,9 @@ virNWFilterDoInstantiate(const unsigned char *vmuuid,
>   */
>  static int
>  virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
> -                                   const unsigned char *vmuuid,
>                                     bool teardownOld,
> -                                   const char *ifname,
> +                                   virNWFilterBindingPtr binding,
>                                     int ifindex,
> -                                   const char *linkdev,
> -                                   const virMacAddr *macaddr,
> -                                   const char *filtername,
> -                                   virHashTablePtr filterparams,
>                                     enum instCase useNewFilter,
>                                     bool forceWithPendingReq,
>                                     bool *foundNewFilter)
> @@ -765,7 +754,6 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
>      const char *drvname = EBIPTABLES_DRIVER_ID;
>      virNWFilterTechDriverPtr techdriver;
>      virNWFilterObjPtr obj;
> -    virHashTablePtr vars, vars1;
>      virNWFilterDefPtr filter;
>      virNWFilterDefPtr newFilter;
>      char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0};
> @@ -781,29 +769,22 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
>          return -1;
>      }
>  
> -    VIR_DEBUG("filter name: %s", filtername);
> +    VIR_DEBUG("filter name: %s", binding->filter);
>  
>      if (!(obj = virNWFilterObjListFindInstantiateFilter(driver->nwfilters,
> -                                                        filtername)))
> +                                                        binding->filter)))
>          return -1;
>  
> -    virMacAddrFormat(macaddr, vmmacaddr);
> +    virMacAddrFormat(&binding->mac, vmmacaddr);
>  
> -    ipaddr = virNWFilterIPAddrMapGetIPAddr(ifname);
> +    ipaddr = virNWFilterIPAddrMapGetIPAddr(binding->portdevname);
>  

The following change should be in a separate patch with an explanation
why it is safe. Originally, we made a copy of filterparams and added
NWFILTER_STD_VAR_MAC and NWFILTER_STD_VAR_IP into the copy. But now this
code just operates directly on filterparams possibly modifying
net-filterparams. This doesn't look like something we should do IMHO.

> -    vars1 = virNWFilterCreateVarHashmap(vmmacaddr, ipaddr);
> -    if (!vars1) {
> +    if (virNWFilterVarHashmapAddStdValues(binding->filterparams,
> +                                          vmmacaddr, ipaddr) < 0) {
>          rc = -1;
>          goto err_exit;
>      }
>  
> -    vars = virNWFilterCreateVarsFrom(vars1,
> -                                     filterparams);
> -    if (!vars) {
> -        rc = -1;
> -        goto err_exit_vars1;
> -    }
> -
>      filter = virNWFilterObjGetDef(obj);
>  
>      switch (useNewFilter) {
> @@ -819,17 +800,11 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
>          break;
>      }
>  
> -    rc = virNWFilterDoInstantiate(vmuuid, techdriver, filter,
> -                                  ifname, ifindex, linkdev,
> -                                  vars, useNewFilter, foundNewFilter,
> -                                  teardownOld, macaddr, driver,
> +    rc = virNWFilterDoInstantiate(techdriver, binding, filter,
> +                                  ifindex, useNewFilter, foundNewFilter,
> +                                  teardownOld, driver,
>                                    forceWithPendingReq);
>  
> -    virHashFree(vars);
> -
> - err_exit_vars1:
> -    virHashFree(vars1);
> -
>   err_exit:
>      virNWFilterObjUnlock(obj);
>  
> @@ -839,15 +814,11 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
>  
>  static int
>  virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver,
> -                                     const unsigned char *vmuuid,
> -                                     const virDomainNetDef *net,
> +                                     virNWFilterBindingPtr binding,
>                                       bool teardownOld,
>                                       enum instCase useNewFilter,
>                                       bool *foundNewFilter)
>  {
> -    const char *linkdev = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT)
> -                          ? net->data.direct.linkdev
> -                          : NULL;
>      int ifindex;
>      int rc;
>  
> @@ -856,8 +827,8 @@ virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver,
>      /* after grabbing the filter update lock check for the interface; if
>         it's not there anymore its filters will be or are being removed
>         (while holding the lock) and we don't want to build new ones */
> -    if (virNetDevExists(net->ifname) != 1 ||
> -        virNetDevGetIndex(net->ifname, &ifindex) < 0) {
> +    if (virNetDevExists(binding->portdevname) != 1 ||
> +        virNetDevGetIndex(binding->portdevname, &ifindex) < 0) {
>          /* interfaces / VMs can disappear during filter instantiation;
>             don't mark it as an error */
>          virResetLastError();
> @@ -865,10 +836,10 @@ virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver,
>          goto cleanup;
>      }
>  
> -    rc = virNWFilterInstantiateFilterUpdate(driver, vmuuid, teardownOld,
> -                                            net->ifname, ifindex, linkdev,
> -                                            &net->mac, net->filter,
> -                                            net->filterparams, useNewFilter,
> +    rc = virNWFilterInstantiateFilterUpdate(driver, teardownOld,
> +                                            binding,
> +                                            ifindex,
> +                                            useNewFilter,
>                                              false, foundNewFilter);
>  
>   cleanup:
> @@ -880,13 +851,8 @@ virNWFilterInstantiateFilterInternal(virNWFilterDriverStatePtr driver,
>  
>  int
>  virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
> -                                 const unsigned char *vmuuid,
> -                                 const char *ifname,
> -                                 int ifindex,
> -                                 const char *linkdev,
> -                                 const virMacAddr *macaddr,
> -                                 const char *filtername,
> -                                 virHashTablePtr filterparams)
> +                                 virNWFilterBindingPtr binding,
> +                                 int ifindex)
>  {
>      int rc;
>      bool foundNewFilter = false;
> @@ -894,18 +860,17 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
>      virNWFilterReadLockFilterUpdates();
>      virMutexLock(&updateMutex);
>  
> -    rc = virNWFilterInstantiateFilterUpdate(driver, vmuuid, true,
> -                                            ifname, ifindex, linkdev,
> -                                            macaddr, filtername, filterparams,
> +    rc = virNWFilterInstantiateFilterUpdate(driver, true,
> +                                            binding, ifindex,
>                                              INSTANTIATE_ALWAYS, true,
>                                              &foundNewFilter);
>      if (rc < 0) {
>          /* something went wrong... 'DOWN' the interface */
> -        if ((virNetDevValidateConfig(ifname, NULL, ifindex) <= 0) ||
> -            (virNetDevSetOnline(ifname, false) < 0)) {
> +        if ((virNetDevValidateConfig(binding->portdevname, NULL, ifindex) <= 0) ||
> +            (virNetDevSetOnline(binding->portdevname, false) < 0)) {
>              virResetLastError();
>              /* assuming interface disappeared... */
> -            _virNWFilterTeardownFilter(ifname);
> +            _virNWFilterTeardownFilter(binding->portdevname);
>          }
>      }
>  
> @@ -918,12 +883,11 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
>  
>  int
>  virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
> -                             const unsigned char *vmuuid,
> -                             const virDomainNetDef *net)
> +                             virNWFilterBindingPtr binding)
>  {
>      bool foundNewFilter = false;
>  
> -    return virNWFilterInstantiateFilterInternal(driver, vmuuid, net,
> +    return virNWFilterInstantiateFilterInternal(driver, binding,
>                                                  1,
>                                                  INSTANTIATE_ALWAYS,
>                                                  &foundNewFilter);
> @@ -932,13 +896,12 @@ virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
>  
>  int
>  virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver,
> -                                   const unsigned char *vmuuid,
> -                                   const virDomainNetDef *net,
> +                                   virNWFilterBindingPtr binding,
>                                     bool *skipIface)
>  {
>      bool foundNewFilter = false;
>  
> -    int rc = virNWFilterInstantiateFilterInternal(driver, vmuuid, net,
> +    int rc = virNWFilterInstantiateFilterInternal(driver, binding,
>                                                    0,
>                                                    INSTANTIATE_FOLLOW_NEWFILTER,
>                                                    &foundNewFilter);
> @@ -948,7 +911,7 @@ virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver,
>  }
>  
>  static int
> -virNWFilterRollbackUpdateFilter(const virDomainNetDef *net)
> +virNWFilterRollbackUpdateFilter(virNWFilterBindingPtr binding)
>  {
>      const char *drvname = EBIPTABLES_DRIVER_ID;
>      int ifindex;
> @@ -964,17 +927,17 @@ virNWFilterRollbackUpdateFilter(const virDomainNetDef *net)
>      }
>  
>      /* don't tear anything while the address is being learned */
> -    if (virNetDevGetIndex(net->ifname, &ifindex) < 0)
> +    if (virNetDevGetIndex(binding->portdevname, &ifindex) < 0)
>          virResetLastError();
>      else if (virNWFilterHasLearnReq(ifindex))
>          return 0;
>  
> -    return techdriver->tearNewRules(net->ifname);
> +    return techdriver->tearNewRules(binding->portdevname);
>  }
>  
>  
>  static int
> -virNWFilterTearOldFilter(virDomainNetDefPtr net)
> +virNWFilterTearOldFilter(virNWFilterBindingPtr binding)
>  {
>      const char *drvname = EBIPTABLES_DRIVER_ID;
>      int ifindex;
> @@ -990,12 +953,12 @@ virNWFilterTearOldFilter(virDomainNetDefPtr net)
>      }
>  
>      /* don't tear anything while the address is being learned */
> -    if (virNetDevGetIndex(net->ifname, &ifindex) < 0)
> +    if (virNetDevGetIndex(binding->portdevname, &ifindex) < 0)
>          virResetLastError();
>      else if (virNWFilterHasLearnReq(ifindex))
>          return 0;
>  
> -    return techdriver->tearOldRules(net->ifname);
> +    return techdriver->tearOldRules(binding->portdevname);
>  }
>  
>  
> @@ -1032,11 +995,11 @@ _virNWFilterTeardownFilter(const char *ifname)
>  
>  
>  int
> -virNWFilterTeardownFilter(const virDomainNetDef *net)
> +virNWFilterTeardownFilter(virNWFilterBindingPtr binding)
>  {
>      int ret;
>      virMutexLock(&updateMutex);
> -    ret = _virNWFilterTeardownFilter(net->ifname);
> +    ret = _virNWFilterTeardownFilter(binding->portdevname);
>      virMutexUnlock(&updateMutex);
>      return ret;
>  }
> @@ -1057,12 +1020,21 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj,
>      if (virDomainObjIsActive(obj)) {
>          for (i = 0; i < vm->nnets; i++) {
>              virDomainNetDefPtr net = vm->nets[i];
> +            virNWFilterBinding binding = {
> +                .ownername = vm->name,
> +                .portdevname = net->ifname,
> +                .linkdevname = (net->type == VIR_DOMAIN_NET_TYPE_DIRECT ?
> +                                net->data.direct.linkdev : NULL),
> +                .mac = net->mac,
> +                .filter = net->filter,
> +                .filterparams = net->filterparams,
> +            };
> +            memcpy(binding.owneruuid, vm->uuid, sizeof(binding.owneruuid));

I'd prefer virNWFilterBindingForNet here too.

>              if ((net->filter) && (net->ifname)) {
>                  switch (cb->step) {
>                  case STEP_APPLY_NEW:
>                      ret = virNWFilterUpdateInstantiateFilter(cb->opaque,
> -                                                             vm->uuid,
> -                                                             net,
> +                                                             &binding,
>                                                               &skipIface);
>                      if (ret == 0 && skipIface) {
>                          /* filter tree unchanged -- no update needed */
> @@ -1074,18 +1046,17 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj,
>  
>                  case STEP_TEAR_NEW:
>                      if (!virHashLookup(cb->skipInterfaces, net->ifname))
> -                        ret = virNWFilterRollbackUpdateFilter(net);
> +                        ret = virNWFilterRollbackUpdateFilter(&binding);
>                      break;
>  
>                  case STEP_TEAR_OLD:
>                      if (!virHashLookup(cb->skipInterfaces, net->ifname))
> -                        ret = virNWFilterTearOldFilter(net);
> +                        ret = virNWFilterTearOldFilter(&binding);
>                      break;
>  
>                  case STEP_APPLY_CURRENT:
>                      ret = virNWFilterInstantiateFilter(cb->opaque,
> -                                                       vm->uuid,
> -                                                       net);
> +                                                       &binding);
>                      if (ret)
>                          virReportError(VIR_ERR_INTERNAL_ERROR,
>                                         _("Failure while applying current filter on "
> @@ -1101,3 +1072,45 @@ virNWFilterDomainFWUpdateCB(virDomainObjPtr obj,
>      virObjectUnlock(obj);
>      return ret;
>  }
> +
> +
> +virNWFilterBindingPtr virNWFilterBindingForNet(const char *vmname,

The type should be on a separate line.

> +                                               const unsigned char *vmuuid,
> +                                               virDomainNetDefPtr net)

This function would better fit in nwfilter_conf.c next to
virNWFilterBindingCopy and it could even be added in the same patch.
Or is there a good reason for having this function in
nwfilter_gentech_driver.c?

> +{
> +    virNWFilterBindingPtr ret;
> +
> +    if (VIR_ALLOC(ret) < 0)
> +        return NULL;
> +
> +    if (VIR_STRDUP(ret->ownername, vmname) < 0)
> +        goto error;
> +
> +    memcpy(ret->owneruuid, vmuuid, sizeof(ret->owneruuid));
> +
> +    if (VIR_STRDUP(ret->portdevname, net->ifname) < 0)
> +        goto error;
> +
> +    if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT &&
> +        net->data.direct.linkdev &&

VIR_STRDUP accepts NULL source.

> +        VIR_STRDUP(ret->linkdevname, net->data.direct.linkdev) < 0)
> +        goto error;
> +
> +    ret->mac = net->mac;
> +
> +    if (VIR_STRDUP(ret->filter, net->filter) < 0)
> +        goto error;
> +
> +    if (!(ret->filterparams = virNWFilterHashTableCreate(0)))
> +        goto error;
> +
> +    if (net->filterparams &&
> +        virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0)
> +        goto error;
> +
> +    return ret;
> +
> + error:
> +    virNWFilterBindingFree(ret);
> +    return NULL;
> +}
> diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h
> index 86cc677e79..0d846dc92f 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.h
> +++ b/src/nwfilter/nwfilter_gentech_driver.h
> @@ -37,25 +37,17 @@ enum instCase {
>      INSTANTIATE_FOLLOW_NEWFILTER,
>  };
>  
> -

Unrelated.

>  int virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
> -                                 const unsigned char *vmuuid,
> -                                 const virDomainNetDef *net);
> +                                 virNWFilterBindingPtr binding);
>  int virNWFilterUpdateInstantiateFilter(virNWFilterDriverStatePtr driver,
> -                                       const unsigned char *vmuuid,
> -                                       const virDomainNetDef *net,
> +                                       virNWFilterBindingPtr binding,
>                                         bool *skipIface);
>  
>  int virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
> -                                     const unsigned char *vmuuid,
> -                                     const char *ifname,
> -                                     int ifindex,
> -                                     const char *linkdev,
> -                                     const virMacAddr *macaddr,
> -                                     const char *filtername,
> -                                     virHashTablePtr filterparams);
> +                                     virNWFilterBindingPtr binding,
> +                                     int ifindex);
>  
> -int virNWFilterTeardownFilter(const virDomainNetDef *net);
> +int virNWFilterTeardownFilter(virNWFilterBindingPtr binding);
>  
>  virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr,
>                                         const virNWFilterVarValue *value);
> @@ -63,4 +55,8 @@ virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr,
>  int virNWFilterDomainFWUpdateCB(virDomainObjPtr vm,
>                                  void *data);
>  
> +virNWFilterBindingPtr virNWFilterBindingForNet(const char *vmname,
> +                                               const unsigned char *vmuuid,
> +                                               virDomainNetDefPtr net);
> +
>  #endif
> diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
> index cc3bfd971c..4b13370661 100644
> --- a/src/nwfilter/nwfilter_learnipaddr.c
> +++ b/src/nwfilter/nwfilter_learnipaddr.c
> @@ -643,19 +643,21 @@ learnIPAddressThread(void *arg)
>          virNWFilterUnlockIface(req->ifname);
>  
>          if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) {
> +            virNWFilterBinding binding = {
> +                .portdevname = req->ifname,
> +                .linkdevname = req->linkdev,
> +                .mac = req->macaddr,
> +                .filter = req->filtername,
> +                .filterparams = req->filterparams,
> +            };
>              if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) {
>                  VIR_ERROR(_("Failed to add IP address %s to IP address "
>                            "cache for interface %s"), inetaddr, req->ifname);
>              }
>  
>              ret = virNWFilterInstantiateFilterLate(req->driver,
> -                                                   NULL,
> -                                                   req->ifname,
> -                                                   req->ifindex,
> -                                                   req->linkdev,
> -                                                   &req->macaddr,
> -                                                   req->filtername,
> -                                                   req->filterparams);
> +                                                   &binding,
> +                                                   req->ifindex);
>              VIR_DEBUG("Result from applying firewall rules on "
>                        "%s with IP addr %s : %d", req->ifname, inetaddr, ret);
>              VIR_FREE(inetaddr);

Jirka




More information about the libvir-list mailing list