| Message ID | 1258489944-12159-12-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
Luiz Capitulino <lcapitulino@redhat.com> 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 <lcapitulino@redhat.com> > --- > 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? > 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 <sys/types.h> > @@ -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. > + } > + > + 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? > + * - "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"? > + * { "device": "floppy0", "type": "floppy", "removable": 1, > + * "locked": 0 } ] > + */ > +void bdrv_info(Monitor *mon, QObject **ret_data) > +{ > + QList *bs_list; > BlockDriverState *bs; > > + bs_list = qlist_new(); > + > for (bs = bdrv_first; bs != NULL; bs = bs->next) { > - monitor_printf(mon, "%s:", bs->device_name); > - monitor_printf(mon, " type="); > + QObject *bs_obj; > + const char *type = "unknown"; > + > switch(bs->type) { > case BDRV_TYPE_HD: > - monitor_printf(mon, "hd"); > + type = "hd"; > break; > case BDRV_TYPE_CDROM: > - monitor_printf(mon, "cdrom"); > + type = "cdrom"; > break; > case BDRV_TYPE_FLOPPY: > - monitor_printf(mon, "floppy"); > + type = "floppy"; > break; > } This changes human-readable output for bad bs->type from "type=" to "type=unknown". Fine with me. > - 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. > + > if (bs->drv) { > - monitor_printf(mon, " file="); > - monitor_print_filename(mon, bs->filename); > + QObject *obj; > + QDict *bs_dict = qobject_to_qdict(bs_obj); > + > + obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, " > + "'encrypted': %i }", > + bs->filename, bs->read_only, > + bs->drv->format_name, > + bdrv_is_encrypted(bs)); > + assert(obj != NULL); Ditto. > if (bs->backing_file[0] != '\0') { > - monitor_printf(mon, " backing_file="); > - monitor_print_filename(mon, bs->backing_file); > + QDict *qdict = qobject_to_qdict(obj); > + qdict_put(qdict, "backing_file", > + qstring_from_str(bs->backing_file)); > } > - monitor_printf(mon, " ro=%d", bs->read_only); > - monitor_printf(mon, " drv=%s", bs->drv->format_name); > - monitor_printf(mon, " encrypted=%d", bdrv_is_encrypted(bs)); > - } else { > - monitor_printf(mon, " [not inserted]"); > + > + qdict_put_obj(bs_dict, "inserted", obj); > } > - monitor_printf(mon, "\n"); > + qlist_append_obj(bs_list, bs_obj); > } > + > + *ret_data = QOBJECT(bs_list); > } > > /* The "info blockstats" command. */ > diff --git a/block.h b/block.h > index 2d4f066..3ca7112 100644 > --- a/block.h > +++ b/block.h > @@ -4,6 +4,7 @@ > #include "qemu-aio.h" > #include "qemu-common.h" > #include "qemu-option.h" > +#include "qobject.h" > > /* block.c */ > typedef struct BlockDriver BlockDriver; > @@ -41,7 +42,8 @@ typedef struct QEMUSnapshotInfo { > > #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB) > > -void bdrv_info(Monitor *mon); > +void bdrv_info_print(Monitor *mon, const QObject *data); > +void bdrv_info(Monitor *mon, QObject **ret_data); > void bdrv_info_stats(Monitor *mon); > > void bdrv_init(void); > diff --git a/monitor.c b/monitor.c > index 1528f08..e4fed10 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2047,7 +2047,8 @@ static const mon_cmd_t info_cmds[] = { > .args_type = "", > .params = "", > .help = "show the block devices", > - .mhandler.info = bdrv_info, > + .user_print = bdrv_info_print, > + .mhandler.info_new = bdrv_info, > }, > { > .name = "blockstats",
On Fri, 20 Nov 2009 15:06:26 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> 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 <lcapitulino@redhat.com> > > --- > > 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 <sys/types.h> > > @@ -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.
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Fri, 20 Nov 2009 15:06:26 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> Luiz Capitulino <lcapitulino@redhat.com> writes: [...] >> > diff --git a/block.c b/block.c >> > index 6fdabff..fc4e2f2 100644 >> > --- a/block.c >> > +++ b/block.c [...] >> > - 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. In that case, assert() catches a programming error. That's fine.
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 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 <sys/types.h> @@ -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")); + } + + 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 + * - "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 } + * { "device": "floppy0", "type": "floppy", "removable": 1, + * "locked": 0 } ] + */ +void bdrv_info(Monitor *mon, QObject **ret_data) +{ + QList *bs_list; BlockDriverState *bs; + bs_list = qlist_new(); + for (bs = bdrv_first; bs != NULL; bs = bs->next) { - monitor_printf(mon, "%s:", bs->device_name); - monitor_printf(mon, " type="); + QObject *bs_obj; + const char *type = "unknown"; + switch(bs->type) { case BDRV_TYPE_HD: - monitor_printf(mon, "hd"); + type = "hd"; break; case BDRV_TYPE_CDROM: - monitor_printf(mon, "cdrom"); + type = "cdrom"; break; case BDRV_TYPE_FLOPPY: - monitor_printf(mon, "floppy"); + type = "floppy"; break; } - 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); + if (bs->drv) { - monitor_printf(mon, " file="); - monitor_print_filename(mon, bs->filename); + QObject *obj; + QDict *bs_dict = qobject_to_qdict(bs_obj); + + obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, " + "'encrypted': %i }", + bs->filename, bs->read_only, + bs->drv->format_name, + bdrv_is_encrypted(bs)); + assert(obj != NULL); if (bs->backing_file[0] != '\0') { - monitor_printf(mon, " backing_file="); - monitor_print_filename(mon, bs->backing_file); + QDict *qdict = qobject_to_qdict(obj); + qdict_put(qdict, "backing_file", + qstring_from_str(bs->backing_file)); } - monitor_printf(mon, " ro=%d", bs->read_only); - monitor_printf(mon, " drv=%s", bs->drv->format_name); - monitor_printf(mon, " encrypted=%d", bdrv_is_encrypted(bs)); - } else { - monitor_printf(mon, " [not inserted]"); + + qdict_put_obj(bs_dict, "inserted", obj); } - monitor_printf(mon, "\n"); + qlist_append_obj(bs_list, bs_obj); } + + *ret_data = QOBJECT(bs_list); } /* The "info blockstats" command. */ diff --git a/block.h b/block.h index 2d4f066..3ca7112 100644 --- a/block.h +++ b/block.h @@ -4,6 +4,7 @@ #include "qemu-aio.h" #include "qemu-common.h" #include "qemu-option.h" +#include "qobject.h" /* block.c */ typedef struct BlockDriver BlockDriver; @@ -41,7 +42,8 @@ typedef struct QEMUSnapshotInfo { #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB) -void bdrv_info(Monitor *mon); +void bdrv_info_print(Monitor *mon, const QObject *data); +void bdrv_info(Monitor *mon, QObject **ret_data); void bdrv_info_stats(Monitor *mon); void bdrv_init(void); diff --git a/monitor.c b/monitor.c index 1528f08..e4fed10 100644 --- a/monitor.c +++ b/monitor.c @@ -2047,7 +2047,8 @@ static const mon_cmd_t info_cmds[] = { .args_type = "", .params = "", .help = "show the block devices", - .mhandler.info = bdrv_info, + .user_print = bdrv_info_print, + .mhandler.info_new = bdrv_info, }, { .name = "blockstats",
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 <lcapitulino@redhat.com> --- Makefile | 2 +- block.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++---------- block.h | 4 +- monitor.c | 3 +- 4 files changed, 109 insertions(+), 23 deletions(-)