Re: [Qemu-devel] [PATCH 0/7] discard blockstats

[Paolo Bonzini]

On 20/11/2017 17:50, Anton Nefedov wrote:
> qmp query-blockstats provides stats info for write/read/flush ops.
> 
> Patches 1-5 implement the similar for discard (unmap) command for scsi
> and ide disks.
> Discard stat "unmap_ops / unmap_bytes" is supposed to account the ops that
> have completed without an error.
> 
> However, discard operation is advisory. Specifically,
>  - common block layer ignores ENOTSUP error code.
>    That might be returned if the block driver does not support discard,
>    or discard has been configured to be ignored.
>  - format drivers such as qcow2 may ignore discard if they were configured
>    to ignore that, or if the corresponding area is already marked unused
>    (unallocated / zero clusters).
> 
> And what is actually useful is the number of bytes actually discarded
> down on the host filesystem.
> To achieve that, driver-specific statistics has been added to blockstats
> (patch 7).
> With patch 6, file-posix driver accounts discard operations on its level too.
> 
> query-blockstat result:
> 
> (note the difference between blockdevice unmap and file discard stats. qcow2
> sends a few ops down to the file as the clusters are actually unallocated
> on qcow2 level)
> 
> {"return": [
> {"device": "drive-scsi0-0-0-0"
>   "parent": {
>     "stats": {
>>      "unmap_operations": 0
>>      "unmap_merged": 0
>       "flush_total_time_ns": 0
>       "wr_highest_offset": 8111718400
>       [..]
>       "invalid_wr_operations": 0
>       "invalid_rd_operations": 0}
>     "node-name": "#block047"
>>    "driver_stats": {
>>      "type": "file"
>>      "data": {
>>        "discard_bytes_ok": 1572864
>>        "discard_nb_failed": 0
>>        "discard_nb_ok": 5}}}
>   "stats": {
>>   "unmap_operations": 472
>>   "unmap_merged": 0
>     "flush_total_time_ns": 44530540
>     "wr_highest_offset": 7106662400
>     "wr_total_time_ns": 45518856
>     "failed_wr_operations": 0
>     "failed_rd_operations": 0
>     "wr_merged": 0
>     "wr_bytes": 889856
>     "timed_stats": []
>>    "failed_unmap_operations": 0
>     "failed_flush_operations": 0
>     "account_invalid": true
>     "rd_total_time_ns": 3306264098
>>    "invalid_unmap_operations": 0
>     "flush_operations": 18
>     "wr_operations": 120
>>    "unmap_bytes": 12312014848
>     "rd_merged": 0
>     "rd_bytes": 137103360
>>    "unmap_total_time_ns": 22664692
>     "invalid_flush_operations": 0
>     "account_failed": true
>     "idle_time_ns": 437316567
>     "rd_operations": 5636
>     "invalid_wr_operations": 0
>     "invalid_rd_operations": 0}
>   "node-name": "#block128"}
> 
>   {"device": "drive-ide0-0-0"
>   [..]
> 
> Anton Nefedov (7):
>   qapi: add unmap to BlockDeviceStats
>   ide: account UNMAP (TRIM) operations
>   scsi: store unmap offset and nb_sectors in request struct
>   scsi: move unmap error checking to the complete callback
>   scsi: account unmap operations
>   file-posix: account discard operations
>   qapi: query-blockstat: add driver specific file-posix stats
> 
>  qapi/block-core.json       | 57 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  include/block/accounting.h |  1 +
>  include/block/block.h      |  1 +
>  include/block/block_int.h  |  1 +
>  block.c                    |  9 ++++++++
>  block/file-posix.c         | 42 ++++++++++++++++++++++++++++++++--
>  block/qapi.c               | 11 +++++++++
>  hw/ide/core.c              | 12 ++++++++++
>  hw/scsi/scsi-disk.c        | 29 ++++++++++++++---------
>  9 files changed, 150 insertions(+), 13 deletions(-)
> 

Hey, looks like this series has fallen through the cracks.  Anton, are
you going to send an updated version?

Thanks,

Paolo