Re: [Qemu-devel] [PATCH v2 1/2] filter: Add 'status' property for filter object

[Jason Wang <jasowang@redhat.com>]

On 02/29/2016 09:46 AM, zhanghailiang wrote:
> With this property, users can control if this filter is 'on'
> or 'off'. The default behavior for filter is 'on'.
>
> For some types of filters, they may need to react to status changing,
> So here, we introduced status changing callback/notifier for filter class.
>
> We will skip the disabled ('off') filter when delivering packets in net layer.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Yang Hongyang <hongyang.yang@easystack.cn>
> ---
> v2:
> - Split the processing of buffer-filter into a new patch (Jason)
> - Use 'status' instead of 'enabled' to store the filter state (Jason)
> - Rename FilterDisable() callback to FilterStatusChanged(Jason)
> ---

Thanks, looks good, just few nits.

>  include/net/filter.h |  4 ++++
>  net/filter.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
>  qemu-options.hx      |  4 +++-
>  3 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 5639976..ebef0dc 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -36,12 +36,15 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
>                                     int iovcnt,
>                                     NetPacketSent *sent_cb);
>  
> +typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp);
> +
>  typedef struct NetFilterClass {
>      ObjectClass parent_class;
>  
>      /* optional */
>      FilterSetup *setup;
>      FilterCleanup *cleanup;
> +    FilterStatusChanged *status_changed;
>      /* mandatory */
>      FilterReceiveIOV *receive_iov;
>  } NetFilterClass;
> @@ -55,6 +58,7 @@ struct NetFilterState {
>      char *netdev_id;
>      NetClientState *netdev;
>      NetFilterDirection direction;
> +    char *status;

Let's use bool instead.