Re: [Qemu-devel] [PATCH COLO-Frame v13 36/39] filter-buffer: Introduce a helper function to enable/disable default filter

[Jason Wang]

On 12/29/2015 03:09 PM, zhanghailiang wrote:
> The default buffer filter doesn't buffer packets in default,
> but we need to buffer packets for COLO or Micro-checkpoint,
> Here we add a helper function to enable/disable filter's buffer
> capability.
>
> Signed-off-by: zhanghailiang <address@hidden>
> Cc: Jason Wang <address@hidden>
> Cc: Yang Hongyang <address@hidden>
> ---
> v12:
> - Rename the heler function to qemu_set_default_filters_status()
> v11:
> - New patch
> ---
>  include/net/filter.h |  1 +
>  include/net/net.h    |  4 ++++
>  net/filter-buffer.c  | 19 +++++++++++++++++++
>  net/net.c            | 29 +++++++++++++++++++++++++++++
>  4 files changed, 53 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 40aa38c..08aa604 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -84,4 +84,5 @@ static inline bool qemu_need_skip_netfilter(NetFilterState 
> *nf)
>  void netdev_add_default_filter_buffer(const char *netdev_id,
>                                        NetFilterDirection direction,
>                                        Error **errp);
> +void qemu_set_default_filters_status(bool enable);
>  #endif /* QEMU_NET_FILTER_H */
> diff --git a/include/net/net.h b/include/net/net.h
> index 7af3e15..5c65c45 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -125,6 +125,10 @@ NetClientState *qemu_find_vlan_client_by_name(Monitor 
> *mon, int vlan_id,
>                                                const char *client_str);
>  typedef void (*qemu_nic_foreach)(NICState *nic, void *opaque);
>  void qemu_foreach_nic(qemu_nic_foreach func, void *opaque);
> +typedef void (*qemu_netfilter_foreach)(NetFilterState *nf, void *opaque,
> +                                       Error **errp);
> +void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
> +                            Error **errp);
>  int qemu_can_send_packet(NetClientState *nc);
>  ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
>                            int iovcnt);
> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
> index 8abac94..90a50cc 100644
> --- a/net/filter-buffer.c
> +++ b/net/filter-buffer.c
> @@ -169,6 +169,25 @@ out:
>      error_propagate(errp, local_err);
>  }
>  
> +static void set_default_filter_status(NetFilterState *nf,
> +                                      void *opaque,
> +                                      Error **errp)
> +{
> +    if (!strcmp(object_get_typename(OBJECT(nf)), TYPE_FILTER_BUFFER)) {
> +        bool *status = opaque;
> +
> +        if (nf->is_default) {
> +            nf->enabled = *status;
> +        }
> +    }
> +}
> +
> +void qemu_set_default_filters_status(bool enable)
> +{
> +    qemu_foreach_netfilter(set_default_filter_status,
> +                           &enable, NULL);
> +}

The name of the function sounds a generic helper but it in fact pass a
type specific function. Consider enable is a generic property of
netfilter, we want a more generic code here.

> +
>  /*
>  * This will be used by COLO or MC FT, for which they will need
>  * to buffer the packets of VM's net devices, Here we add a default
> diff --git a/net/net.c b/net/net.c
> index fd53cfc..30946c5 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -259,6 +259,35 @@ static char *assign_name(NetClientState *nc1, const char 
> *model)
>      return g_strdup_printf("%s.%d", model, id);
>  }
>  
> +void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
> +                            Error **errp)
> +{
> +    NetClientState *nc;
> +    NetFilterState *nf;
> +
> +    QTAILQ_FOREACH(nc, &net_clients, next) {
> +        if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> +            continue;
> +        }
> +        /* FIXME: Not support multiqueue */
> +        if (nc->queue_index > 1) {
> +            error_setg(errp, "%s: multiqueue is not supported", __func__);
> +            return;
> +        }

Do we really need this? Looks like netfilter_complete() has already
checked this.

> +        QTAILQ_FOREACH(nf, &nc->filters, next) {
> +            if (func) {
> +                Error *local_err = NULL;
> +
> +                func(nf, opaque, &local_err);
> +                if (local_err) {
> +                    error_propagate(errp, local_err);
> +                    return;
> +                }
> +            }
> +        }
> +    }
> +}

Need a separate patch for this helper.

> +
>  static void qemu_net_client_destructor(NetClientState *nc)
>  {
>      g_free(nc);