diff mbox series

[3/3] qapi: add block size histogram interface

Message ID 1561020872-6214-4-git-send-email-pizhenwei@bytedance.com (mailing list archive)
State New, archived
Headers show
Series Add block size histogram qapi interface | expand

Commit Message

zhenwei pi June 20, 2019, 8:54 a.m. UTC
Set/Clear block size histograms through new command
x-block-size-histogram-set and show new statistics in
query-blockstats results.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 block/qapi.c         |  24 ++++++++++++
 blockdev.c           |  56 +++++++++++++++++++++++++++
 qapi/block-core.json | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 184 insertions(+), 1 deletion(-)

Comments

Eric Blake June 20, 2019, 2:03 p.m. UTC | #1
On 6/20/19 3:54 AM, zhenwei pi wrote:
> Set/Clear block size histograms through new command
> x-block-size-histogram-set and show new statistics in
> query-blockstats results.
> 

I'm guessing this is modeled after the existing
block-latency-histogram-set command?

> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  block/qapi.c         |  24 ++++++++++++
>  blockdev.c           |  56 +++++++++++++++++++++++++++
>  qapi/block-core.json | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 184 insertions(+), 1 deletion(-)

> +++ b/qapi/block-core.json
> @@ -633,6 +633,100 @@
>             '*boundaries-flush': ['uint64'] } }
>  
>  ##
> +# @BlockSizeHistogramInfo:
> +#
> +# Block size histogram.
> +#
> +# @boundaries: list of interval boundary values in nanoseconds, all greater
> +#              than zero and in ascending order.
> +#              For example, the list [8193, 32769, 131073] produces the
> +#              following histogram intervals:
> +#              [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf).
> +#
> +# @bins: list of io request counts corresponding to histogram intervals.
> +#        len(@bins) = len(@boundaries) + 1
> +#        For the example above, @bins may be something like [6, 3, 7, 9],
> +#        and corresponding histogram looks like:
> +#
> +# Since: 4.0

You've missed 4.0; the next release is 4.1.

