From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NypxY-0006u0-5S for qemu-devel@nongnu.org; Mon, 05 Apr 2010 13:19:36 -0400 Received: from [140.186.70.92] (port=53706 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NypxV-0006s2-Hb for qemu-devel@nongnu.org; Mon, 05 Apr 2010 13:19:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1NypxN-0007o5-RN for qemu-devel@nongnu.org; Mon, 05 Apr 2010 13:19:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48180) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1NypxN-0007nr-JP for qemu-devel@nongnu.org; Mon, 05 Apr 2010 13:19:25 -0400 Date: Mon, 5 Apr 2010 14:19:14 -0300 From: Luiz Capitulino Message-ID: <20100405141914.10db4728@redhat.com> In-Reply-To: <1270330321-13279-1-git-send-email-miguel.filho@gmail.com> References: <1270330321-13279-1-git-send-email-miguel.filho@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] net: Convert do_info_network() to QObject List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Miguel Di Ciurcio Filho Cc: armbru@redhat.com, qemu-devel@nongnu.org On Sat, 3 Apr 2010 18:32:01 -0300 Miguel Di Ciurcio Filho wrote: > Each device is represented by a QDict. The returned QObject is a QList > of all devices. > > This commit should not change user output. > > Signed-off-by: Miguel Di Ciurcio Filho > --- > > This is my initial contribution, aiming at participating in Summer of Code. > > monitor.c | 3 +- > net.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------- > net.h | 4 ++- > 3 files changed, 69 insertions(+), 13 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 822dc27..0b3fdfc 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2604,7 +2604,8 @@ static const mon_cmd_t info_cmds[] = { > .args_type = "", > .params = "", > .help = "show the network state", > - .mhandler.info = do_info_network, > + .user_print = do_info_network_print, > + .mhandler.info_new = do_info_network, > }, > { > .name = "chardev", > diff --git a/net.c b/net.c > index 3ede738..7698b0c 100644 > --- a/net.c > +++ b/net.c > @@ -35,6 +35,7 @@ > #include "sysemu.h" > #include "qemu-common.h" > #include "qemu_socket.h" > +#include "qemu-objects.h" > #include "hw/qdev.h" > > static QTAILQ_HEAD(, VLANState) vlans; > @@ -1218,26 +1219,78 @@ void net_set_boot_mask(int net_boot_mask) > } > } > > -void do_info_network(Monitor *mon) > +void do_info_network_print(Monitor *mon, const QObject *ret_data) > +{ > + QList *qlist; > + QListEntry *entry; > + QDict *net_device; > + > + qlist = qobject_to_qlist(ret_data); > + net_device = qdict_new(); Why allocating a new qdict here? > + > + QTAILQ_FOREACH(entry, &qlist->head, next) { While I understand that this is convenient, it assumes (and exposes) that qlist uses QTAILQ internally, this code will break if we change to something else. Better to use qlist's own transverse functions (please, check qlist.h). (to be honest, it's arguable if we should allow that, but let's make this commit do what the current code does) > + net_device = qobject_to_qdict(entry->value); > + > + if (!qdict_haskey(net_device, "vlan")) > + continue; > + > + net_device = qobject_to_qdict(entry->value); > + monitor_printf(mon, "VLAN %d devices:\n", (int)qdict_get_int(net_device, "vlan")); > + monitor_printf(mon, " %s: %s\n", qdict_get_str(net_device, "name"), qdict_get_str(net_device, "info")); > + } You're printing the header "VLAN %d devices:" for each device, please move it outside the loop. Also, as we have talked in private, 'info_str' should be a qdict (and not a string to be parsed). > + > + monitor_printf(mon, "Devices not on any VLAN:\n"); > + > + QTAILQ_FOREACH(entry, &qlist->head, next) { > + net_device = qobject_to_qdict(entry->value); > + > + if (qdict_haskey(net_device, "vlan")) > + continue; > + > + net_device = qobject_to_qdict(entry->value); > + monitor_printf(mon, " %s: %s", qdict_get_str(net_device, "name"), qdict_get_str(net_device, "info")); > + > + if (qdict_haskey(net_device, "peer")) > + monitor_printf(mon, " peer=%s", qdict_get_str(net_device, "peer")); > + > + monitor_printf(mon, "\n"); > + } > + > +} > + > +void do_info_network(Monitor *mon, QObject **ret_data) > { Please, add documentation as in bdrv_info() and others converted functions. > VLANState *vlan; > VLANClientState *vc; > - > + QList *device_list; > + device_list = qlist_new(); > + > QTAILQ_FOREACH(vlan, &vlans, next) { > - monitor_printf(mon, "VLAN %d devices:\n", vlan->id); > - > + QObject *obj; > + > QTAILQ_FOREACH(vc, &vlan->clients, next) { > - monitor_printf(mon, " %s: %s\n", vc->name, vc->info_str); > + > + obj = qobject_from_jsonf("{ 'vlan': %d, 'name': %s, 'info': %s}", vlan->id, vc->name, vc->info_str); > + > + qlist_append(device_list, qobject_to_qdict(obj)); > + > } > } > - monitor_printf(mon, "Devices not on any VLAN:\n"); > + > QTAILQ_FOREACH(vc, &non_vlan_clients, next) { > - monitor_printf(mon, " %s: %s", vc->name, vc->info_str); > - if (vc->peer) { > - monitor_printf(mon, " peer=%s", vc->peer->name); > - } > - monitor_printf(mon, "\n"); > + QObject *obj; > + QDict *net_device; > + > + obj = qobject_from_jsonf("{ 'name': %s, 'info': %s }", vc->name, vc->info_str); > + net_device = qobject_to_qdict(obj); > + > + if (vc->peer) > + qdict_put(net_device, "peer", qstring_from_str(vc->peer->name)); > + > + qlist_append(device_list, net_device); > } > + > + *ret_data = QOBJECT(device_list); So, this returns a list of all devices, each of them is a qdict. If the device is on vlan, it will have a vlan key. This is simple and looks ok to me. > } > > void do_set_link(Monitor *mon, const QDict *qdict) > diff --git a/net.h b/net.h > index 16f19c5..f16c2d6 100644 > --- a/net.h > +++ b/net.h > @@ -6,6 +6,7 @@ > #include "qemu-common.h" > #include "qdict.h" > #include "qemu-option.h" > +#include "qobject.h" > #include "net/queue.h" > > struct MACAddr { > @@ -117,7 +118,8 @@ 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 do_info_network(Monitor *mon); > +void do_info_network_print(Monitor *mon, const QObject *ret_data); > +void do_info_network(Monitor *mon, QObject **ret_data); > void do_set_link(Monitor *mon, const QDict *qdict); > > /* NIC info */