On 02/01/2016 03:56 PM, Hailiang Zhang wrote:
> On 2016/2/1 15:46, Jason Wang wrote:
>>
>>
>> On 02/01/2016 02:13 PM, Hailiang Zhang wrote:
>>> On 2016/2/1 11:14, Jason Wang wrote:
>>>>
>>>>
>>>> On 01/27/2016 04:29 PM, zhanghailiang wrote:
>>>>> We add a new helper function netdev_add_filter(), this function
>>>>> can help adding a filter object to a netdev.
>>>>> Besides, we add a is_default member for struct NetFilterState
>>>>> to indicate whether the filter is default or not.
>>>>>
>>>>> Signed-off-by: zhanghailiang <[email protected]>
>>>>> ---
>>>>> v2:
>>>>> -Re-implement netdev_add_filter() by re-using object_create()
>>>>> (Jason's suggestion)
>>>>> ---
>>>>> include/net/filter.h | 7 +++++
>>>>> net/filter.c | 80
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 87 insertions(+)
>>>>>
>>>>>
[...]
>>>>> +
>>>>> + optarg =
>>>>> g_strdup_printf("qom-type=%s,id=%s,netdev=%s,status=%s",
>>>>> + filter_type, id, netdev_id, is_default ? "disable" :
>>>>> "enable"
>>>>
>>>> Instead of this, I wonder maybe it's better to:
>>>>
>>>> - store the default filter property into a pointer to string
>>>
>>> Do you mean, pass a string parameter which stores the filter property
>>> instead of
>>> assemble it in this helper ?
>>
>> Yes. E.g just a global string which could be changed by any subsystem.
>> E.g colo may change it to "filter-buffer,interval=0,status=disable". But
>> filter ids need to be generated automatically.
>>
>
> Got it. Then we don't need the global default_netfilter_type[] in
> patch 5,
Yes.
> Just use this global string instead ?
Right.
>
>>>
>>>> - colo code may change the pointer to "filter-buffer,status=disable"
>>>>
>>>
>>>> Then, there's no need for lots of codes above:
>>>> - no need a "is_default" parameter in netdev_add_filter which does not
>>>> scale consider we may want to have more property in the future
>>>> - no need to hacking like "qemu_filter_opts"
>>>
>>> Yes, we can use qemu_find_opts("object") instead of it.
>>>
>>>> - no need to have a special flag like "is_default"
>>>>
>>>
>>> But we have to distinguish the default filter from the common
>>> filter, use the name (id) to distinguish it ?
>>
>> What's the reason that you want to distinguish default filters from
>> others?
>>
>
> The default filters will be used by COLO or MC, (In COLO, we will use it
> to control packets buffering/releasing).
A question is how will you do this?
> For COLO, we don't want to control (use) other filters that added by
> users.
>
> Thanks,
> Hailiang
>
>> Thanks
>>
>>>
>>> Thanks,
>>> Hailiang
>>>
>>>> Thoughts?
>>>>
>>>>> + opts = qemu_opts_parse_noisily(&qemu_filter_opts,
>>>>> + optarg, false);
>>>>> + if (!opts) {
>>>>> + error_report("Failed to parse param '%s'", optarg);
>>>>> + exit(1);
>>>>> + }
>>>>> + g_free(optarg);
>>>>> + if (object_create(NULL, opts, &err) < 0) {
>>>>> + error_report("Failed to create object");
>>>>> + goto out_clean;
>>>>> + }
>>>>> + filter_set_default_flag(id, is_default, &err);
>>>>> +
>>>>> +out_clean:
>>>>> + qemu_opts_del(opts);
>>>>> + if (err) {
>>>>> + error_propagate(errp, err);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> static void netfilter_finalize(Object *obj)
>>>>> {
>>>>> NetFilterState *nf = NETFILTER(obj);
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>>
>>
>> .
>>
>
>
>
