On Fri, May 25, 2012 at 10:17 PM, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
> On 2012-05-25 11:02, zwu.kernel@xxxxxxxxx wrote:
>> From: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
>>
>> Reviewed-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>
> Please don't put reviewed-by tags here before the reviewer had a chance
> to look at the code.
>
>> Signed-off-by: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
>> ---
>> net.c | 18 ++++++++++++++----
>> net.h | 12 ++++++++++++
>> net/hub.c | 23 +++++++++++++++++++++--
>> net/hub.h | 1 +
>> 4 files changed, 48 insertions(+), 6 deletions(-)
>>
>> diff --git a/net.c b/net.c
>> index 61dc28d..ae0deec 100644
>> --- a/net.c
>> +++ b/net.c
>> @@ -887,6 +887,12 @@ static const struct {
>> },
>> },
>> #endif /* CONFIG_NET_BRIDGE */
>> + [NET_CLIENT_TYPE_HUB] = {
>> + .type = "hubport",
>> + .desc = {
>> + { /* end of list */ }
>> + },
>> + },
>
> This should be obsolete now.
>
>> };
>>
>> int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
>> @@ -1068,7 +1074,7 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
>> return 0;
>> }
>>
>> -static void print_net_client(Monitor *mon, NetClientState *vc)
>> +void print_net_client(Monitor *mon, NetClientState *vc)
>> {
>> monitor_printf(mon, "%s: type=%s,%s\n", vc->name,
>> net_client_types[vc->info->type].type, vc->info_str);
>> @@ -1079,12 +1085,17 @@ void do_info_network(Monitor *mon)
>> NetClientState *nc, *peer;
>> net_client_type 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_TYPE_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_TYPE_NIC) {
>> @@ -1092,7 +1103,6 @@ void do_info_network(Monitor *mon)
>> print_net_client(mon, peer);
>> }
>> }
>> - net_hub_info(mon);
>
> Why introduce a different hub output format at all? Do it in the final
> right from the start.
>
>> }
>>
>> void qmp_set_link(const char *name, bool up, Error **errp)
>> diff --git a/net.h b/net.h
>> index 250669a..08306a4 100644
>> --- a/net.h
>> +++ b/net.h
>> @@ -112,6 +112,18 @@ 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);
>>
>> +ssize_t qemu_deliver_packet(NetClientState *sender,
>> + unsigned flags,
>> + const uint8_t *data,
>> + size_t size,
>> + void *opaque);
>> +ssize_t qemu_deliver_packet_iov(NetClientState *sender,
>> + unsigned flags,
>> + const struct iovec *iov,
>> + int iovcnt,
>> + void *opaque);
>> +
>
> I bet those two prototypes are required by some other patch (or are even
> redundant).
I have noticed this, and split out them to other patch.
>
>> +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 122de69..8c77d03 100644
>> --- a/net/hub.c
>> +++ b/net/hub.c
>> @@ -184,6 +184,25 @@ NetClientState *net_hub_find_client_by_name(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)
>> @@ -194,8 +213,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, " \\ ");
This will have the following layout:
(qemu) info network
hub 1
\ dump.0: type=dump,dump to qemu-vlan1.pcap (len=65536)
\ user.1: type=user,net=10.0.2.0,restrict=off
\ virtio-net-pci.0: type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:58
hub 0
\ user.0: type=user,net=10.0.2.0,restrict=off
\ e1000.0: type=nic,model=e1000,macaddr=52:54:00:12:34:57
virtio-net-pci.1: type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
\ ur: type=tap,ifname=tap0,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
^^^
>
> Two space too much of indention (you remove them above for the existing
It is three space, not two.
> peer \ peer outputs).
Do you mean that it should be like "monitor_printf(mon, "\\ ");" but
this indention will not match with those peers in hub case.
>
>> + print_net_client(mon, port->nc.peer);
>> }
>> }
>> }
>> diff --git a/net/hub.h b/net/hub.h
>> index ff5024a..550189b 100644
>> --- a/net/hub.h
>> +++ b/net/hub.h
>> @@ -23,5 +23,6 @@ NetClientState *net_hub_find_client_by_name(unsigned int hub_id,
>> void net_hub_info(Monitor *mon);
>> int net_hub_id_for_client(NetClientState *nc, unsigned int *id);
>> void net_hub_check_clients(void);
>> +bool net_hub_port_peer_nc(NetClientState *nc);
>>
>> #endif /* NET_HUB_H */
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
--
Regards,
Zhi Yong Wu
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
