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

[Anton Nefedov]

ping-2

On 18/9/2018 11:12 AM, Anton Nefedov wrote:
> ping
> 
> do you think we might proceed with this? or is there any general doubt
> about the idea?
> 
> thanks,
> 
> On 21/8/2018 12:46 PM, Anton Nefedov wrote:
>> new in v4:
>>      - patch 7: discard and write-zeroes code paths had been separated in
>>        34fa110e: file-posix: Fix write_zeroes with unmap on block 
>> devices.
>>        This patch now only accounts discards that come explicitly
>>        through .bdrv_co_pdiscard handler.
>>      - qapi 'Since' clauses changed 3.0 -> 3.1
>>
>> v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg03688.html
>>
>> ----
>>
>> qmp query-blockstats provides stats info for write/read/flush ops.
>>
>> Patches 1-6 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 8).
>> With patch 7, 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 fewer ops down to the file as the clusters are actually unallocated
>> on qcow2 level)
>>
>> {
>>    "return": [
>>      {
>>        "device": "drive-scsi0-0-0-0",
>>        "parent": {
>>>        "discard-bytes-ok": 262144,
>>>        "discard-nb-ok": 4,
>>          "stats": {
>>>          "unmap_operations": 0,
>>>          "unmap_merged": 0,
>>            "flush_total_time_ns": 0,
>>            "wr_highest_offset": 8111718400,
>>            "wr_total_time_ns": 0,
>>            "failed_wr_operations": 0,
>>            "failed_rd_operations": 0,
>>            "wr_merged": 0,
>>            "wr_bytes": 0,
>>            "timed_stats": [
>>            ],
>>>          "failed_unmap_operations": 0,
>>            "failed_flush_operations": 0,
>>            "account_invalid": false,
>>            "rd_total_time_ns": 0,
>>>          "invalid_unmap_operations": 0,
>>            "flush_operations": 0,
>>            "wr_operations": 0,
>>>          "unmap_bytes": 0,
>>            "rd_merged": 0,
>>            "rd_bytes": 0,
>>>          "unmap_total_time_ns": 0,
>>            "invalid_flush_operations": 0,
>>            "account_failed": false,
>>            "rd_operations": 0,
>>            "invalid_wr_operations": 0,
>>            "invalid_rd_operations": 0
>>          },
>>          "node-name": "#block012",
>>>        "driver": "file",
>>>        "discard-nb-failed": 0
>>        },
>>        "stats": {
>>>        "unmap_operations": 860,
>>>        "unmap_merged": 0,
>>          "flush_total_time_ns": 21506733,
>>          "wr_highest_offset": 13411741696,
>>          "wr_total_time_ns": 2212749334,
>>          "failed_wr_operations": 0,
>>          "failed_rd_operations": 0,
>>          "wr_merged": 0,
>>          "wr_bytes": 3426304,
>>          "timed_stats": [
>>          ],
>>>        "failed_unmap_operations": 0,
>>          "failed_flush_operations": 0,
>>          "account_invalid": true,
>>          "rd_total_time_ns": 3617478206,
>>>        "invalid_unmap_operations": 0,
>>          "flush_operations": 24,
>>          "wr_operations": 309,
>>>        "unmap_bytes": 11949633536,
>>          "rd_merged": 0,
>>          "rd_bytes": 141967360,
>>>        "unmap_total_time_ns": 14871816,
>>          [..]
>>
>> Anton Nefedov (8):
>>    qapi: group BlockDeviceStats fields
>>    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       | 82 
>> +++++++++++++++++++++++++++++++++++++++-------
>>   include/block/accounting.h |  1 +
>>   include/block/block.h      |  1 +
>>   include/block/block_int.h  |  1 +
>>   block.c                    |  9 +++++
>>   block/file-posix.c         | 45 +++++++++++++++++++++++--
>>   block/qapi.c               | 11 +++++++
>>   hw/ide/core.c              | 12 +++++++
>>   hw/scsi/scsi-disk.c        | 29 +++++++++-------
>>   tests/qemu-iotests/227.out | 18 ++++++++++
>>   10 files changed, 184 insertions(+), 25 deletions(-)
>>