[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH v3 11/12] monitor/net: Convert do_info_network()
From: |
Luiz Capitulino |
Subject: |
[Qemu-devel] Re: [PATCH v3 11/12] monitor/net: Convert do_info_network() to QObject/QMP |
Date: |
Fri, 23 Apr 2010 18:13:16 -0300 |
On Thu, 15 Apr 2010 11:07:06 -0300
Miguel Di Ciurcio Filho <address@hidden> wrote:
> Each device is represented by a QDict. The returned QObject is a QList
> of all devices.
>
> This commit slightly changes the monitor output when 'info network' is used.
Actually, it's much more than that.
For example, the socket (connection type) was:
(qemu) info network
VLAN 0 devices:
foobar.1: socket: connection from 127.0.0.1:41383
Devices not on any VLAN:
(qemu)
Now it became:
(qemu) info network
Devices on VLANs:
foobar.1: vlan=0 family=ipv4 service=41390 host=127.0.0.1
Devices not on any VLAN:
(qemu)
So, we're degrading the user Monitor here.
My suggestion is:
1. If a device already print "low-level" info (eg. slirp), then it's
ok to just dump the dict. Actually, this is going to be the default
behavior for new devices (when we get them)
2. If a device prints a user-friendly formatted string, then we'll have to
maintain it (eg. socket). You can do that by checking the device
model in the iterator and formatting the user-friendly string
accordingly.
Another possibility is to have a 'user_str' in VLANClientState, which
can be formatted at 'info_dict' creation time.
Also, this change:
> Old output:
> (qemu) info network
> VLAN 1 devices:
> e1000.0: model=e1000,macaddr=52:54:00:12:34:56
> VLAN 0 devices:
> tap.0: ifname=tap0,script=/etc/kvm/kvm-ifup,downscript=/etc/qemu-ifdown
> Devices not on any VLAN:
>
> New output:
> (qemu) info network
> Devices on VLANs:
> e1000.0: vlan=1 model=e1000 macaddr=52:54:00:12:34:56
> tap.0: vlan=0 script=/etc/kvm/kvm-ifup downscript=/etc/qemu-ifdown
> ifname=tap0
> Devices not on any VLAN:
doesn't look so good either, as 'info network' won't be that useful if we
get several devices and several vlans. Not sure whether this is a real scenario,
but at least the current code supports it.
An (inefficient) way to maintain the old format is:
1. Add a key with the number of vlans to the dict (say 'vlans_total')
2. In do_info_network_print() loop using vlans_total in the conditional,
like this:
for (i = 0; i < vlans_total; i++) {
monitor_printf(mon, "VLAN %d devices\n", i);
qlist_iter(...)
}
Also put 'i' either into a global variable or allocate a struct to pass
it and 'mon' down
3. In vlan_devices_iter() only print devices that correspond to vlan 'i'
>
> Signed-off-by: Miguel Di Ciurcio Filho <address@hidden>
> ---
> monitor.c | 3 +-
> net.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> net.h | 3 +-
> 3 files changed, 122 insertions(+), 11 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 4c6275e..34781ba 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2610,7 +2610,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 8c02f28..46e8873 100644
> --- a/net.c
> +++ b/net.c
> @@ -35,6 +35,8 @@
> #include "sysemu.h"
> #include "qemu-common.h"
> #include "qemu_socket.h"
> +#include "qemu-objects.h"
> +#include "qobject.h"
> #include "qdict.h"
> #include "qstring.h"
> #include "hw/qdev.h"
> @@ -1285,28 +1287,135 @@ void net_set_boot_mask(int net_boot_mask)
> }
> }
>
> -void do_info_network(Monitor *mon)
> +static void vlan_devices_iter(QObject *obj, void *opaque)
> +{
> +
> + Monitor *mon = opaque;
> + QDict *net_device = qobject_to_qdict(obj);
> +
> + if (!qdict_haskey(net_device, "vlan"))
> + return;
> +
> + monitor_printf(mon, " %s: vlan=%d ", qdict_get_str(net_device, "name"),
> + (int)qdict_get_int(net_device, "vlan"));
> + monitor_printf(mon,
> + qstring_get_str(qdict_to_qstring(qdict_get_qdict(net_device,
> "info"))));
> +
> + monitor_printf(mon, " \n");
> +}
> +
> +static void non_vlan_devices_iter(QObject *obj, void *opaque)
> +{
> +
> + Monitor *mon = opaque;
> + QDict *net_device = qobject_to_qdict(obj);
> +
> + if (qdict_haskey(net_device, "vlan"))
> + return;
> +
> + monitor_printf(mon, " %s: ", qdict_get_str(net_device, "name"));
> +
> + if (qdict_haskey(net_device, "peer"))
> + monitor_printf(mon, "peer=%s ", qdict_get_str(net_device, "peer"));
> +
> + monitor_printf(mon,
> + qstring_get_str(qdict_to_qstring(qdict_get_qdict(net_device,
> "info"))));
> +
> + monitor_printf(mon, "\n");
> +
> +}
> +
> +void do_info_network_print(Monitor *mon, const QObject *ret_data)
> +{
> + QList *qlist;
> +
> + qlist = qobject_to_qlist(ret_data);
> +
> + monitor_printf(mon, "Devices on VLANs:\n");
> +
> + qlist_iter(qlist, vlan_devices_iter, mon);
> +
> + monitor_printf(mon, "Devices not on any VLAN:\n");
> +
> + qlist_iter(qlist, non_vlan_devices_iter, mon);
> +
> +}
> +
> +/**
> + * do_network_info(): Network information
It's do_info_network()
> + *
> + * Each network device information is stored in a QDict and the
> + * returned QObject is a QList of all devices.
> + *
> + * The QDict contains the following:
> + *
> + * - "name": device name
Isn't 'name' the device id?
> + * - "vlan": only present if the device is attached to a VLAN, it is the id
> + * of the VLAN
Please, fix the indentation, like:
* - "vlan": only present if the device is attached to a VLAN, it is the id
of the VLAN
> + * - "info": it is a QDict that may contain any of the following, depending
> on
> + * the type of the device:
> + * - "model": type of the device
> + * - "macaddr": MAC address
> + * - "script": path to script used to configure the device
> + * - "downscript": path to script used to deconfigure the device
> + * - "fd": handle to the device
> + * - "ifname": name of the host device connected to the guest device
> + * - "host": IP address of a socket
> + * - "service": port of a socket
> + * - "family": Internet protocol family (IPv4 or IPv6)
> + *
> + * Example:
> + *
> + * [ { "name": "tap.0", "vlan": 0,
> + "info": { "script": "/etc/kvm/kvm-ifup", "downscript":
> +"/etc/qemu-ifdown",
> + "ifname": "tap0" } },
> + { "name": "e1000.0", "vlan": 1,
> + "info": { "model": "e1000", "macaddr": "52:54:00:12:34:56" } } ]
> + */
Please, indent this, like bdrv_info().
> +void do_info_network(Monitor *mon, QObject **ret_data)
> {
> VLANState *vlan;
> VLANClientState *vc;
> + QDict *net_device;
> + QList *device_list;
> + device_list = qlist_new();
>
> QTAILQ_FOREACH(vlan, &vlans, next) {
> - monitor_printf(mon, "VLAN %d devices:\n", vlan->id);
> + QObject *obj;
This is also used by the other loop, let's declare it as a function
variable to avoid duplication.
>
> 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 }", vlan->id,
> +vc->name);
> + net_device = qobject_to_qdict(obj);
> +
> + QINCREF(vc->info_dict);
> + qdict_put(net_device, "info", vc->info_dict);
> +
> + 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;
> + obj = qobject_from_jsonf("{ 'name': %s }", vc->name);
> + net_device = qobject_to_qdict(obj);
> +
> + QINCREF(vc->info_dict);
> + qdict_put(net_device, "info", vc->info_dict);
> +
> + if (vc->peer)
> + qdict_put(net_device, "peer", qstring_from_str(vc->peer->name));
Did you check if 'peer' is QMPable?
> +
> + qlist_append(device_list, net_device);
> }
> +
> + *ret_data = QOBJECT(device_list);
> }
>
> +
> int do_set_link(Monitor *mon, const QDict *qdict, QObject **ret_data)
> {
> VLANState *vlan;
> diff --git a/net.h b/net.h
> index d12276a..058420e 100644
> --- a/net.h
> +++ b/net.h
> @@ -119,7 +119,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);
> int do_set_link(Monitor *mon, const QDict *qdict, QObject **ret_data);
>
> /* NIC info */
- [Qemu-devel] [PATCH v3 00/12] Convert do_info_network() to QObject/QMP, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 12/12] net: Remove info_str from VLANClientState, not needed anymore, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 11/12] monitor/net: Convert do_info_network() to QObject/QMP, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] Re: [PATCH v3 11/12] monitor/net: Convert do_info_network() to QObject/QMP,
Luiz Capitulino <=
- [Qemu-devel] [PATCH v3 10/12] net: xen: use info_dict instead of info_str, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 09/12] net: socket: use info_dict instead of info_str, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 07/12] net: vde: use info_dict instead of info_str, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 08/12] net: dump: use info_dict instead of info_str, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 04/12] net: various devices: replace qemu_format_nic_info_str by qemu_format_nic_info_dict, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 06/12] net: tap/tap-win32: use info_dict instead of info_str, Miguel Di Ciurcio Filho, 2010/04/15
- [Qemu-devel] [PATCH v3 03/12] net: eepro100: replace qemu_format_nic_info_str by qemu_format_nic_info_dict, Miguel Di Ciurcio Filho, 2010/04/15