[libvirt] [PATCH] qemu: support live update of an interface's filter

[Stefan Berger]

On 11/20/2012 01:49 PM, 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 at sagersystems.de>, who tested an earlier version, and wrote an
> initial version of the nwfilterHashTable comparison function.
>
> I didn't spend any time trying to understand the contents of the
> nwfilterHashTable entries, or whether the comparison function is
> fully/correctly comparing the entries. I would appreciate if someone
> with better knowledge of that data structure (Stefan?) could check it
> out and provide suggestions.
> ---
>   src/conf/nwfilter_params.c | 24 ++++++++++++++++++++
>   src/conf/nwfilter_params.h |  4 +++-
>   src/libvirt_private.syms   |  1 +
>   src/qemu/qemu_hotplug.c    | 55 +++++++++++++++++++++++++++++++++++++++++++---
>   4 files changed, 80 insertions(+), 4 deletions(-)
>
> diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
> index 6dc4baa..e42a54c 100644
> --- a/src/conf/nwfilter_params.c
> +++ b/src/conf/nwfilter_params.c
> @@ -764,6 +764,30 @@ err_exit:
>       return -1;
>   }
>
> +static int
> +virNWFilterHashTableCompare(const void *a, const void *b)
> +{
> +    /* need to return 0 if equal */
> +    return STRNEQ_NULLABLE(a, b);

The contents of this table should be of type virNWFilterVarValuePtr and 
therefore would have to call a function comparing two objects of this 
type. The needed function virNWFilterVarValueEqual() unfortunately does 
not exist.

> +}
> +
> +bool
> +virNWFilterHashTableEqual(virNWFilterHashTablePtr a,
> +                          virNWFilterHashTablePtr b)
> +{
> +    if (!(a || b))
> +        return true;
> +    if (!(a && b))
> +        return false;

Do you mean to compare NULL pointers above?
if (!a || !b)
     return false;

Simple equality:

if (a == b)
     return true;

> +    if (a->nNames != b->nNames)
> +        return false;

I would not test for this one. The nNames and array depends on whether 
copies of hash map entries were made. If they were added with different 
copy option, then you may end up with different size of array here.

> +    if (!(a->hashTable || b->hashTable))
> +        return true;

Did you mean the following?

if (a->hashTable == b->hashTable)
     return true;

This should be covered by a==b, otherwise something would be wrong.

> +    if (!(a->hashTable && b->hashTable))
> +        return false;

a->hashTable == NULL -> a should not exist.

> +
> +    return virHashEqual(a->hashTable, b->hashTable, virNWFilterHashTableCompare);
> +}


The rest I think should work if we get the above in order.

Regards,
     Stefan