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

Re: [libvirt] [PATCHv2 2/2] qemu: support live update of an interface's filter



On 29.11.2012 19:30, Laine Stump wrote:
> Since we can't (currently) rely on the ability to provide blanket
> support for all possible network changes by calling the toplevel
> netdev hostside disconnect/connect functions (due to qemu only
> supporting a lockstep between initialization of host side and guest
> side of devices), in order to support live change of an interface's
> nwfilter we need to make a special purpose function to only call the
> nwfilter teardown and setup functions if the filter for an interface
> (or its parameters) changes. The pattern is nearly identical to that
> used to change the bridge that an interface is connected to.
> 
> This patch was inspired by a request from Guido Winkelmann
> <guido sagersystems de>, who tested an earlier version.
> ---
>  src/conf/nwfilter_params.c | 21 ++++++++++++++++++
>  src/conf/nwfilter_params.h |  4 +++-
>  src/libvirt_private.syms   |  1 +
>  src/qemu/qemu_hotplug.c    | 55 +++++++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 77 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
> index 3ac1303..7254519 100644
> --- a/src/conf/nwfilter_params.c
> +++ b/src/conf/nwfilter_params.c
> @@ -795,6 +795,27 @@ err_exit:
>      return -1;
>  }
>  
> +/* The general purpose function virNWFilterVarValueEqual returns a
> + * bool, but the comparison callback for virHashEqual (called below)
> + * needs to return an int of 0 for == and non-0 for !=
> + */
> +static int
> +virNWFilterVarValueCompare(const void *a, const void *b)
> +{
> +    return virNWFilterVarValueEqual((const virNWFilterVarValuePtr) a,
> +                                    (const virNWFilterVarValuePtr) b) ? 0 : 1;
> +}
> +
> +bool
> +virNWFilterHashTableEqual(virNWFilterHashTablePtr a,
> +                          virNWFilterHashTablePtr b)
> +{
> +    if (!(a || b))
> +        return true;
> +    if (!(a && b))
> +        return false;
> +    return virHashEqual(a->hashTable, b->hashTable, virNWFilterVarValueCompare);
> +}
>  
>  static bool
>  isValidVarName(const char *var)
> diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h
> index 96d3033..6c3ce54 100644
> --- a/src/conf/nwfilter_params.h
> +++ b/src/conf/nwfilter_params.h
> @@ -1,7 +1,7 @@
>  /*
>   * nwfilter_params.h: parsing and data maintenance of filter parameters
>   *
> - * Copyright (C) 2011 Red Hat, Inc.
> + * Copyright (C) 2011-2012 Red Hat, Inc.
>   * Copyright (C) 2010 IBM Corporation
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -87,6 +87,8 @@ void *virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr table,
>                                        const char *name);
>  int virNWFilterHashTablePutAll(virNWFilterHashTablePtr src,
>                                 virNWFilterHashTablePtr dest);
> +bool virNWFilterHashTableEqual(virNWFilterHashTablePtr a,
> +                               virNWFilterHashTablePtr b);
>  
>  # define VALID_VARNAME \
>    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ada73fb..5598682 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -952,6 +952,7 @@ virNWFilterIPAddrMapShutdown;
>  
>  # nwfilter_params.h
>  virNWFilterHashTableCreate;
> +virNWFilterHashTableEqual;
>  virNWFilterHashTableFree;
>  virNWFilterHashTablePut;
>  virNWFilterHashTablePutAll;
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 0bc2259..5058198 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1330,6 +1330,43 @@ cleanup:
>      return ret;
>  }
>  
> +static int
> +qemuDomainChangeNetFilter(virConnectPtr conn,
> +                          virDomainObjPtr vm,
> +                          virDomainNetDefPtr olddev,
> +                          virDomainNetDefPtr newdev)
> +{
> +    /* make sure this type of device supports filters. */
> +    switch (virDomainNetGetActualType(newdev)) {
> +    case VIR_DOMAIN_NET_TYPE_ETHERNET:
> +    case VIR_DOMAIN_NET_TYPE_BRIDGE:
> +    case VIR_DOMAIN_NET_TYPE_NETWORK:
> +        break;
> +    default:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("filters not supported on interfaces of type %s"),
> +                       virDomainNetTypeToString(virDomainNetGetActualType(newdev)));
> +        return -1;
> +    }
> +
> +    virDomainConfNWFilterTeardown(olddev);
> +
> +    if (virDomainConfNWFilterInstantiate(conn, vm->def->uuid, newdev) < 0) {
> +        virErrorPtr errobj;
> +
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       _("failed to add new filter rules to '%s' "
> +                         "- attempting to restore old rules"),
> +                       olddev->ifname);
> +        errobj = virSaveLastError();
> +        ignore_value(virDomainConfNWFilterInstantiate(conn, vm->def->uuid, olddev));
> +        virSetError(errobj);
> +        virFreeError(errobj);
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  int qemuDomainChangeNetLinkState(struct qemud_driver *driver,
>                                   virDomainObjPtr vm,
>                                   virDomainNetDefPtr dev,
> @@ -1373,6 +1410,7 @@ qemuDomainChangeNet(struct qemud_driver *driver,
>      int oldType, newType;
>      bool needReconnect = false;
>      bool needBridgeChange = false;
> +    bool needFilterChange = false;
>      bool needLinkStateChange = false;
>      bool needReplaceDevDef = false;
>      int ret = -1;
> @@ -1506,8 +1544,10 @@ qemuDomainChangeNet(struct qemud_driver *driver,
>      }
>      /* (end of device info checks) */
>  
> -    if (STRNEQ_NULLABLE(olddev->filter, newdev->filter))
> -        needReconnect = true;
> +    if (STRNEQ_NULLABLE(olddev->filter, newdev->filter) ||
> +        !virNWFilterHashTableEqual(olddev->filterparams, newdev->filterparams)) {
> +        needFilterChange = true;
> +    }
>  
>      /* bandwidth can be modified, and will be checked later */
>      /* vlan can be modified, and will be checked later */
> @@ -1665,7 +1705,16 @@ qemuDomainChangeNet(struct qemud_driver *driver,
>              goto cleanup;
>          /* we successfully switched to the new bridge, and we've
>           * determined that the rest of newdev is equivalent to olddev,
> -         * so move newdev into place, so that the  */
> +         * so move newdev into place */
> +        needReplaceDevDef = true;
> +    }
> +
> +    if (needFilterChange) {
> +        if (qemuDomainChangeNetFilter(dom->conn, vm, olddev, newdev) < 0)
> +            goto cleanup;
> +        /* we successfully switched to the new filter, and we've
> +         * determined that the rest of newdev is equivalent to olddev,
> +         * so move newdev into place */
>          needReplaceDevDef = true;
>      }
>  
> 

ACK

Michal


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