Re: [Qemu-devel] [PATCH 08/10] monitor: Convert bdrv_info() to QObject

[Anthony Liguori]

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 <address@hidden>
> ---
>  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