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 <zhang.zhanghaili...@huawei.com>
>>> Cc: Jason Wang <jasow...@redhat.com>
>>> Cc: Yang Hongyang <hongyang.y...@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.
>>
>
> Er, then status=true means 'on' ? false means 'off' ?
> That looks odd. What about using 'bool status_on' ?
Or just "bool on" :)
>
>>
>> .
>>
>
>
