[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat

[Anthony Liguori <anthony@codemonkey.ws>]

On 04/28/2010 10:56 AM, Kevin Wolf wrote:
> This adds the wr_highest_sector blockstat which implements what is generally
> known as the high watermark. It is the highest offset of a sector written to
> the respective BlockDriverState since it has been opened.
>
> The query-blockstat QMP command is extended to add this value to the result,
> and also to add the statistics of the underlying protocol in a new "parent"
> field. Note that to get the "high watermark" of a qcow2 image, you need to look
> into the wr_highest_sector field of the parent (which can be a file, a
> host_device, ...). The wr_highest_sector of the qcow2 BlockDriverState itself
> is the highest offset on the _virtual_ disk that the guest has written to.
>
> Signed-off-by: Kevin Wolf<kwolf@redhat.com>
>    

I see, you did print out stats for each layer.

I don't think we should take 2/2.  I don't mind QMP having more features 
than the user monitor.

Regards,

Anthony Liguori

> ---
>   block.c     |   76 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>   block_int.h |    1 +
>   2 files changed, 62 insertions(+), 15 deletions(-)
>
> diff --git a/block.c b/block.c
> index 91fecab..b75cef2 100644
> --- a/block.c
> +++ b/block.c
> @@ -880,6 +880,10 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,
>           set_dirty_bitmap(bs, sector_num, nb_sectors, 1);
>       }
>
> +    if (bs->wr_highest_sector<  sector_num + nb_sectors - 1) {
> +        bs->wr_highest_sector = sector_num + nb_sectors - 1;
> +    }
> +
>       return drv->bdrv_write(bs, sector_num, buf, nb_sectors);
>   }
>
> @@ -1523,6 +1527,10 @@ static void bdrv_stats_iter(QObject *data, void *opaque)
>                           qdict_get_int(qdict, "wr_bytes"),
>                           qdict_get_int(qdict, "rd_operations"),
>                           qdict_get_int(qdict, "wr_operations"));
> +    if (qdict_haskey(qdict, "parent")) {
> +        QObject *parent = qdict_get(qdict, "parent");
> +        bdrv_stats_iter(parent, mon);
> +    }
>   }
>
>   void bdrv_stats_print(Monitor *mon, const QObject *data)
> @@ -1530,6 +1538,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)
>       qlist_iter(qobject_to_qlist(data), bdrv_stats_iter, mon);
>   }
>
> +static QObject* bdrv_info_stats_bs(BlockDriverState *bs)
> +{
> +    QObject *res;
> +    QObject *parent = NULL;
> +
> +    if (bs->file) {
> +        parent = bdrv_info_stats_bs(bs->file);
> +    }
> +
> +    res = qobject_from_jsonf("{ 'device': %s, 'stats': {"
> +                             "'rd_bytes': %" PRId64 ","
> +                             "'wr_bytes': %" PRId64 ","
> +                             "'rd_operations': %" PRId64 ","
> +                             "'wr_operations': %" PRId64 ","
> +                             "'wr_highest_offset': %" PRId64
> +                             "} }",
> +                             bs->device_name,
> +                             bs->rd_bytes, bs->wr_bytes,
> +                             bs->rd_ops, bs->wr_ops,
> +                             bs->wr_highest_sector * 512);
> +    if (parent) {
> +        QDict *dict = qobject_to_qdict(res);
> +        qdict_put_obj(dict, "parent", parent);
> +    }
> +
> +    return res;
> +}
> +
>   /**
>    * bdrv_info_stats(): show block device statistics
>    *
> @@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)
>    *     - "wr_bytes": bytes written
>    *     - "rd_operations": read operations
>    *     - "wr_operations": write operations
> - *
> + *     - "wr_highest_offset": Highest offset of a sector written since the
> + *       BlockDriverState has been opened
> + *     - "parent": Contains recursively the statistics of the underlying
> + *       protocol (e.g. the host file for a qcow2 image). If there is no
> + *       underlying protocol, this field is omitted.
> + *
>    * Example:
>    *
>    * [ { "device": "ide0-hd0",
>    *               "stats": { "rd_bytes": 512,
>    *                          "wr_bytes": 0,
>    *                          "rd_operations": 1,
> - *                          "wr_operations": 0 } },
> + *                          "wr_operations": 0,
> + *                          "wr_highest_offset": 0,
> + *                          "parent": {
> + *                              "stats": { "rd_bytes": 1024,
> + *                                         "wr_bytes": 0,
> + *                                         "rd_operations": 2,
> + *                                         "wr_operations": 0,
> + *                                         "wr_highest_offset": 0,
> + *                              }
> + *                          } } },
>    *   { "device": "ide1-cd0",
>    *               "stats": { "rd_bytes": 0,
>    *                          "wr_bytes": 0,
>    *                          "rd_operations": 0,
> - *                          "wr_operations": 0 } } ]
> + *                          "wr_operations": 0,
> + *                          "wr_highest_offset": 0 } },
>    */
>   void bdrv_info_stats(Monitor *mon, QObject **ret_data)
>   {
> @@ -1567,15 +1618,7 @@ void bdrv_info_stats(Monitor *mon, QObject **ret_data)
>       devices = qlist_new();
>
>       QTAILQ_FOREACH(bs,&bdrv_states, list) {
> -        obj = qobject_from_jsonf("{ 'device': %s, 'stats': {"
> -                                 "'rd_bytes': %" PRId64 ","
> -                                 "'wr_bytes': %" PRId64 ","
> -                                 "'rd_operations': %" PRId64 ","
> -                                 "'wr_operations': %" PRId64
> -                                 "} }",
> -                                 bs->device_name,
> -                                 bs->rd_bytes, bs->wr_bytes,
> -                                 bs->rd_ops, bs->wr_ops);
> +        obj = bdrv_info_stats_bs(bs);
>           qlist_append_obj(devices, obj);
>       }
>
> @@ -1834,9 +1877,12 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, int64_t sector_num,
>                                  cb, opaque);
>
>       if (ret) {
> -	/* Update stats even though technically transfer has not happened. */
> -	bs->wr_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
> -	bs->wr_ops ++;
> +        /* Update stats even though technically transfer has not happened. */
> +        bs->wr_bytes += (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
> +        bs->wr_ops ++;
> +        if (bs->wr_highest_sector<  sector_num + nb_sectors - 1) {
> +            bs->wr_highest_sector = sector_num + nb_sectors - 1;
> +        }
>       }
>
>       return ret;
> diff --git a/block_int.h b/block_int.h
> index a3afe63..1a7240c 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -167,6 +167,7 @@ struct BlockDriverState {
>       uint64_t wr_bytes;
>       uint64_t rd_ops;
>       uint64_t wr_ops;
> +    uint64_t wr_highest_sector;
>
>       /* Whether the disk can expand beyond total_sectors */
>       int growable;
>