From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwDzc-0007PF-Bi for qemu-devel@nongnu.org; Tue, 10 Nov 2015 13:50:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwDzX-0005Kp-8Q for qemu-devel@nongnu.org; Tue, 10 Nov 2015 13:50:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53878) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwDzX-0005Ke-3Z for qemu-devel@nongnu.org; Tue, 10 Nov 2015 13:50:03 -0500 From: Markus Armbruster References: <1447164879-6756-1-git-send-email-stefanha@redhat.com> <1447164879-6756-34-git-send-email-stefanha@redhat.com> <56422818.601@redhat.com> Date: Tue, 10 Nov 2015 19:49:59 +0100 In-Reply-To: <56422818.601@redhat.com> (Eric Blake's message of "Tue, 10 Nov 2015 10:23:36 -0700") Message-ID: <876119n0yw.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PULL 33/44] block: New option to define the intervals for collecting I/O statistics List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Peter Maydell , Alberto Garcia , qemu-devel@nongnu.org, Stefan Hajnoczi Eric Blake writes: > On 11/10/2015 07:14 AM, Stefan Hajnoczi wrote: >> From: Alberto Garcia >> >> The BlockAcctStats structure contains a list of BlockAcctTimedStats. >> Each one of these collects statistics about the minimum, maximum and >> average latencies of all I/O operations in a certain interval of time. >> >> This patch adds a new "stats-intervals" option that allows defining >> these intervals. >> >> Signed-off-by: Alberto Garcia >> Message-id: 41cbcd334a61c6157f0f495cdfd21eff6c156f2a.1446044837.git.berto@igalia.com >> Signed-off-by: Stefan Hajnoczi >> --- >> blockdev.c | 37 +++++++++++++++++++++++++++++++++++++ >> qapi/block-core.json | 4 ++++ >> 2 files changed, 41 insertions(+) > >> +++ b/qapi/block-core.json >> @@ -1503,6 +1503,9 @@ >> # @stats-account-failed: #optional whether to include failed >> # operations when computing latency and last >> # access statistics (default: true) (Since 2.5) >> +# @stats-intervals: #optional colon-separated list of intervals for >> +# collecting I/O statistics, in seconds (default: none) >> +# (Since 2.5) > > Eww. Sorry for not noticing this sooner, but can we please fix this to be: > > '*stats-intervals':['int'] > > Having to post-process parse for colons means that the JSON interface > was not properly defined. Basic QMP rule: never encode in strings when a natural JSON encoding exists. If you want a fancy string encoding for HMP or command line, use suitable visitors. If I remember correctly, NumaNodeOptions member cpus can serve as example for a list of integers. Suggest to start at parse_numa(). > I'm okay if the fix is a followup, but we need to get it in before 2.5 > bakes in the gross interface. For 2.5, either fix it, revert it, or rename it to x-. It must not become ABI in this state. >> # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1) >> # (default: off) >> # >> @@ -1520,6 +1523,7 @@ >> '*read-only': 'bool', >> '*stats-account-invalid': 'bool', >> '*stats-account-failed': 'bool', >> + '*stats-intervals': 'str', >> '*detect-zeroes': 'BlockdevDetectZeroesOptions' } } >> >> ## >>