diff mbox

[13/16] net: Make "info network" output more readable info

Message ID 1342785709-3152-14-git-send-email-stefanha@linux.vnet.ibm.com
State New
Headers show

Commit Message

Stefan Hajnoczi July 20, 2012, 12:01 p.m. UTC 
From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

Reviewed-by:   Jan Kiszka  <jan.kiszka@siemens.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 net.c     |   14 +++++++++-----
 net.h     |    1 +
 net/hub.c |   23 +++++++++++++++++++++--
 net/hub.h |    1 +
 4 files changed, 32 insertions(+), 7 deletions(-)

Comments

Laszlo Ersek July 23, 2012, 9:54 p.m. UTC | #1 
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 : "<none>");
> +            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
Stefan Hajnoczi July 24, 2012, 10:26 a.m. UTC | #2 
On Mon, Jul 23, 2012 at 10:54 PM, Laszlo Ersek <lersek@redhat.com> wrote:
Good points, will fix.
diff mbox

Patch

diff --git a/net.c b/net.c
index cdc6cc7..2eca563 100644
--- a/net.c
+++ b/net.c
@@ -856,7 +856,7 @@  void qmp_netdev_del(const char *id, Error **errp)
     qemu_opts_del(qemu_opts_find(qemu_find_opts_err("netdev", errp), id));
 }
 
-static void print_net_client(Monitor *mon, NetClientState *nc)
+void print_net_client(Monitor *mon, NetClientState *nc)
 {
     monitor_printf(mon, "%s: type=%s,%s\n", nc->name,
                    NetClientOptionsKind_lookup[nc->info->type], nc->info_str);
@@ -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;
+        }
+
         if (!peer || type == NET_CLIENT_OPTIONS_KIND_NIC) {
-            monitor_printf(mon, "  ");
             print_net_client(mon, nc);
         } /* else it's a netdev connected to a NIC, printed with the NIC */
         if (peer && type == NET_CLIENT_OPTIONS_KIND_NIC) {
-            monitor_printf(mon, "   \\ ");
+            monitor_printf(mon, " \\ ");
             print_net_client(mon, peer);
         }
     }
-    net_hub_info(mon);
 }
 
 void qmp_set_link(const char *name, bool up, Error **errp)
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 */
diff --git a/net/hub.c b/net/hub.c
index 1e88fb2..f697a78 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -214,6 +214,25 @@  NetClientState *net_hub_port_find(unsigned int hub_id)
 }
 
 /**
+ * Determine if one nc peers with one hub port
+ */
+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;
+}
+
+/**
  * Print hub configuration
  */
 void net_hub_info(Monitor *mon)
@@ -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 : "<none>");
+            monitor_printf(mon, " \\ ");
+            print_net_client(mon, port->nc.peer);
         }
     }
 }
diff --git a/net/hub.h b/net/hub.h
index d47dc96..83af09b 100644
--- a/net/hub.h
+++ b/net/hub.h
@@ -26,5 +26,6 @@  void net_hub_info(Monitor *mon);
 int net_hub_id_for_client(NetClientState *nc, unsigned int *id);
 void net_hub_check_clients(void);
 NetClientState *net_hub_port_find(unsigned int hub_id);
+bool net_hub_port_peer_nc(NetClientState *nc);
 
 #endif /* NET_HUB_H */