From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaaCW-00083p-4s for qemu-devel@nongnu.org; Mon, 29 Feb 2016 21:38:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaaCR-0001V4-Sm for qemu-devel@nongnu.org; Mon, 29 Feb 2016 21:38:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48261) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaaCR-0001Ux-MT for qemu-devel@nongnu.org; Mon, 29 Feb 2016 21:38:11 -0500 References: <1456710366-10980-1-git-send-email-zhang.zhanghailiang@huawei.com> <1456710366-10980-2-git-send-email-zhang.zhanghailiang@huawei.com> <56D3F2A0.9060403@redhat.com> <56D3F46C.7030205@huawei.com> From: Jason Wang Message-ID: <56D5007C.6080505@redhat.com> Date: Tue, 1 Mar 2016 10:37:48 +0800 MIME-Version: 1.0 In-Reply-To: <56D3F46C.7030205@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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hailiang Zhang , qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com, hongyang.yang@easystack.cn On 02/29/2016 03:34 PM, Hailiang Zhang wrote: > On 2016/2/29 15:26, Jason Wang wrote: >> >> >> 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 >>> Cc: Jason Wang >>> Cc: Yang Hongyang >>> --- >>> 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. >> > > Er, then status=true means 'on' ? false means 'off' ? > That looks odd. What about using 'bool status_on' ? Or just "bool on" :) > >> >> . >> > >