From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42904) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSSuv-0004Ru-V3 for qemu-devel@nongnu.org; Thu, 20 Aug 2015 12:42:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZSSuq-0001PY-0i for qemu-devel@nongnu.org; Thu, 20 Aug 2015 12:42:17 -0400 Received: from [59.151.112.132] (port=31471 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSSuo-0001Ox-Ct for qemu-devel@nongnu.org; Thu, 20 Aug 2015 12:42:11 -0400 Message-ID: <55D60356.60603@cn.fujitsu.com> Date: Fri, 21 Aug 2015 00:41:58 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <1438915585-30367-1-git-send-email-yanghy@cn.fujitsu.com> <1438915585-30367-3-git-send-email-yanghy@cn.fujitsu.com> <55C86C82.4030706@redhat.com> In-Reply-To: <55C86C82.4030706@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 02/10] init/cleanup of netfilter object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: thuth@redhat.com, mrhines@linux.vnet.ibm.com, stefanha@redhat.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com On 08/10/2015 05:18 PM, Jason Wang wrote: > > > On 08/07/2015 10:46 AM, Yang Hongyang wrote: >> QTAILQ_ENTRY global_list but used by filter layer, so that we can >> manage all filters together. >> QTAILQ_ENTRY next used by netdev, filter belongs to the specific netdev is >> in this queue. >> This is mostly the same with init/cleanup of netdev object. >> >> Signed-off-by: Yang Hongyang >> --- >> v6: add multiqueue support (net_filter_init1) >> v5: remove model from NetFilterState >> add a sent_cb param to receive_iov API >> --- >> include/net/filter.h | 42 +++++++++++++++ >> include/net/net.h | 1 + >> include/qemu/typedefs.h | 1 + >> net/filter.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++ >> net/net.c | 1 + >> qapi-schema.json | 37 +++++++++++++ >> 6 files changed, 223 insertions(+) >> >> diff --git a/include/net/filter.h b/include/net/filter.h >> index 4242ded..7a858d8 100644 >> --- a/include/net/filter.h > [...] >> +static >> +NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = { >> +}; >> + >> +static int net_filter_init1(const NetFilter *netfilter, Error **errp) >> +{ >> + NetClientState *ncs[MAX_QUEUE_NUM]; >> + const char *name = netfilter->id; >> + const char *netdev_id = netfilter->netdev; >> + const char *chain_str = NULL; >> + const NetFilterOptions *opts = netfilter->opts; >> + int chain, queues, i; >> + >> + if (!net_filter_init_fun[opts->kind]) { >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type", >> + "a net filter type"); >> + return -1; >> + } >> + >> + if (netfilter->has_chain) { >> + chain_str = netfilter->chain; >> + if (!strcmp(chain_str, "in")) { >> + chain = NET_FILTER_IN; >> + } else if (!strcmp(chain_str, "out")) { >> + chain = NET_FILTER_OUT; >> + } else if (!strcmp(chain_str, "all")) { >> + chain = NET_FILTER_ALL; >> + } else { >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "chain", >> + "netfilter chain (in/out/all)"); >> + return -1; >> + } >> + } else { >> + /* default */ >> + chain = NET_FILTER_ALL; >> + } >> + >> + queues = qemu_find_net_clients_except(netdev_id, ncs, >> + NET_CLIENT_OPTIONS_KIND_NIC, >> + MAX_QUEUE_NUM); >> + if (queues < 1) { >> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev", >> + "a network backend id"); >> + return -1; >> + } > > Let's fail when vhost is used here. I think you mean vhost-user here? > >> + >> + for (i = 0; i < queues; i++) { >> + if (net_filter_init_fun[opts->kind](opts, name, >> + chain, ncs[i], errp) < 0) { >> + if (errp && !*errp) { >> + error_setg(errp, QERR_DEVICE_INIT_FAILED, >> + NetFilterOptionsKind_lookup[opts->kind]); >> + } >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> + >> +static int net_init_filter(void *dummy, QemuOpts *opts, Error **errp) >> +{ >> + NetFilter *object = NULL; >> + Error *err = NULL; >> + int ret = -1; >> + OptsVisitor *ov = opts_visitor_new(opts); >> + >> + visit_type_NetFilter(opts_get_visitor(ov), &object, NULL, &err); >> + opts_visitor_cleanup(ov); >> + >> + if (!err) { >> + ret = net_filter_init1(object, &err); >> + } >> + >> + if (object) { >> + QapiDeallocVisitor *dv = qapi_dealloc_visitor_new(); >> + >> + visit_type_NetFilter(qapi_dealloc_get_visitor(dv), &object, NULL, NULL); >> + qapi_dealloc_visitor_cleanup(dv); >> + } >> + >> + error_propagate(errp, err); should print out the error here instead of propagate it,otherwise the error msg is lost. >> + return ret; >> +} >> >> int net_init_filters(void) >> { >> + QTAILQ_INIT(&net_filters); >> + >> + if (qemu_opts_foreach(qemu_find_opts("netfilter"), >> + net_init_filter, NULL, NULL)) { >> + return -1; >> + } >> + >> return 0; >> } > > Then errors will be lost here? > Yes, good catch, thank you ! > . > -- Thanks, Yang.