> +##
> +{ 'struct': 'BlockSizeHistogramInfo',
> +  'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }

This is identical to struct BlockLatencyHistogramInfo; can we instead
rename the type (which does not affect API) and share it between both
implementations, instead of duplicating it?

> +
> +##
> +# @x-block-size-histogram-set:

Does this need to be experimental from the get-go? Or can it be stable
by dropping 'x-' since it matches the fact that
block-latency-histogram-set is stable?

> +#
> +# Manage read, write and flush size histograms for the device.
> +#
> +# If only @id parameter is specified, remove all present size histograms
> +# for the device. Otherwise, add/reset some of (or all) size histograms.
> +#
> +# @id: The name or QOM path of the guest device.
> +#
> +# @boundaries: list of interval boundary values (see description in
> +#              BlockSizeHistogramInfo definition). If specified, all
> +#              size histograms are removed, and empty ones created for all
> +#              io types with intervals corresponding to @boundaries (except for
> +#              io types, for which specific boundaries are set through the
> +#              following parameters).
> +#
> +# @boundaries-read: list of interval boundary values for read size
> +#                   histogram. If specified, old read size histogram is
> +#                   removed, and empty one created with intervals
> +#                   corresponding to @boundaries-read. The parameter has higher
> +#                   priority then @boundaries.
> +#
> +# @boundaries-write: list of interval boundary values for write size
> +#                    histogram.
> +#
> +# @boundaries-flush: list of interval boundary values for flush size
> +#                    histogram.
> +#
> +# Returns: error if device is not found or any boundary arrays are invalid.
> +#
> +# Since: 4.0

4.1

> +#
> +# Example: set new histograms for all io types with intervals
> +# [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf):
> +#
> +# -> { "execute": "x-block-size-histogram-set",
> +#      "arguments": { "id": "drive0",
> +#                     "boundaries": [8193, 32769, 131073] } }
> +# <- { "return": {} }
> +#
> +# Example: set new histogram only for write, other histograms will remain
> +# not changed (or not created):
> +#
> +# -> { "execute": "x-block-size-histogram-set",
> +#      "arguments": { "id": "drive0",
> +#                     "boundaries-write": [8193, 32769, 131073] } }
> +# <- { "return": {} }
> +#
> +# Example: set new histograms with the following intervals:
> +#   read, flush: [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf)
> +#   write: [0, 4097), [4097, 8193), [8193, 32769), [32769, +inf)
> +#
> +# -> { "execute": "x-block-size-histogram-set",
> +#      "arguments": { "id": "drive0",
> +#                     "boundaries": [8193, 32769, 131073],
> +#                     "boundaries-write": [4097, 8193, 32769] } }
> +# <- { "return": {} }
> +#
> +# Example: remove all size histograms:
> +#
> +# -> { "execute": "x-block-size-histogram-set",
> +#      "arguments": { "id": "drive0" } }
> +# <- { "return": {} }
> +##
> +{ 'command': 'x-block-size-histogram-set',
> +  'data': {'id': 'str',
> +           '*boundaries': ['uint64'],
> +           '*boundaries-read': ['uint64'],
> +           '*boundaries-write': ['uint64'],
> +           '*boundaries-flush': ['uint64'] } }

Again, this copies heavily from block-latency-histogram-set.  But
changing the command name is not API compatible.  Should we have a
single new command 'block-histogram-set' which takes an enum choosing
between 'latency' and 'size', and start the deprecation clock on
'block-latency-histogram-set'?
 (and defaulting to 'latency' for back-compat

> +
> +
> +##
>  # @BlockInfo:
>  #
>  # Block device information.  This structure describes a virtual device and
> @@ -918,6 +1012,12 @@
>  #
>  # @flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
>  #
> +# @x_rd_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
> +#
> +# @x_wr_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
> +#
> +# @x_flush_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)

since 4.1 on all of these additions.

> +#
>  # Since: 0.14.0
>  ##
>  { 'struct': 'BlockDeviceStats',
> @@ -933,7 +1033,10 @@
>             'timed_stats': ['BlockDeviceTimedStats'],
>             '*rd_latency_histogram': 'BlockLatencyHistogramInfo',
>             '*wr_latency_histogram': 'BlockLatencyHistogramInfo',
> -           '*flush_latency_histogram': 'BlockLatencyHistogramInfo' } }
> +           '*flush_latency_histogram': 'BlockLatencyHistogramInfo',
> +           '*x_rd_size_histogram': 'BlockSizeHistogramInfo',
> +           '*x_wr_size_histogram': 'BlockSizeHistogramInfo',
> +           '*x_flush_size_histogram': 'BlockSizeHistogramInfo' } }
>  
>  ##
>  # @BlockStats:
>
zhenwei pi June 21, 2019, 1:52 a.m. UTC | #2
On 6/20/19 10:03 PM, Eric Blake wrote:

> On 6/20/19 3:54 AM, zhenwei pi wrote:
>> Set/Clear block size histograms through new command
>> x-block-size-histogram-set and show new statistics in
>> query-blockstats results.
>>
> I'm guessing this is modeled after the existing
> block-latency-histogram-set command?

zhenwei: Yes, it is.

>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>> ---
>>   block/qapi.c         |  24 ++++++++++++
>>   blockdev.c           |  56 +++++++++++++++++++++++++++
>>   qapi/block-core.json | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   3 files changed, 184 insertions(+), 1 deletion(-)
>> +++ b/qapi/block-core.json
>> @@ -633,6 +633,100 @@
>>              '*boundaries-flush': ['uint64'] } }
>>   
>>   ##
>> +# @BlockSizeHistogramInfo:
>> +#
>> +# Block size histogram.
>> +#
>> +# @boundaries: list of interval boundary values in nanoseconds, all greater
>> +#              than zero and in ascending order.
>> +#              For example, the list [8193, 32769, 131073] produces the
>> +#              following histogram intervals:
>> +#              [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf).
>> +#
>> +# @bins: list of io request counts corresponding to histogram intervals.
>> +#        len(@bins) = len(@boundaries) + 1
>> +#        For the example above, @bins may be something like [6, 3, 7, 9],
>> +#        and corresponding histogram looks like:
>> +#
>> +# Since: 4.0
> You've missed 4.0; the next release is 4.1.

zhenwei: OK, I will fix all the version info.

