|
From: | Hailiang Zhang |
Subject: | Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev |
Date: | Mon, 1 Feb 2016 17:22:03 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 |
On 2016/2/1 17:04, Jason Wang wrote:
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 <address@hidden> --- 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 stringDo 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?
Er, for COLO, we will enable all the default filter in the initialization stage, then the buffer-filter will buffer all netdev's packets, after doing a checkpoint, we will release all the buffered packets (Flush all default filters' packets). If VM is failover, we will set all default filters back to disabled status. (This is a periodic mode for COLO, different from another mode, which we will call it hybrid mode, that is based on colo-proxy, which is in developing by zhangchen) Thanks, Hailiang
For COLO, we don't want to control (use) other filters that added by users. Thanks, HailiangThanksThanks, HailiangThoughts?+ 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);...
[Prev in Thread] | Current Thread | [Next in Thread] |