| Message ID | 1255037747-3340-9-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | Under Review |
| 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. > > 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 > - "backing_file": backing file name if one is used Isn't this in "interted" rather than here? If yes, the comment in the code needs fixing, too. > - "inserted": only present if the device is inserted, it is a QDict I'd call this "media". > containing the following: > - "file": device file name > - "ro": 1 if read-only 0 otherwise > - "drv": driver format name > - "encrypted": 1 if encrypted 0 otherwise > > The current implemention uses integers as booleans, to make > things simple those integers are stored in the QDict. Ideally, > we would have a QBool type and this is probably going to be > a requirement for the protocol. > > But the integers will do the job for now. > > This commit should not change user output, the following is an > example of the returned QList: > > [ { "device": "ide0-hd0", "type": "hd", "removable": 0, > "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 } > { "device": "floppy0", "type": "floppy", "removable": 1, > "locked": 0 } ] [...]
On Sat, 10 Oct 2009 14:18:03 +0200 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. > > > > 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 > > - "backing_file": backing file name if one is used > > Isn't this in "interted" rather than here? > > If yes, the comment in the code needs fixing, too. Yes, although I don't think I should resend just because of that.
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Sat, 10 Oct 2009 14:18:03 +0200 > 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. >> > >> > 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 >> > - "backing_file": backing file name if one is used >> >> Isn't this in "interted" rather than here? >> >> If yes, the comment in the code needs fixing, too. > > Yes, although I don't think I should resend just because of that. Any other way to reliably avoid commit of a misleading comment with a misleading commit message? If it goes in that way, we need to follow up with a commit fixing the comment.
Luiz Capitulino wrote: > 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 > - "backing_file": backing file name if one is used > - "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 > - "encrypted": 1 if encrypted 0 otherwise > > The current implemention uses integers as booleans, to make > things simple those integers are stored in the QDict. Ideally, > we would have a QBool type and this is probably going to be > a requirement for the protocol. > > But the integers will do the job for now. > > This commit should not change user output, the following is an > example of the returned QList: > > [ { "device": "ide0-hd0", "type": "hd", "removable": 0, > "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 } > { "device": "floppy0", "type": "floppy", "removable": 1, > "locked": 0 } ] > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > Makefile | 1 + > block.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++--------- > block.h | 4 +- > monitor.c | 3 +- > 4 files changed, 118 insertions(+), 22 deletions(-) > > diff --git a/Makefile b/Makefile > index 182f176..d29d871 100644 > --- a/Makefile > +++ b/Makefile > @@ -62,6 +62,7 @@ recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES) > # 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 += qint.o qstring.o qlist.o qdict.o qmisc.o > block-obj-y += nbd.o block.o aio.o aes.o osdep.o > 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 33f3d65..31a58e4 100644 > --- a/block.c > +++ b/block.c > @@ -26,6 +26,10 @@ > #include "monitor.h" > #include "block_int.h" > #include "module.h" > +#include "qlist.h" > +#include "qdict.h" > +#include "qstring.h" > +#include "qmisc.h" > > #ifdef CONFIG_BSD > #include <sys/types.h> > @@ -1075,43 +1079,131 @@ 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; > + > + assert(qobject_type(obj) == QTYPE_QDICT); > + 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"), > + (int) qdict_get_int(bs_dict, "removable")); > This is a very common format. So much so, it probably makes sense to have: monitor_print_dict(mon, "device", bs_dict); > + > + if (qdict_get_int(bs_dict, "removable")) { > + monitor_printf(mon, " locked=%d",(int)qdict_get_int(bs_dict, "locked")); > + } > Which suggests that the dict entry should be locked, not removable. > + if (qdict_haskey(bs_dict, "inserted")) { > I guess you could flatten nested dicts. > + QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); > + monitor_printf(mon, " file=%s", qdict_get_str(qdict, "file")); > + if (qdict_haskey(qdict, "backing_file")) { > + monitor_printf(mon, " backing_file=%s", > + qdict_get_str(qdict, "backing_file")); > + } > + monitor_printf(mon, " ro=%d drv=%s encrypted=%d", > + (int) qdict_get_int(qdict, "ro"), > + qdict_get_str(qdict, "drv"), > + (int) qdict_get_int(qdict, "encrypted")); > + } else { > + monitor_printf(mon, " [not inserted]"); > This bit you would probably have to handle manually. Alternatively, you could build a compat dict from the new dict format and then print that with the monitor_print_dict function. Regards, Anthony Liguori
diff --git a/Makefile b/Makefile index 182f176..d29d871 100644 --- a/Makefile +++ b/Makefile @@ -62,6 +62,7 @@ recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES) # 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 += qint.o qstring.o qlist.o qdict.o qmisc.o block-obj-y += nbd.o block.o aio.o aes.o osdep.o 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 33f3d65..31a58e4 100644 --- a/block.c +++ b/block.c @@ -26,6 +26,10 @@ #include "monitor.h" #include "block_int.h" #include "module.h" +#include "qlist.h" +#include "qdict.h" +#include "qstring.h" +#include "qmisc.h" #ifdef CONFIG_BSD #include <sys/types.h> @@ -1075,43 +1079,131 @@ 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; + + assert(qobject_type(obj) == QTYPE_QDICT); + 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"), + (int) qdict_get_int(bs_dict, "removable")); + + if (qdict_get_int(bs_dict, "removable")) { + monitor_printf(mon, " locked=%d",(int)qdict_get_int(bs_dict, "locked")); + } + + if (qdict_haskey(bs_dict, "inserted")) { + QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); + monitor_printf(mon, " file=%s", qdict_get_str(qdict, "file")); + if (qdict_haskey(qdict, "backing_file")) { + monitor_printf(mon, " backing_file=%s", + qdict_get_str(qdict, "backing_file")); + } + monitor_printf(mon, " ro=%d drv=%s encrypted=%d", + (int) qdict_get_int(qdict, "ro"), + qdict_get_str(qdict, "drv"), + (int) qdict_get_int(qdict, "encrypted")); + } else { + monitor_printf(mon, " [not inserted]"); + } + + monitor_printf(mon, "\n"); +} + +void bdrv_user_print(Monitor *mon, const QObject *data) +{ + QList *bs_list; + + assert(qobject_type(data) == QTYPE_QLIST); + bs_list = qobject_to_qlist(data); + + qlist_iter(bs_list, bdrv_print_dict, mon); +} + +/** + * bdrv_info(): Show block devices + * + * 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 + * - "backing_file": backing file name if one is used + * - "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 + * - "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_fmt("{ s: s, s: s, s: i, s: i }", + "device", bs->device_name, + "type", type, + "removable", bs->removable, + "locked", 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_fmt("{ s: s, s: i, s: s, s: i }", + "file", bs->filename, + "ro", bs->read_only, + "drv", bs->drv->format_name, + "encrypted", 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 a966afb..99dc360 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_user_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 7e5c07d..39d3201 100644 --- a/monitor.c +++ b/monitor.c @@ -1979,7 +1979,8 @@ static const mon_cmd_t info_cmds[] = { .args_type = "", .params = "", .help = "show the block devices", - .mhandler.info = bdrv_info, + .user_print = bdrv_user_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. 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 - "backing_file": backing file name if one is used - "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 - "encrypted": 1 if encrypted 0 otherwise The current implemention uses integers as booleans, to make things simple those integers are stored in the QDict. Ideally, we would have a QBool type and this is probably going to be a requirement for the protocol. But the integers will do the job for now. This commit should not change user output, the following is an example of the returned QList: [ { "device": "ide0-hd0", "type": "hd", "removable": 0, "file": "/tmp/foobar", "ro": 0, "drv": "qcow2", "encrypted": 0 } { "device": "floppy0", "type": "floppy", "removable": 1, "locked": 0 } ] Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- Makefile | 1 + block.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++--------- block.h | 4 +- monitor.c | 3 +- 4 files changed, 118 insertions(+), 22 deletions(-)