From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NCYrK-0004Ec-IP for qemu-devel@nongnu.org; Mon, 23 Nov 2009 08:21:38 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NCYrF-0004CR-JV for qemu-devel@nongnu.org; Mon, 23 Nov 2009 08:21:37 -0500 Received: from [199.232.76.173] (port=36889 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NCYrF-0004CK-DP for qemu-devel@nongnu.org; Mon, 23 Nov 2009 08:21:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49463) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NCYrE-0007FY-3p for qemu-devel@nongnu.org; Mon, 23 Nov 2009 08:21:32 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nANDLVtq032169 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 23 Nov 2009 08:21:31 -0500 Date: Mon, 23 Nov 2009 11:21:23 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 11/17] block: Convert bdrv_info() to QObject Message-ID: <20091123112123.06338f41@doriath> In-Reply-To: References: <1258489944-12159-1-git-send-email-lcapitulino@redhat.com> <1258489944-12159-12-git-send-email-lcapitulino@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org On Fri, 20 Nov 2009 15:06:26 +0100 Markus Armbruster wrote: > Luiz Capitulino writes: > > > Each block device information is stored in a QDict and the > > returned QObject is a QList of all devices. > > > > This commit should not change user output. > > > > Signed-off-by: Luiz Capitulino > > --- > > Makefile | 2 +- > > block.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++---------- > > block.h | 4 +- > > monitor.c | 3 +- > > 4 files changed, 109 insertions(+), 23 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 6be75a1..3424cdb 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -80,7 +80,7 @@ qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o > > # block-obj-y is code used by both qemu system emulation and qemu-img > > > > block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o > > -block-obj-y += nbd.o block.o aio.o aes.o osdep.o > > +block-obj-y += nbd.o block.o aio.o aes.o osdep.o $(qobject-obj-y) > > block-obj-$(CONFIG_POSIX) += posix-aio-compat.o > > block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o > > > > Both $(block-obj-y) and $(qobject-obj-y) go into obj-y, which thus list > all the $(qobject-obj-y) objects twice. Sure that's okay? $(qobject-obj-y) is a dependency of qemu- block tools, it can be moved there then. > > diff --git a/block.c b/block.c > > index 6fdabff..fc4e2f2 100644 > > --- a/block.c > > +++ b/block.c > > @@ -26,6 +26,7 @@ > > #include "monitor.h" > > #include "block_int.h" > > #include "module.h" > > +#include "qemu-objects.h" > > > > #ifdef CONFIG_BSD > > #include > > @@ -1133,43 +1134,125 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, > > return bs->drv->bdrv_is_allocated(bs, sector_num, nb_sectors, pnum); > > } > > > > -void bdrv_info(Monitor *mon) > > +static void bdrv_print_dict(QObject *obj, void *opaque) > > { > > + QDict *bs_dict; > > + Monitor *mon = opaque; > > + > > + bs_dict = qobject_to_qdict(obj); > > + > > + monitor_printf(mon, "%s: type=%s removable=%d", > > + qdict_get_str(bs_dict, "device"), > > + qdict_get_str(bs_dict, "type"), > > + qdict_get_bool(bs_dict, "removable")); > > + > > + if (qdict_get_bool(bs_dict, "removable")) { > > + monitor_printf(mon, " locked=%d", (int)qdict_get_bool(bs_dict, "locked")); > > qdict_get_bool() returns int, no need to cast. Right. > > + } > > + > > + if (qdict_haskey(bs_dict, "inserted")) { > > + QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); > > + > > + monitor_printf(mon, " file="); > > + monitor_print_filename(mon, qdict_get_str(qdict, "file")); > > + if (qdict_haskey(qdict, "backing_file")) { > > + monitor_printf(mon, " backing_file="); > > + monitor_print_filename(mon, qdict_get_str(qdict, "backing_file")); > > + } > > + monitor_printf(mon, " ro=%d drv=%s encrypted=%d", > > + qdict_get_bool(qdict, "ro"), > > + qdict_get_str(qdict, "drv"), > > + qdict_get_bool(qdict, "encrypted")); > > + } else { > > + monitor_printf(mon, " [not inserted]"); > > + } > > + > > + monitor_printf(mon, "\n"); > > +} > > + > > +void bdrv_info_print(Monitor *mon, const QObject *data) > > +{ > > + qlist_iter(qobject_to_qlist(data), bdrv_print_dict, mon); > > +} > > + > > +/** > > + * bdrv_info(): Block devices information > > + * > > + * Each block device information is stored in a QDict and the > > + * returned QObject is a QList of all devices. > > + * > > + * The QDict contains the following: > > + * > > + * - "device": device name > > + * - "type": device type > > + * - "removable": 1 if the device is removable 0 otherwise > > + * - "locked": 1 if the device is locked 0 otherwise > > Use above and example below suggest that "locked" is only present if > device is removable. Document? Yes. > > + * - "inserted": only present if the device is inserted, it is a QDict > > + * containing the following: > > + * - "file": device file name > > + * - "ro": 1 if read-only 0 otherwise > > + * - "drv": driver format name > > + * - "backing_file": backing file name if one is used > > + * - "encrypted": 1 if encrypted 0 otherwise > > + * > > + * Example: > > + * > > + * [ { "device": "ide0-hd0", "type": "hd", "removable": 0, > > + * "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 } > > Shouldn't "file" & friends ne in nested dictionary "inserted"? Yes, will fix. > > - monitor_printf(mon, " removable=%d", bs->removable); > > - if (bs->removable) { > > - monitor_printf(mon, " locked=%d", bs->locked); > > - } > > + > > + bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': %s, " > > + "'removable': %i, 'locked': %i }", > > + bs->device_name, type, bs->removable, > > + bs->locked); > > + assert(bs_obj != NULL); > > Failure modes of qobject_from_jsonf()? I'm asking because depending on > the answer assert() may not be appropriate here. As far as I know it will fail on wrong syntax.