From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60984) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZDd1M-0005dE-6d for qemu-devel@nongnu.org; Fri, 10 Jul 2015 14:27:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZDd1I-0003g1-26 for qemu-devel@nongnu.org; Fri, 10 Jul 2015 14:27:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38921) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZDd1H-0003fQ-SZ for qemu-devel@nongnu.org; Fri, 10 Jul 2015 14:27:31 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 570804C09A for ; Fri, 10 Jul 2015 18:27:31 +0000 (UTC) Date: Fri, 10 Jul 2015 20:27:24 +0200 From: Thomas Huth Message-ID: <20150710202724.61198530@thh440s> In-Reply-To: <87k2uhbhua.fsf@blackfin.pond.sub.org> References: <1435161381-31521-1-git-send-email-thuth@redhat.com> <1435161381-31521-5-git-send-email-thuth@redhat.com> <87k2uhbhua.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/5] net/dump: Add dump option for netdev devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Jason Wang , qemu-devel@nongnu.org, Stefan Hajnoczi On Fri, 03 Jul 2015 13:28:45 +0200 Markus Armbruster wrote: > Thomas Huth writes: > > > So far it is not possible to dump network traffic with the "-netdev" > > option yet - this is only possible with the "-net" option and an > > internal "hub". > > This patch now fixes this ugliness by adding a proper, generic > > dumpfile parameter to the "-netdev" option. > > > > Signed-off-by: Thomas Huth > > --- > > include/net/net.h | 1 + > > net/net.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > > qapi-schema.json | 12 ++++++++++-- > > 3 files changed, 59 insertions(+), 2 deletions(-) > > > > diff --git a/include/net/net.h b/include/net/net.h > > index b265047..62abc98 100644 > > --- a/include/net/net.h > > +++ b/include/net/net.h ... > > @@ -877,6 +889,36 @@ static int net_init_nic(const NetClientOptions *opts, const char *name, > > return idx; > > } > > > > +static int netdev_init_dumpfile(const Netdev *netdev, const char *name, > > + Error **errp) > > +{ > > + NetClientState *nc; > > + int dumplen = 65536; > > + int rc; > > + > > + if (netdev->opts->kind == NET_CLIENT_OPTIONS_KIND_TAP > > + && netdev->opts->tap->has_vhost && netdev->opts->tap->vhost) { > > + error_setg(errp, "dumping does not work with vhost enabled"); > > + return -1; > > + } > > + > > + if (netdev->has_dumplen) { > > + dumplen = netdev->dumplen; > > I keep reading "dumpling" for some reason... %-) Were you hungry while reading the patch? :-) > > + } > > + > > + nc = qemu_find_netdev(name); > > + if (!nc) { > > + error_setg(errp, "failed to lookup netdev for dump"); > > + return -1; > > Can this happen? > > Hmm, see below. ... > net_client_init_fun() creates a netdev with this name. Since it doesn't > return it, we have to look it up with qemu_find_netdev(). So the answer > to "can this happen?" appears to be no. assert(!rc)? I agree, an assert() should be enough here! Thomas