On 07/20/12 14:01, Stefan Hajnoczi wrote: > @@ -679,8 +680,10 @@ void do_info_usernet(Monitor *mon) > SlirpState *s; > > QTAILQ_FOREACH(s, &slirp_stacks, entry) { > + unsigned int id; > + bool got_vlan_id = net_hub_id_for_client(&s->nc, &id) == 0; > monitor_printf(mon, "VLAN %d (%s):\n", > - s->nc.vlan ? s->nc.vlan->id : -1, > + got_vlan_id ? id : -1, > s->nc.name); > slirp_connection_info(s->slirp, mon); > }
This patch seems OK, so let me quickly posit: Reviewed-by: Laszlo Ersek <ler...@redhat.com> But this hunk made me think again about net_hub_id_for_client(). The above lookup used to take a constant number of pointer dereferences, now it takes a loop in a loop. Before: VLANClientState | VLANClientState -- VLAN -- VLANClientState | VLANClientState Now: "HubPortPeer" | HubPort | "HubPortPeer" -- HubPort -- Hub -- HubPort -- "HubPortPeer" | HubPort | "HubPortPeer" net_hub_id_for_client() seems to provide two searches in one: it can key off the hub port itself on the hub, but it can also key off the port's peer (the true net device). Couldn't we just implement net_hub_id_for_client() as follows (still providing both searches)? qemu_new_net_client() sets the peer links both ways. int net_hub_id_for_client(VLANClientState *nc, unsigned int *id) { NetHubPort *port; if (nc->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) { port = DO_UPCAST(NetHubPort, nc, nc); } else if (nc->peer != NULL && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) { port = DO_UPCAST(NetHubPort, nc, nc->peer); } else { return -ENOENT; } *id = port->hub->id; return 0; } Sorry if this is a dumb question, but I have to understand if I want to review the series sensibily. Thanks! Laszlo