From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=32815 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OEQRL-0006PB-Lz for qemu-devel@nongnu.org; Tue, 18 May 2010 13:18:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OEQHz-0005dW-7B for qemu-devel@nongnu.org; Tue, 18 May 2010 13:09:11 -0400 Received: from fmmailgate02.web.de ([217.72.192.227]:48996) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OEQHy-0005d9-DK for qemu-devel@nongnu.org; Tue, 18 May 2010 13:09:07 -0400 Message-ID: <4BF2C9AC.4060601@web.de> Date: Tue, 18 May 2010 19:09:00 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <5133a97cd7783ca686347dba01e6ed0451ab99b6.1273843151.git.jan.kiszka@siemens.com> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigEB42379492DC116541CA91BE" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH 4/8] monitor: Add basic device state visualization List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Anthony Liguori , Juan Quintela , Avi Kivity , qemu-devel@nongnu.org, Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigEB42379492DC116541CA91BE Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Markus Armbruster wrote: > Jan Kiszka writes: >=20 >> This introduces device_show, a monitor command that saves the vmstate = of >> a qdev device and visualizes it. QMP is also supported. Buffers are cu= t >> after 16 byte by default, but the full content can be requested via >> '-f'. To pretty-print sub-arrays, vmstate is extended to store the sta= rt >> index name. >> >> Signed-off-by: Jan Kiszka >=20 > Neato! Comments inline. >=20 >> --- >> hw/hw.h | 2 + >> hw/qdev.c | 287 ++++++++++++++++++++++++++++++++++++++++++++++= +++++++++ >> hw/qdev.h | 2 + >> qemu-monitor.hx | 16 +++ >> 4 files changed, 307 insertions(+), 0 deletions(-) >> >> diff --git a/hw/hw.h b/hw/hw.h >> index 328b704..1ff8e40 100644 >> --- a/hw/hw.h >> +++ b/hw/hw.h >> @@ -299,6 +299,7 @@ enum VMStateFlags { >> =20 >> typedef struct { >> const char *name; >> + const char *start_index; >> size_t offset; >> size_t size; >> size_t start; >=20 > Why is start_index a string? It can, actually should be a symbolic constant (from e1000: "mac_reg[RA+03]: ...", ie. "RA" is the start index here). >=20 >> @@ -414,6 +415,7 @@ extern const VMStateInfo vmstate_info_unused_buffe= r; >> .size =3D sizeof(_type), = \ >> .flags =3D VMS_ARRAY, = \ >> .offset =3D vmstate_offset_sub_array(_state, _field, _type, _= start), \ >> + .start_index =3D (stringify(_start)), = \ >> } >> =20 >> #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _i= nfo, _type) {\ >> diff --git a/hw/qdev.c b/hw/qdev.c >> index fe49e71..c989010 100644 >> --- a/hw/qdev.c >> +++ b/hw/qdev.c >> @@ -29,6 +29,9 @@ >> #include "qdev.h" >> #include "sysemu.h" >> #include "monitor.h" >> +#include "qjson.h" >> +#include "qint.h" >> +#include "qbuffer.h" >> =20 >> static int qdev_hotplug =3D 0; >> =20 >> @@ -824,3 +827,287 @@ int do_device_del(Monitor *mon, const QDict *qdi= ct, QObject **ret_data) >> } >> return qdev_unplug(dev); >> } >> + >> +static int print_array_index(Monitor *mon, const char *start, int ind= ex) >> +{ >> + char index_str[32]; >> + int len; >> + >> + if (start) { >> + len =3D snprintf(index_str, sizeof(index_str), "[%s+%02x]", s= tart, >> + index); >> + } else { >> + len =3D snprintf(index_str, sizeof(index_str), "[%02x]", inde= x); >> + } >> + monitor_printf(mon, index_str); >> + return len; >=20 > You go via the snprintf() detour just because monitor_printf() doesn't > return the number of characters printed. Wouldn't it be better to fix > that? Yeah, good point. Will address this. >=20 >> +} >> + >> +#define NAME_COLUMN_WIDTH 23 >> + >> +static void print_field(Monitor *mon, const QDict *qfield, int indent= ); >> + >> +static void print_elem(Monitor *mon, const QObject *qelem, size_t siz= e, >> + int column_pos, int indent) >> +{ >> + int64_t data_size; >> + const void *data; >> + int n; >> + >> + if (qobject_type(qelem) =3D=3D QTYPE_QDICT) { >> + if (column_pos >=3D 0) { >> + monitor_printf(mon, ".\n"); >> + } >> + } else { >> + monitor_printf(mon, ":"); >> + column_pos++; >> + if (column_pos < NAME_COLUMN_WIDTH) { >> + monitor_printf(mon, "%*c", NAME_COLUMN_WIDTH - column_pos= , ' '); >> + } >> + } >> + >> + switch (qobject_type(qelem)) { >> + case QTYPE_QDICT: >> + print_field(mon, qobject_to_qdict(qelem), indent + 2); >> + break; >> + case QTYPE_QBUFFER: >> + data =3D qbuffer_get_data(qobject_to_qbuffer(qelem)); >> + data_size =3D qbuffer_get_size(qobject_to_qbuffer(qelem)); >> + for (n =3D 0; n < data_size; ) { >> + monitor_printf(mon, " %02x", *((uint8_t *)data+n)); >> + if (++n < size) { >> + if (n % 16 =3D=3D 0) { >> + monitor_printf(mon, "\n%*c", NAME_COLUMN_WIDTH, '= '); >> + } else if (n % 8 =3D=3D 0) { >> + monitor_printf(mon, " -"); >> + } >> + } >> + } >> + if (data_size < size) { >> + monitor_printf(mon, " ..."); >> + } >> + monitor_printf(mon, "\n"); >> + break; >> + case QTYPE_QINT: >> + monitor_printf(mon, " %0*x\n", (int)size * 2, >> + (int)qint_get_int(qobject_to_qint(qelem))); >=20 > I'm confused. Doesn't casting qint_get_int() to int lose bits? You are right, will fix! >=20 >> + break; >> + default: >> + assert(0); >> + } >> +} >> + >> +static void print_field(Monitor *mon, const QDict *qfield, int indent= ) >> +{ >> + const char *name =3D qdict_get_str(qfield, "name"); >> + const char *start =3D qdict_get_try_str(qfield, "start"); >> + int64_t size =3D qdict_get_int(qfield, "size"); >> + QList *qlist =3D qdict_get_qlist(qfield, "elems"); >> + QListEntry *entry, *sub_entry; >> + QList *sub_list; >> + int elem_no =3D 0; >> + >> + QLIST_FOREACH_ENTRY(qlist, entry) { >> + QObject *qelem =3D qlist_entry_obj(entry); >> + int pos =3D indent + strlen(name); >> + >> + if (qobject_type(qelem) =3D=3D QTYPE_QLIST) { >> + monitor_printf(mon, "%*c%s", indent, ' ', name); >> + pos +=3D print_array_index(mon, start, elem_no); >> + sub_list =3D qobject_to_qlist(qelem); >> + QLIST_FOREACH_ENTRY(sub_list, sub_entry) { >> + print_elem(mon, qlist_entry_obj(sub_entry), size, pos= , >> + indent + 2); >> + pos =3D -1; >> + } >> + } else { >> + if (elem_no =3D=3D 0) { >> + monitor_printf(mon, "%*c%s", indent, ' ', name); >> + } else { >> + pos =3D -1; >> + } >> + print_elem(mon, qelem, size, pos, indent); >> + } >> + elem_no++; >> + } >> +} >> + >> +void device_user_print(Monitor *mon, const QObject *data) >> +{ >> + QDict *qdict =3D qobject_to_qdict(data); >> + QList *qlist =3D qdict_get_qlist(qdict, "fields"); >> + QListEntry *entry; >> + >> + monitor_printf(mon, "dev: %s, id \"%s\"\n", >> + qdict_get_str(qdict, "device"), >> + qdict_get_str(qdict, "id")); >> + >> + QLIST_FOREACH_ENTRY(qlist, entry) { >> + print_field(mon, qobject_to_qdict(qlist_entry_obj(entry)), 2)= ; >> + } >> +} >> + >> +static void parse_vmstate(const VMStateDescription *vmsd, void *opaqu= e, >> + QList *qlist, int full_buffers) >> +{ >> + VMStateField *field =3D vmsd->fields; >> + >> + if (vmsd->pre_save) { >> + vmsd->pre_save(opaque); >> + } >> + while(field->name) { >> + if (!field->field_exists || >> + field->field_exists(opaque, vmsd->version_id)) { >> + void *base_addr =3D opaque + field->offset; >> + int i, n_elems =3D 1; >> + int is_array =3D 1; >> + size_t size =3D field->size; >> + QDict *qfield =3D qdict_new(); >> + QList *qelems =3D qlist_new(); >> + >> + qlist_append_obj(qlist, QOBJECT(qfield)); >> + >> + qdict_put_obj(qfield, "name", >> + QOBJECT(qstring_from_str(field->name))); >> + qdict_put_obj(qfield, "elems", QOBJECT(qelems)); >> + >> + if (field->flags & VMS_VBUFFER) { >> + size =3D *(int32_t *)(opaque + field->size_offset); >> + if (field->flags & VMS_MULTIPLY) { >> + size *=3D field->size; >> + } >> + } >> + qdict_put_obj(qfield, "size", QOBJECT(qint_from_int(size)= )); >> + if (field->start_index) { >> + qdict_put_obj(qfield, "start", >> + QOBJECT(qstring_from_str(field->start_i= ndex))); >> + } >> + >> + if (field->flags & VMS_ARRAY) { >> + n_elems =3D field->num; >> + } else if (field->flags & VMS_VARRAY_INT32) { >> + n_elems =3D *(int32_t *)(opaque + field->num_offset);= >> + } else if (field->flags & VMS_VARRAY_UINT16) { >> + n_elems =3D *(uint16_t *)(opaque + field->num_offset)= ; >> + } else { >> + is_array =3D 0; >> + } >> + if (field->flags & VMS_POINTER) { >> + base_addr =3D *(void **)base_addr + field->start; >> + } >> + for (i =3D 0; i < n_elems; i++) { >> + void *addr =3D base_addr + size * i; >> + QList *sub_elems =3D qelems; >> + int val; >> + >> + if (is_array) { >> + sub_elems =3D qlist_new(); >> + qlist_append_obj(qelems, QOBJECT(sub_elems)); >> + } >> + if (field->flags & VMS_ARRAY_OF_POINTER) { >> + addr =3D *(void **)addr; >> + } >> + if (field->flags & VMS_STRUCT) { >> + parse_vmstate(field->vmsd, addr, sub_elems, full_= buffers); >> + } else { >> + if (field->flags & (VMS_BUFFER | VMS_VBUFFER)) { >> + if (!full_buffers && size > 16) { >> + size =3D 16; >> + } >> + qlist_append_obj(sub_elems, >> + QOBJECT(qbuffer_from_data(ad= dr, >> + si= ze))); >> + } else { >> + switch (size) { >> + case 1: >> + val =3D *(uint8_t *)addr; >> + break; >> + case 2: >> + val =3D *(uint16_t *)addr; >> + break; >> + case 4: >> + val =3D *(uint32_t *)addr; >> + break; >> + case 8: >> + val =3D *(uint64_t *)addr; >> + break; >> + default: >> + assert(0); >> + } >> + qlist_append_obj(sub_elems, >> + QOBJECT(qint_from_int(val)))= ; >> + } >> + } >> + } >> + } >> + field++; >> + } >> +} >> + >> +static DeviceState *qdev_find(const char *path) >> +{ >> + const char *dev_name; >> + DeviceState *dev; >> + char *bus_path; >> + BusState *bus; >> + >> + dev_name =3D strrchr(path, '/'); >> + if (!dev_name) { >> + bus =3D main_system_bus; >> + dev_name =3D path; >> + } else { >> + dev_name++; >> + bus_path =3D qemu_strdup(path); >> + bus_path[dev_name - path] =3D 0; >> + >> + bus =3D qbus_find(bus_path); >> + qemu_free(bus_path); >> + >> + if (!bus) { >> + /* qbus_find already reported the error */ >> + return NULL; >> + } >> + } >> + dev =3D qbus_find_dev(bus, dev_name); >> + if (!dev) { >> + qerror_report(QERR_DEVICE_NOT_FOUND, dev_name); >> + if (!monitor_cur_is_qmp()) { >> + qbus_list_dev(bus); >> + } >> + } >> + return dev; >> +} >> + >> +int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_da= ta) >=20 > Command documentation missing. In the make (depends on QMP doc infrastructure). >=20 >> +{ >> + const char *path =3D qdict_get_str(qdict, "path"); >> + const VMStateDescription *vmsd; >> + DeviceState *dev; >> + QList *qlist; >> + >> + dev =3D qdev_find_recursive(main_system_bus, path); >> + if (!dev) { >> + dev =3D qdev_find(path); >> + if (!dev) { >> + return -1; >> + } >> + } >=20 > This is inconsistent with how do_device_del() looks up devices by ID. > Let's agree on one lookup, put it into a function, and use it > everywhere. Will adjust device_del to the same scheme. BTW, qdev path completion will also be available for both commands in my next series. >=20 >> + >> + vmsd =3D dev->info->vmsd; >> + if (!vmsd) { >> + qerror_report(QERR_UNDEFINED_ERROR); >> + if (!monitor_cur_is_qmp()) { >> + error_printf_unless_qmp("Device '%s' does not support sta= te " >> + "dumping\n", path); >> + } >=20 > Please use a suitable QERR_. Define one if necessary. Already fixed in my tree (forgot this bit for v1). Just this redundant qmp check was still present. >=20 >> + return -1; >> + } >> + >> + *ret_data =3D qobject_from_jsonf("{ 'device': %s, 'id': %s }", >> + dev->info->name, dev->id ? : ""); >> + qlist =3D qlist_new(); >> + parse_vmstate(vmsd, dev, qlist, qdict_get_int(qdict, "full")); >> + qdict_put_obj(qobject_to_qdict(*ret_data), "fields", QOBJECT(qlis= t)); >> + >> + return 0; >> +} >> diff --git a/hw/qdev.h b/hw/qdev.h >> index d8fbc73..f8436ec 100644 >> --- a/hw/qdev.h >> +++ b/hw/qdev.h >> @@ -177,6 +177,8 @@ void do_info_qtree(Monitor *mon); >> void do_info_qdm(Monitor *mon); >> int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_dat= a); >> int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_dat= a); >> +void device_user_print(Monitor *mon, const QObject *data); >> +int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_da= ta); >> =20 >> /*** qdev-properties.c ***/ >> =20 >> diff --git a/qemu-monitor.hx b/qemu-monitor.hx >> index a8f194c..449f012 100644 >> --- a/qemu-monitor.hx >> +++ b/qemu-monitor.hx >> @@ -599,6 +599,22 @@ Remove device @var{id}. >> ETEXI >> =20 >> { >> + .name =3D "device_show", >> + .args_type =3D "path:s,full:-f", >> + .params =3D "device [-f]", >> + .help =3D "show device state (specify -f for full buffe= r dumping)", >> + .user_print =3D device_user_print, >> + .mhandler.cmd_new =3D do_device_show, >> + }, >> + >> +STEXI >> +@item device_show @var{id} [@code{-f}] >> + >> +Show state of device @var{id} in a human-readable form. Buffers are c= ut after >> +16 bytes unless a full dump is requested via @code{-f}=20 >> +ETEXI >> + >> + { >> .name =3D "cpu", >> .args_type =3D "index:i", >> .params =3D "index", >=20 >=20 Thanks for the comments! Jan --------------enigEB42379492DC116541CA91BE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkvyybAACgkQitSsb3rl5xQ1hACg55GbI6blMDLlCEN6nlrJo10x PbAAn1tSNY3vlbUVlmkpXeDciqTFOU4H =jrQg -----END PGP SIGNATURE----- --------------enigEB42379492DC116541CA91BE--