>> +##
>> +{ 'struct': 'BlockSizeHistogramInfo',
>> +  'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }
> This is identical to struct BlockLatencyHistogramInfo; can we instead
> rename the type (which does not affect API) and share it between both
> implementations, instead of duplicating it?
>
zhenwei: Good idea. But I am confused about the compatibility of the
structure BlockLatencyHistogramInfo. If I rename BlockLatencyHistogramInfo
to BlockHistogramInfo, it will be common.

>> +
>> +##
>> +# @x-block-size-histogram-set:
> Does this need to be experimental from the get-go? Or can it be stable
> by dropping 'x-' since it matches the fact that
> block-latency-histogram-set is stable?

zhenwei: OK, I will drop 'x-' prefix.

>> +#
>> +# Manage read, write and flush size histograms for the device.
>> +#
>> +# If only @id parameter is specified, remove all present size histograms
>> +# for the device. Otherwise, add/reset some of (or all) size histograms.
>> +#
>> +# @id: The name or QOM path of the guest device.
>> +#
>> +# @boundaries: list of interval boundary values (see description in
>> +#              BlockSizeHistogramInfo definition). If specified, all
>> +#              size histograms are removed, and empty ones created for all
>> +#              io types with intervals corresponding to @boundaries (except for
>> +#              io types, for which specific boundaries are set through the
>> +#              following parameters).
>> +#
>> +# @boundaries-read: list of interval boundary values for read size
>> +#                   histogram. If specified, old read size histogram is
>> +#                   removed, and empty one created with intervals
>> +#                   corresponding to @boundaries-read. The parameter has higher
>> +#                   priority then @boundaries.
>> +#
>> +# @boundaries-write: list of interval boundary values for write size
>> +#                    histogram.
>> +#
>> +# @boundaries-flush: list of interval boundary values for flush size
>> +#                    histogram.
>> +#
>> +# Returns: error if device is not found or any boundary arrays are invalid.
>> +#
>> +# Since: 4.0
> 4.1
>
>> +#
>> +# Example: set new histograms for all io types with intervals
>> +# [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf):
>> +#
>> +# -> { "execute": "x-block-size-histogram-set",
>> +#      "arguments": { "id": "drive0",
>> +#                     "boundaries": [8193, 32769, 131073] } }
>> +# <- { "return": {} }
>> +#
>> +# Example: set new histogram only for write, other histograms will remain
>> +# not changed (or not created):
>> +#
>> +# -> { "execute": "x-block-size-histogram-set",
>> +#      "arguments": { "id": "drive0",
>> +#                     "boundaries-write": [8193, 32769, 131073] } }
>> +# <- { "return": {} }
>> +#
>> +# Example: set new histograms with the following intervals:
>> +#   read, flush: [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf)
>> +#   write: [0, 4097), [4097, 8193), [8193, 32769), [32769, +inf)
>> +#
>> +# -> { "execute": "x-block-size-histogram-set",
>> +#      "arguments": { "id": "drive0",
>> +#                     "boundaries": [8193, 32769, 131073],
>> +#                     "boundaries-write": [4097, 8193, 32769] } }
>> +# <- { "return": {} }
>> +#
>> +# Example: remove all size histograms:
>> +#
>> +# -> { "execute": "x-block-size-histogram-set",
>> +#      "arguments": { "id": "drive0" } }
>> +# <- { "return": {} }
>> +##
>> +{ 'command': 'x-block-size-histogram-set',
>> +  'data': {'id': 'str',
>> +           '*boundaries': ['uint64'],
>> +           '*boundaries-read': ['uint64'],
>> +           '*boundaries-write': ['uint64'],
>> +           '*boundaries-flush': ['uint64'] } }
> Again, this copies heavily from block-latency-histogram-set.  But
> changing the command name is not API compatible.  Should we have a
> single new command 'block-histogram-set' which takes an enum choosing
> between 'latency' and 'size', and start the deprecation clock on
> 'block-latency-histogram-set'?
>   (and defaulting to 'latency' for back-compat
>
zhenwei: this actually copies from block-latency-histogram-set, because the
two APIs have the similar logic but different structure
BlockLatencyHistogramInfo and BlockSizeHistogramInfo.
For further extension, a single new command 'block-histogram-set' with
enum operation is better.
So, should I remove 'block-latency-histogram-set' and add 'block-histogram-set'?

>> +
>> +
>> +##
>>   # @BlockInfo:
>>   #
>>   # Block device information.  This structure describes a virtual device and
>> @@ -918,6 +1012,12 @@
>>   #
>>   # @flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
>>   #
>> +# @x_rd_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
>> +#
>> +# @x_wr_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
>> +#
>> +# @x_flush_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
> since 4.1 on all of these additions.
>
>> +#
>>   # Since: 0.14.0
>>   ##
>>   { 'struct': 'BlockDeviceStats',
>> @@ -933,7 +1033,10 @@
>>              'timed_stats': ['BlockDeviceTimedStats'],
>>              '*rd_latency_histogram': 'BlockLatencyHistogramInfo',
>>              '*wr_latency_histogram': 'BlockLatencyHistogramInfo',
>> -           '*flush_latency_histogram': 'BlockLatencyHistogramInfo' } }
>> +           '*flush_latency_histogram': 'BlockLatencyHistogramInfo',
>> +           '*x_rd_size_histogram': 'BlockSizeHistogramInfo',
>> +           '*x_wr_size_histogram': 'BlockSizeHistogramInfo',
>> +           '*x_flush_size_histogram': 'BlockSizeHistogramInfo' } }
>>   
>>   ##
>>   # @BlockStats:
>>
Vladimir Sementsov-Ogievskiy June 21, 2019, 9:26 a.m. UTC | #3
21.06.2019 4:52, zhenwei pi wrote:
> On 6/20/19 10:03 PM, Eric Blake wrote:
> 
>> On 6/20/19 3:54 AM, zhenwei pi wrote:
>>> Set/Clear block size histograms through new command
>>> x-block-size-histogram-set and show new statistics in
>>> query-blockstats results.
>>>
>> I'm guessing this is modeled after the existing
>> block-latency-histogram-set command?
> 
> zhenwei: Yes, it is.
> 
>>> Signed-off-by: zhenwei pi<pizhenwei@bytedance.com>
>>> ---
>>>   block/qapi.c         |  24 ++++++++++++
>>>   blockdev.c           |  56 +++++++++++++++++++++++++++
>>>   qapi/block-core.json | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>   3 files changed, 184 insertions(+), 1 deletion(-)
>>> +++ b/qapi/block-core.json
>>> @@ -633,6 +633,100 @@
>>>              '*boundaries-flush': ['uint64'] } }
>>>   
>>>   ##
>>> +# @BlockSizeHistogramInfo:
>>> +#
>>> +# Block size histogram.
>>> +#
>>> +# @boundaries: list of interval boundary values in nanoseconds, all greater
>>> +#              than zero and in ascending order.
>>> +#              For example, the list [8193, 32769, 131073] produces the
>>> +#              following histogram intervals:
>>> +#              [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf).
>>> +#
>>> +# @bins: list of io request counts corresponding to histogram intervals.
>>> +#        len(@bins) = len(@boundaries) + 1
>>> +#        For the example above, @bins may be something like [6, 3, 7, 9],
>>> +#        and corresponding histogram looks like:
>>> +#
>>> +# Since: 4.0
>> You've missed 4.0; the next release is 4.1.
> 
> zhenwei: OK, I will fix all the version info.
> 
>>> +##
>>> +{ 'struct': 'BlockSizeHistogramInfo',
>>> +  'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }
>> This is identical to struct BlockLatencyHistogramInfo; can we instead
>> rename the type (which does not affect API) and share it between both
>> implementations, instead of duplicating it?
>>
> zhenwei: Good idea. But I am confused about the compatibility of the
> structure BlockLatencyHistogramInfo. If I rename BlockLatencyHistogramInfo
> to BlockHistogramInfo, it will be common.
> 
>>> +
>>> +##
>>> +# @x-block-size-histogram-set:
>> Does this need to be experimental from the get-go? Or can it be stable
>> by dropping 'x-' since it matches the fact that
>> block-latency-histogram-set is stable?
> 
> zhenwei: OK, I will drop 'x-' prefix.
> 
>>> +#
>>> +# Manage read, write and flush size histograms for the device.
>>> +#
>>> +# If only @id parameter is specified, remove all present size histograms
>>> +# for the device. Otherwise, add/reset some of (or all) size histograms.
>>> +#
>>> +# @id: The name or QOM path of the guest device.
>>> +#
>>> +# @boundaries: list of interval boundary values (see description in
>>> +#              BlockSizeHistogramInfo definition). If specified, all
>>> +#              size histograms are removed, and empty ones created for all
>>> +#              io types with intervals corresponding to @boundaries (except for
>>> +#              io types, for which specific boundaries are set through the
>>> +#              following parameters).
>>> +#
>>> +# @boundaries-read: list of interval boundary values for read size
>>> +#                   histogram. If specified, old read size histogram is
>>> +#                   removed, and empty one created with intervals
>>> +#                   corresponding to @boundaries-read. The parameter has higher
>>> +#                   priority then @boundaries.
>>> +#
>>> +# @boundaries-write: list of interval boundary values for write size
>>> +#                    histogram.
>>> +#
>>> +# @boundaries-flush: list of interval boundary values for flush size
>>> +#                    histogram.
>>> +#
>>> +# Returns: error if device is not found or any boundary arrays are invalid.
>>> +#
>>> +# Since: 4.0
>> 4.1
>>
>>> +#
>>> +# Example: set new histograms for all io types with intervals
>>> +# [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf):
>>> +#
>>> +# -> { "execute": "x-block-size-histogram-set",
>>> +#      "arguments": { "id": "drive0",
>>> +#                     "boundaries": [8193, 32769, 131073] } }
>>> +# <- { "return": {} }
>>> +#
>>> +# Example: set new histogram only for write, other histograms will remain
>>> +# not changed (or not created):
>>> +#
>>> +# -> { "execute": "x-block-size-histogram-set",
>>> +#      "arguments": { "id": "drive0",
>>> +#                     "boundaries-write": [8193, 32769, 131073] } }
>>> +# <- { "return": {} }
>>> +#
>>> +# Example: set new histograms with the following intervals:
>>> +#   read, flush: [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf)
>>> +#   write: [0, 4097), [4097, 8193), [8193, 32769), [32769, +inf)
>>> +#
>>> +# -> { "execute": "x-block-size-histogram-set",
>>> +#      "arguments": { "id": "drive0",
>>> +#                     "boundaries": [8193, 32769, 131073],
>>> +#                     "boundaries-write": [4097, 8193, 32769] } }
>>> +# <- { "return": {} }
>>> +#
>>> +# Example: remove all size histograms:
>>> +#
>>> +# -> { "execute": "x-block-size-histogram-set",
>>> +#      "arguments": { "id": "drive0" } }
>>> +# <- { "return": {} }
>>> +##
>>> +{ 'command': 'x-block-size-histogram-set',
>>> +  'data': {'id': 'str',
>>> +           '*boundaries': ['uint64'],
>>> +           '*boundaries-read': ['uint64'],
>>> +           '*boundaries-write': ['uint64'],
>>> +           '*boundaries-flush': ['uint64'] } }
>> Again, this copies heavily from block-latency-histogram-set.  But
>> changing the command name is not API compatible.  Should we have a
>> single new command 'block-histogram-set' which takes an enum choosing
>> between 'latency' and 'size', and start the deprecation clock on
>> 'block-latency-histogram-set'?
>>   (and defaulting to 'latency' for back-compat
>>
> zhenwei: this actually copies from block-latency-histogram-set, because the
> two APIs have the similar logic but different structure
> BlockLatencyHistogramInfo and BlockSizeHistogramInfo.
> For further extension, a single new command 'block-histogram-set' with
> enum operation is better.
> So, should I remove 'block-latency-histogram-set' and add 'block-histogram-set'?
> 

Hi Zhenwei!

Glad to see that my work is useful not only for my company! And sad that you didn't
propose it before dropping x-prefixes. But the interface is yang and I think it's
deprecation should not be a problem (for us at least, if I'm not mistaken, we only
use it for something near to internal purposes)..

Then, agree with Eric that it's better to make it common, avoiding duplication. Even
if we keep both commands (and anyway we have to, at least during deprecation period)
we can put their parameters to a separate structure to be shared between them, like
BlockDirtyBitmap is shared parameter structure for block-dirty-bitmap-clear,
block-dirty-bitmap-enable, etc.

Interesting, can we go further and create some generic histogram API, not only for
block subsystem? Hmm.. I don't see a compatible solution.. Histogram possible should
be a separate object with personal id, and than we bind it to our block stats, but
this definitely more complicated API than what we have..
diff mbox series

Patch

diff --git a/block/qapi.c b/block/qapi.c
index f3a84f776e..04edbd5243 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -428,6 +428,20 @@  static void bdrv_latency_histogram_stats(BlockHistogram *hist,
     }
 }
 
+static void bdrv_size_histogram_stats(BlockHistogram *hist,
+                                         bool *not_null,
+                                         BlockSizeHistogramInfo **info)
+{
+    *not_null = hist->bins != NULL;
+    if (*not_null) {
+        *info = g_new0(BlockSizeHistogramInfo, 1);
+
+        (*info)->boundaries = uint64_list(hist->boundaries, hist->nbins - 1);
+        (*info)->bins = uint64_list(hist->bins, hist->nbins);
+    }
+}
+
+
 static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
 {
     BlockAcctStats *stats = blk_get_stats(blk);
@@ -503,6 +517,16 @@  static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
     bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_FLUSH],
                                  &ds->has_flush_latency_histogram,
                                  &ds->flush_latency_histogram);
