11.03.2018 06:20, Eric Blake wrote:
On 03/09/2018 10:52 AM, Vladimir Sementsov-Ogievskiy wrote:
Set (and clear) histogram through new command
block-latency-histogram-set and show new statistics in
query-blockstats results.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
qapi/block-core.json | 111
++++++++++++++++++++++++++++++++++++++++++++++++++-
block/qapi.c | 41 +++++++++++++++++++
blockdev.c | 43 ++++++++++++++++++++
3 files changed, 194 insertions(+), 1 deletion(-)
+# @boundaries: list of interval boundary values in nanoseconds, all
greater
+# than zero and in ascending order.
+# For example, the list [10, 50, 100] produces the
following
+# histogram intervals: [0, 10), [10, 50), [50, 100),
[100, +inf).
10 vs. 50 nanoseconds is a bit unrealistic; the example makes sense
but you may want to scale the boundaries into values that are more
likely to make sense during use. Can be a followup.
I've just taken Paolo's suggestion), not realistic, but do not occupy so
much space as mine. Be free to adjust it if you want
+#
+# @bins: list of io request counts corresponding to histogram
intervals.
+# len(@bins) = len(@boundaries) + 1
+# For the example above, @bins may be something like [3, 1,
5, 2],
+# and corresponding histogram looks like:
+#
+# 5| *
+# 4| *
+# 3| * *
+# 2| * * *
+# 1| * * * *
+# +------------------
+# 10 50 100
+#
+# Since: 2.12
+##
+{ 'struct': 'XBlockLatencyHistogramInfo',
Struct names have no impact to introspection; I see no reason to use a
leading X here.
+ 'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }
+
+##
+# @x-block-latency-histogram-set:
I guess you've decided that this should be experimental in 2.12? If
you want to change the name and promote it to a released interface,
you don't have much time left.
Yes. This is not random feature, it's required by our customers. And I
understood (after I remake it to support different bins for different io
types) that we are on the early stage of doing it, so it would be good
to have it in 2.12 with x-.
+#
+# @boundaries-read: list of interval boundary values for read latency
+# histogram. If specified, old read latency
histogram is
+# removed, and empty one created with interavals
s/interavals/intervals/
+# corresponding to @boundaries-read. The parameter
has higher
+# priority then @boundaries.
+#
+# @boundaries-write: list of interaval boundary values for write
latency
and again
+# histogram.
+#
+# @boundaries-flush: list of interaval boundary values for flush
latency
copy-paste strikes again :)
+# histogram.
+#
+# Returns: error if device is not found or @boundaries* arrays are
invalid.
s/@boundaries*/any boundary arrays/
+#
+# Since: 2.12
+#
+# Example: set new histograms for all io types with intervals
+# [0, 10), [10, 50), [50, 100), [100, +inf):
Again, are these reasonable numbers in nanoseconds? And spelling out
the time unit in the example documentation may be helpful.
@@ -730,6 +830,12 @@
# @timed_stats: Statistics specific to the set of previously defined
# intervals of time (Since 2.5)
#
+# @x_rd_latency_histogram: @BlockLatencyHistogramInfo. (Since 2.12)
Here, the naming makes sense (the _ is consistent with this being an
older interface and already using it elsewhere, and the leading x
matches with your command being marked experimental).
+++ b/blockdev.c
@@ -4180,6 +4180,49 @@ void qmp_x_blockdev_set_iothread(const char
*node_name, StrOrNull *iothread,
aio_context_release(old_context);
}
+void qmp_x_block_latency_histogram_set(
+ const char *device,
I'll fix up the trivial typos, but you may want a followup patch.
Thank you!
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Best regards,
Vladimir