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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 28 Apr 2010 17:56:20 +0200
Kevin Wolf <kwolf@redhat.com> 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>
> ---
>  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);
> +    }

 Doesn't build, I guess you meant 'backing_hd'.

> +
> +    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.

 Looks like you are not pushing 'protocol', we already have 'format'
though (not sure if they overlap).

 Also, 'device' is "" for 'parent'. You should consider not pushing the
key in this case (and noting that it's optional in the doc above).

> + *
>   * 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;