+
+    bdrv_size_histogram_stats(&stats->size_histogram[BLOCK_ACCT_READ],
+                                 &ds->has_x_rd_size_histogram,
+                                 &ds->x_rd_size_histogram);
+    bdrv_size_histogram_stats(&stats->size_histogram[BLOCK_ACCT_WRITE],
+                                 &ds->has_x_wr_size_histogram,
+                                 &ds->x_wr_size_histogram);
+    bdrv_size_histogram_stats(&stats->size_histogram[BLOCK_ACCT_FLUSH],
+                                 &ds->has_x_flush_size_histogram,
+                                 &ds->x_flush_size_histogram);
 }
 
 static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs,
diff --git a/blockdev.c b/blockdev.c
index 5d6a13dea9..c3f893891d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4563,6 +4563,62 @@  void qmp_block_latency_histogram_set(
     }
 }
 
+void qmp_x_block_size_histogram_set(
+    const char *id,
+    bool has_boundaries, uint64List *boundaries,
+    bool has_boundaries_read, uint64List *boundaries_read,
+    bool has_boundaries_write, uint64List *boundaries_write,
+    bool has_boundaries_flush, uint64List *boundaries_flush,
+    Error **errp)
+{
+    BlockBackend *blk = qmp_get_blk(NULL, id, errp);
+    BlockAcctStats *stats;
+    int ret;
+
+    if (!blk) {
+        return;
+    }
+
+    stats = blk_get_stats(blk);
+
+    if (!has_boundaries && !has_boundaries_read && !has_boundaries_write &&
+        !has_boundaries_flush)
+    {
+        block_size_histograms_clear(stats);
+        return;
+    }
+
+    if (has_boundaries || has_boundaries_read) {
+        ret = block_size_histogram_set(
+            stats, BLOCK_ACCT_READ,
+            has_boundaries_read ? boundaries_read : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set read boundaries fail", id);
+            return;
+        }
+    }
+
+    if (has_boundaries || has_boundaries_write) {
+        ret = block_size_histogram_set(
+            stats, BLOCK_ACCT_WRITE,
+            has_boundaries_write ? boundaries_write : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set write boundaries fail", id);
+            return;
+        }
+    }
+
+    if (has_boundaries || has_boundaries_flush) {
+        ret = block_size_histogram_set(
+            stats, BLOCK_ACCT_FLUSH,
+            has_boundaries_flush ? boundaries_flush : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set flush boundaries fail", id);
+            return;
+        }
+    }
+}
+
 QemuOptsList qemu_common_drive_opts = {
     .name = "drive",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d43d4f37c..cae45c9db5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -633,6 +633,100 @@ 
            '*boundaries-flush': ['uint64'] } }
 
 ##
+# @BlockSizeHistogramInfo:
+#
+# Block size histogram.
+#
+# @boundaries: list of interval boundary values in nanoseconds, all greater
+#              than zero and in ascending order.
+#              For example, the list [8193, 32769, 131073] produces the
+#              following histogram intervals:
+#              [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf).
+#
+# @bins: list of io request counts corresponding to histogram intervals.
+#        len(@bins) = len(@boundaries) + 1
+#        For the example above, @bins may be something like [6, 3, 7, 9],
+#        and corresponding histogram looks like:
+#
+# Since: 4.0
+##
+{ 'struct': 'BlockSizeHistogramInfo',
+  'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }
+
+##
+# @x-block-size-histogram-set:
+#
+# Manage read, write and flush size histograms for the device.
+#
+# If only @id parameter is specified, remove all present size histograms
+# for the device. Otherwise, add/reset some of (or all) size histograms.
+#
+# @id: The name or QOM path of the guest device.
+#
+# @boundaries: list of interval boundary values (see description in
+#              BlockSizeHistogramInfo definition). If specified, all
+#              size histograms are removed, and empty ones created for all
+#              io types with intervals corresponding to @boundaries (except for
+#              io types, for which specific boundaries are set through the
+#              following parameters).
+#
+# @boundaries-read: list of interval boundary values for read size
+#                   histogram. If specified, old read size histogram is
+#                   removed, and empty one created with intervals
+#                   corresponding to @boundaries-read. The parameter has higher
+#                   priority then @boundaries.
+#
+# @boundaries-write: list of interval boundary values for write size
+#                    histogram.
+#
+# @boundaries-flush: list of interval boundary values for flush size
+#                    histogram.
+#
+# Returns: error if device is not found or any boundary arrays are invalid.
+#
+# Since: 4.0
+#
+# Example: set new histograms for all io types with intervals
+# [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf):
+#
+# -> { "execute": "x-block-size-histogram-set",
+#      "arguments": { "id": "drive0",
+#                     "boundaries": [8193, 32769, 131073] } }
+# <- { "return": {} }
+#
+# Example: set new histogram only for write, other histograms will remain
+# not changed (or not created):
+#
+# -> { "execute": "x-block-size-histogram-set",
+#      "arguments": { "id": "drive0",
+#                     "boundaries-write": [8193, 32769, 131073] } }
+# <- { "return": {} }
+#
+# Example: set new histograms with the following intervals:
+#   read, flush: [0, 8193), [8193, 32769), [32769, 131073), [131073, +inf)
+#   write: [0, 4097), [4097, 8193), [8193, 32769), [32769, +inf)
+#
+# -> { "execute": "x-block-size-histogram-set",
+#      "arguments": { "id": "drive0",
+#                     "boundaries": [8193, 32769, 131073],
+#                     "boundaries-write": [4097, 8193, 32769] } }
+# <- { "return": {} }
+#
+# Example: remove all size histograms:
+#
+# -> { "execute": "x-block-size-histogram-set",
+#      "arguments": { "id": "drive0" } }
+# <- { "return": {} }
+##
+{ 'command': 'x-block-size-histogram-set',
+  'data': {'id': 'str',
+           '*boundaries': ['uint64'],
+           '*boundaries-read': ['uint64'],
+           '*boundaries-write': ['uint64'],
+           '*boundaries-flush': ['uint64'] } }
+
+
+##
 # @BlockInfo:
 #
 # Block device information.  This structure describes a virtual device and
@@ -918,6 +1012,12 @@ 
 #
 # @flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
 #
+# @x_rd_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
+#
+# @x_wr_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
+#
+# @x_flush_size_histogram: @BlockSizeHistogramInfo. (Since 4.0)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'BlockDeviceStats',
@@ -933,7 +1033,10 @@ 
            'timed_stats': ['BlockDeviceTimedStats'],
            '*rd_latency_histogram': 'BlockLatencyHistogramInfo',
            '*wr_latency_histogram': 'BlockLatencyHistogramInfo',
-           '*flush_latency_histogram': 'BlockLatencyHistogramInfo' } }
+           '*flush_latency_histogram': 'BlockLatencyHistogramInfo',
+           '*x_rd_size_histogram': 'BlockSizeHistogramInfo',
+           '*x_wr_size_histogram': 'BlockSizeHistogramInfo',
+           '*x_flush_size_histogram': 'BlockSizeHistogramInfo' } }
 
 ##
 # @BlockStats: