From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:49914)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1aaIE4-0001Ko-Fn
	for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:26:44 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1aaIDy-0003Lv-Vr
	for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:26:40 -0500
Received: from mx1.redhat.com ([209.132.183.28]:50838)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1aaIDy-0003Lo-Px
	for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:26:34 -0500
References: <1456710366-10980-1-git-send-email-zhang.zhanghailiang@huawei.com>
	<1456710366-10980-2-git-send-email-zhang.zhanghailiang@huawei.com>
From: Jason Wang <jasowang@redhat.com>
Message-ID: <56D3F2A0.9060403@redhat.com>
Date: Mon, 29 Feb 2016 15:26:24 +0800
MIME-Version: 1.0
In-Reply-To: <1456710366-10980-2-git-send-email-zhang.zhanghailiang@huawei.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 1/2] filter: Add 'status' property for
 filter object
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: zhanghailiang <zhang.zhanghailiang@huawei.com>, qemu-devel@nongnu.org
Cc: hongyang.yang@easystack.cn



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.