[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v0] fsdev: QMP interface for throttling
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v0] fsdev: QMP interface for throttling |
Date: |
Mon, 20 Mar 2017 20:15:06 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 03/20/2017 08:07 AM, Pradeep Jagadeesh wrote:
> This patchset enables qmp interfaces for the 9pfs
> devices (fsdev).This provides two interfaces one
Space between English sentences, after '.'
> for querying all the 9pfs devices info. The second one
> to set the IO limits for the required 9pfs device.
>
> Signed-off-by: Pradeep Jagadeesh <address@hidden>
> ---
> +++ b/qapi-schema.json
> @@ -81,6 +81,9 @@
> # QAPI block definitions
> { 'include': 'qapi/block.json' }
>
> +# QAPI 9pfs definitions
> +{ 'include': 'qapi/9pfs.json' }
> +
> # QAPI event definitions
> { 'include': 'qapi/event.json' }
>
> diff --git a/qapi/9pfs.json b/qapi/9pfs.json
> new file mode 100644
> index 0000000..c068474
> --- /dev/null
> +++ b/qapi/9pfs.json
> @@ -0,0 +1,169 @@
> +# -*- Mode: Python -*-
> +
> +##
> +# == QAPI 9p definitions
> +##
> +
> +# QAPI common definitions
> +{ 'include': 'common.json' }
> +
> +##
> +# @fs9p_set_io_throttle:
> +#
> +# Change I/O limits for a 9p/fsdev device.
> +#
> +# Since QEMU 2.9, I/0 limits can be enabled on each fsdev(9pfs) device
This says 2.9...
> +#
> +# I/O limits can be disabled by setting all of them to 0.
> +#
> +# Returns: Nothing on success
> +# If @device is not a valid 9p device, DeviceNotFound
> +#
> +# Since: 2:10
...but this says 2:10 (typo, should be 2.10). No need to mention the
version twice, especially if one of them is wrong (keep the Since: line).
> +#
> +# Example:
> +#
> +# -> { "execute": "fs9p_set_io_throttle",
> +# "arguments": { "device": "ide0-1-0",
> +# "bps": 1000000,
> +# "bps_rd": 0,
> +# "bps_wr": 0,
> +# "iops": 0,
> +# "iops_rd": 0,
> +# "iops_wr": 0,
> +# "bps_max": 8000000,
> +# "bps_rd_max": 0,
> +# "bps_wr_max": 0,
> +# "iops_max": 0,
> +# "iops_rd_max": 0,
> +# "iops_wr_max": 0,
> +# "bps_max_length": 60,
> +# "iops_size": 0 } }
> +# <- { "returns": {} }
> +##
> +{ 'command': 'fs9p_set_io_throttle', 'boxed': true,
> + 'data': 'FS9PIOThrottle' }
New commands and members should be named with '-' rather than '_' as the
word separator, so this should be 'fs9p-set-io-throttle', 'bps-rd', etc.
> +##
> +# @FS9PIOThrottle:
> +#
> +# A set of parameters describing block
> +#
> +# @device: Block device name
> +#
> +# @bps: total throughput limit in bytes per second
> +#
> +# @bps_rd: read throughput limit in bytes per second
> +#
> +# @bps_wr: write throughput limit in bytes per second
> +#
> +# @iops: total I/O operations per second
> +#
> +# @iops_rd: read I/O operations per second
> +#
> +# @iops_wr: write I/O operations per second
> +#
> +# @bps_max: total throughput limit during bursts,
> +# in bytes (Since 1.7)
You're introducing this struct in 2.10, so this member is not since 1.7.
Either that, or you're copying-and-pasting when you should be sharing
code and reusing an existing struct.
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'FS9PIOThrottle',
> + 'data': { '*device': 'str', 'bps': 'int', 'bps_rd': 'int',
> + 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr':
> 'int',
> + '*bps_max': 'int', '*bps_rd_max': 'int',
> + '*bps_wr_max': 'int', '*iops_max': 'int',
> + '*iops_rd_max': 'int', '*iops_wr_max': 'int',
> + '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
> + '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
> + '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
> + '*iops_size': 'int' } }
If you reuse an existing struct that uses _ instead of -, then that
explains your naming. But in that case, why do you need to declare a
new (copied) struct, instead of just reusing the existing one?
> +
> +##
> +# @query-9pfs-io-throttle:
> +#
> +# Return a list of information about each iothread
> +#
> +# Returns: @FS9PIOIOThrottle
> +#
> +# Since: 2.10
> +#
> +# Example:
> +#
> +# -> { "Execute": "query-9pfs-io-throttle" }
> +# <- { "returns" : [
> +# {
> +# "device": "ide0-hd0",
> +# "bps":1000000,
> +# "bps_rd":0,
> +# "bps_wr":0,
> +# "iops":1000000,
> +# "iops_rd":0,
> +# "iops_wr":0,
> +# "bps_max": 8000000,
> +# "bps_rd_max": 0,
> +# "bps_wr_max": 0,
You are not consistent on whether to include a space after ':'. The
easiest way to get this right is to paste actual output from pretty qmp
mode.
> +# "iops_max": 0,
> +# "iops_rd_max": 0,
> +# "iops_wr_max": 0,
> +# "bps_max_length": 0,
> +# "bps_rd_max_length": 0,
> +# "bps_wr_max_length": 0,
> +# "iops_max_length": 0,
> +# "iops_rd_max_length": 0,
> +# "iops_wr_max_length": 0,
> +# "iops_size": 0,
> +# }
This is not valid JSON. No trailing commas.
> +# ]
> +# }
> +#
> +##
> +{ 'command': 'query-9pfs-io-throttle', 'returns': [ 'FS9PIOThrottle' ] }
> +
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature