From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46502) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StQZU-0007SH-83 for qemu-devel@nongnu.org; Mon, 23 Jul 2012 17:53:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1StQZT-0001fE-5V for qemu-devel@nongnu.org; Mon, 23 Jul 2012 17:53:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47666) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StQZS-0001f6-Tu for qemu-devel@nongnu.org; Mon, 23 Jul 2012 17:53:43 -0400 Message-ID: <500DC827.6020402@redhat.com> Date: Mon, 23 Jul 2012 23:54:47 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1342785709-3152-1-git-send-email-stefanha@linux.vnet.ibm.com> <1342785709-3152-14-git-send-email-stefanha@linux.vnet.ibm.com> In-Reply-To: <1342785709-3152-14-git-send-email-stefanha@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 13/16] net: Make "info network" output more readable info List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Paolo Bonzini , Zhi Yong Wu , qemu-devel@nongnu.org, Zhi Yong Wu On 07/20/12 14:01, Stefan Hajnoczi wrote: > diff --git a/net.h b/net.h > index 7e629d3..c819de4 100644 > --- a/net.h > +++ b/net.h > @@ -100,6 +100,7 @@ void qemu_check_nic_model(NICInfo *nd, const char *model); > int qemu_find_nic_model(NICInfo *nd, const char * const *models, > const char *default_model); > > +void print_net_client(Monitor *mon, NetClientState *vc); > void do_info_network(Monitor *mon); > > /* NIC info */ The prototype should say "nc", not "vc". > @@ -867,20 +867,24 @@ void do_info_network(Monitor *mon) > NetClientState *nc, *peer; > NetClientOptionsKind type; > > - monitor_printf(mon, "Devices not on any VLAN:\n"); > + net_hub_info(mon); > + > QTAILQ_FOREACH(nc, &net_clients, next) { > peer = nc->peer; > type = nc->info->type; > + > + if (net_hub_port_peer_nc(nc)) { > + continue; > + } > + OK, hub-port-peers are dealt with before the loop, so skip them in the loop. > +bool net_hub_port_peer_nc(NetClientState *nc) > +{ > + NetHub *hub; > + NetHubPort *port; > + > + QLIST_FOREACH(hub, &hubs, next) { > + QLIST_FOREACH(port, &hub->ports, next) { > + if (nc == port->nc.peer) { > + return true; > + } > + } > + } > + > + return false; > +} I think this function should (a) be dropped, and do_info_network() should call net_hub_id_for_client(nc, NULL) == 0 instead, just like assign_name() does starting with patch 2 ("net: Use hubs for the vlan feature"). (BTW the comment in patch 2 shouldn't say "on a vlan" any more, it should say "on a hub".) Or, this function should (b) stay, but call the updated net_hub_id_for_client(), or (c) pick up the same update as net_hub_id_for_client(). > @@ -224,8 +243,8 @@ void net_hub_info(Monitor *mon) > QLIST_FOREACH(hub, &hubs, next) { > monitor_printf(mon, "hub %u\n", hub->id); > QLIST_FOREACH(port, &hub->ports, next) { > - monitor_printf(mon, " port %u peer %s\n", port->id, > - port->nc.peer ? port->nc.peer->name : ""); > + monitor_printf(mon, " \\ "); > + print_net_client(mon, port->nc.peer); > } > } Are we sure "port->nc.peer" can't be NULL any longer? print_net_client() doesn't verify it. Leastways net_hub_port_find() (called by set_vlan() from "hw/qdev-properties.c") concerns itself with a NULL port-peer (although set_vlan() might be restricted to a separate, prior "phase", for what I know.) Thanks, Laszlo