| Message ID | 1499182263-19139-3-git-send-email-pradeep.jagadeesh@huawei.com |
|---|---|
| State | New |
| Headers | show |
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes: > This patch enables qmp interfaces for the fsdev > devices. This provides two interfaces one > for querying info of all the fsdev devices. The second one > to set the IO limits for the required fsdev device. > > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> > Reviewed-by: Greg Kurz <groug@kaod.org> > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Alberto Garcia <berto@igalia.com> > --- > qapi/block-core.json | 76 ++------------------------------------------- > qapi/iothrottle.json | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 91 insertions(+), 73 deletions(-) > create mode 100644 qapi/iothrottle.json > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index f85c223..9320974 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -6,6 +6,7 @@ > > # QAPI common definitions > { 'include': 'common.json' } > +{ 'include': 'iothrottle.json' } > > ## > # @SnapshotInfo: > @@ -1761,84 +1762,13 @@ > # > # @device: Block device name (deprecated, use @id instead) > # > -# @id: The name or QOM path of the guest device (since: 2.8) > -# > -# @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) > -# > -# @bps_rd_max: read throughput limit during bursts, > -# in bytes (Since 1.7) > -# > -# @bps_wr_max: write throughput limit during bursts, > -# in bytes (Since 1.7) > -# > -# @iops_max: total I/O operations per second during bursts, > -# in bytes (Since 1.7) > -# > -# @iops_rd_max: read I/O operations per second during bursts, > -# in bytes (Since 1.7) > -# > -# @iops_wr_max: write I/O operations per second during bursts, > -# in bytes (Since 1.7) > -# > -# @bps_max_length: maximum length of the @bps_max burst > -# period, in seconds. It must only > -# be set if @bps_max is set as well. > -# Defaults to 1. (Since 2.6) > -# > -# @bps_rd_max_length: maximum length of the @bps_rd_max > -# burst period, in seconds. It must only > -# be set if @bps_rd_max is set as well. > -# Defaults to 1. (Since 2.6) > -# > -# @bps_wr_max_length: maximum length of the @bps_wr_max > -# burst period, in seconds. It must only > -# be set if @bps_wr_max is set as well. > -# Defaults to 1. (Since 2.6) > -# > -# @iops_max_length: maximum length of the @iops burst > -# period, in seconds. It must only > -# be set if @iops_max is set as well. > -# Defaults to 1. (Since 2.6) > -# > -# @iops_rd_max_length: maximum length of the @iops_rd_max > -# burst period, in seconds. It must only > -# be set if @iops_rd_max is set as well. > -# Defaults to 1. (Since 2.6) > -# > -# @iops_wr_max_length: maximum length of the @iops_wr_max > -# burst period, in seconds. It must only > -# be set if @iops_wr_max is set as well. > -# Defaults to 1. (Since 2.6) > -# > -# @iops_size: an I/O size in bytes (Since 1.7) > -# > # @group: throttle group name (Since 2.4) > # > # Since: 1.1 > ## > { 'struct': 'BlockIOThrottle', > - 'data': { '*device': 'str', '*id': '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', '*group': 'str' } } > + 'base': 'IOThrottle', > + 'data': { '*device': 'str', '*group': 'str' } } > > ## > # @block-stream: > diff --git a/qapi/iothrottle.json b/qapi/iothrottle.json > new file mode 100644 > index 0000000..0f067c3 > --- /dev/null > +++ b/qapi/iothrottle.json > @@ -0,0 +1,88 @@ > +# -*- Mode: Python -*- > + > +## > +# == QAPI IOThrottle definitions > +## > + > +## > +# @IOThrottle: > +# > +# A set of parameters describing IO throttling > +# > +# @id: The name or QOM path of the guest device (since: 2.8) > +# > +# @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) > +# > +# @bps_rd_max: read throughput limit during bursts, > +# in bytes (Since 1.7) > +# > +# @bps_wr_max: write throughput limit during bursts, > +# in bytes (Since 1.7) > +# > +# @iops_max: total I/O operations per second during bursts, > +# in bytes (Since 1.7) > +# > +# @iops_rd_max: read I/O operations per second during bursts, > +# in bytes (Since 1.7) > +# > +# @iops_wr_max: write I/O operations per second during bursts, > +# in bytes (Since 1.7) > +# > +# @bps_max_length: maximum length of the @bps_max burst > +# period, in seconds. It must only > +# be set if @bps_max is set as well. > +# Defaults to 1. (Since 2.6) > +# > +# @bps_rd_max_length: maximum length of the @bps_rd_max > +# burst period, in seconds. It must only > +# be set if @bps_rd_max is set as well. > +# Defaults to 1. (Since 2.6) > +# > +# @bps_wr_max_length: maximum length of the @bps_wr_max > +# burst period, in seconds. It must only > +# be set if @bps_wr_max is set as well. > +# Defaults to 1. (Since 2.6) > +# > +# @iops_max_length: maximum length of the @iops burst > +# period, in seconds. It must only > +# be set if @iops_max is set as well. > +# Defaults to 1. (Since 2.6) > +# > +# @iops_rd_max_length: maximum length of the @iops_rd_max > +# burst period, in seconds. It must only > +# be set if @iops_rd_max is set as well. > +# Defaults to 1. (Since 2.6) > +# > +# @iops_wr_max_length: maximum length of the @iops_wr_max > +# burst period, in seconds. It must only > +# be set if @iops_wr_max is set as well. > +# Defaults to 1. (Since 2.6) > +# > +# @iops_size: an I/O size in bytes (Since 1.7) > +# > +# > +# Since: 2.10 > +## > +{ 'struct': 'IOThrottle', > + 'data': { '*id': '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' } } Awkward question for a v7, but here goes anyway: why is IOThrottle worth its very own .json file? Diff can't show the differences (if any), because you factor IOThrottle out of BlockIOThrottle and move it in a single patch. I had to extract and diff by hand.
On 7/6/2017 7:55 PM, Markus Armbruster wrote: > Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes: > >> This patch enables qmp interfaces for the fsdev >> devices. This provides two interfaces one >> for querying info of all the fsdev devices. The second one >> to set the IO limits for the required fsdev device. >> >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> >> Reviewed-by: Greg Kurz <groug@kaod.org> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> Reviewed-by: Alberto Garcia <berto@igalia.com> >> --- >> qapi/block-core.json | 76 ++------------------------------------------- >> qapi/iothrottle.json | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 91 insertions(+), 73 deletions(-) >> create mode 100644 qapi/iothrottle.json >> >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index f85c223..9320974 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -6,6 +6,7 @@ >> >> # QAPI common definitions >> { 'include': 'common.json' } >> +{ 'include': 'iothrottle.json' } >> >> ## >> # @SnapshotInfo: >> @@ -1761,84 +1762,13 @@ >> # >> # @device: Block device name (deprecated, use @id instead) >> # >> -# @id: The name or QOM path of the guest device (since: 2.8) >> -# >> -# @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) >> -# >> -# @bps_rd_max: read throughput limit during bursts, >> -# in bytes (Since 1.7) >> -# >> -# @bps_wr_max: write throughput limit during bursts, >> -# in bytes (Since 1.7) >> -# >> -# @iops_max: total I/O operations per second during bursts, >> -# in bytes (Since 1.7) >> -# >> -# @iops_rd_max: read I/O operations per second during bursts, >> -# in bytes (Since 1.7) >> -# >> -# @iops_wr_max: write I/O operations per second during bursts, >> -# in bytes (Since 1.7) >> -# >> -# @bps_max_length: maximum length of the @bps_max burst >> -# period, in seconds. It must only >> -# be set if @bps_max is set as well. >> -# Defaults to 1. (Since 2.6) >> -# >> -# @bps_rd_max_length: maximum length of the @bps_rd_max >> -# burst period, in seconds. It must only >> -# be set if @bps_rd_max is set as well. >> -# Defaults to 1. (Since 2.6) >> -# >> -# @bps_wr_max_length: maximum length of the @bps_wr_max >> -# burst period, in seconds. It must only >> -# be set if @bps_wr_max is set as well. >> -# Defaults to 1. (Since 2.6) >> -# >> -# @iops_max_length: maximum length of the @iops burst >> -# period, in seconds. It must only >> -# be set if @iops_max is set as well. >> -# Defaults to 1. (Since 2.6) >> -# >> -# @iops_rd_max_length: maximum length of the @iops_rd_max >> -# burst period, in seconds. It must only >> -# be set if @iops_rd_max is set as well. >> -# Defaults to 1. (Since 2.6) >> -# >> -# @iops_wr_max_length: maximum length of the @iops_wr_max >> -# burst period, in seconds. It must only >> -# be set if @iops_wr_max is set as well. >> -# Defaults to 1. (Since 2.6) >> -# >> -# @iops_size: an I/O size in bytes (Since 1.7) >> -# >> # @group: throttle group name (Since 2.4) >> # >> # Since: 1.1 >> ## >> { 'struct': 'BlockIOThrottle', >> - 'data': { '*device': 'str', '*id': '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', '*group': 'str' } } >> + 'base': 'IOThrottle', >> + 'data': { '*device': 'str', '*group': 'str' } } >> >> ## >> # @block-stream: >> diff --git a/qapi/iothrottle.json b/qapi/iothrottle.json >> new file mode 100644 >> index 0000000..0f067c3 >> --- /dev/null >> +++ b/qapi/iothrottle.json >> @@ -0,0 +1,88 @@ >> +# -*- Mode: Python -*- >> + >> +## >> +# == QAPI IOThrottle definitions >> +## >> + >> +## >> +# @IOThrottle: >> +# >> +# A set of parameters describing IO throttling >> +# >> +# @id: The name or QOM path of the guest device (since: 2.8) >> +# >> +# @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) >> +# >> +# @bps_rd_max: read throughput limit during bursts, >> +# in bytes (Since 1.7) >> +# >> +# @bps_wr_max: write throughput limit during bursts, >> +# in bytes (Since 1.7) >> +# >> +# @iops_max: total I/O operations per second during bursts, >> +# in bytes (Since 1.7) >> +# >> +# @iops_rd_max: read I/O operations per second during bursts, >> +# in bytes (Since 1.7) >> +# >> +# @iops_wr_max: write I/O operations per second during bursts, >> +# in bytes (Since 1.7) >> +# >> +# @bps_max_length: maximum length of the @bps_max burst >> +# period, in seconds. It must only >> +# be set if @bps_max is set as well. >> +# Defaults to 1. (Since 2.6) >> +# >> +# @bps_rd_max_length: maximum length of the @bps_rd_max >> +# burst period, in seconds. It must only >> +# be set if @bps_rd_max is set as well. >> +# Defaults to 1. (Since 2.6) >> +# >> +# @bps_wr_max_length: maximum length of the @bps_wr_max >> +# burst period, in seconds. It must only >> +# be set if @bps_wr_max is set as well. >> +# Defaults to 1. (Since 2.6) >> +# >> +# @iops_max_length: maximum length of the @iops burst >> +# period, in seconds. It must only >> +# be set if @iops_max is set as well. >> +# Defaults to 1. (Since 2.6) >> +# >> +# @iops_rd_max_length: maximum length of the @iops_rd_max >> +# burst period, in seconds. It must only >> +# be set if @iops_rd_max is set as well. >> +# Defaults to 1. (Since 2.6) >> +# >> +# @iops_wr_max_length: maximum length of the @iops_wr_max >> +# burst period, in seconds. It must only >> +# be set if @iops_wr_max is set as well. >> +# Defaults to 1. (Since 2.6) >> +# >> +# @iops_size: an I/O size in bytes (Since 1.7) >> +# >> +# >> +# Since: 2.10 >> +## >> +{ 'struct': 'IOThrottle', >> + 'data': { '*id': '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' } } > > Awkward question for a v7, but here goes anyway: why is IOThrottle worth > its very own .json file? I feel this is a common throttle structure that is used by block devices as well as fsdev, so moved to a separate file. -Pradeep > > Diff can't show the differences (if any), because you factor IOThrottle > out of BlockIOThrottle and move it in a single patch. I had to extract > and diff by hand. >
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> writes: > On 7/6/2017 7:55 PM, Markus Armbruster wrote: >> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes: >> >>> This patch enables qmp interfaces for the fsdev >>> devices. This provides two interfaces one >>> for querying info of all the fsdev devices. The second one >>> to set the IO limits for the required fsdev device. >>> >>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> >>> Reviewed-by: Greg Kurz <groug@kaod.org> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> Reviewed-by: Alberto Garcia <berto@igalia.com> >>> --- >>> qapi/block-core.json | 76 ++------------------------------------------- >>> qapi/iothrottle.json | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 91 insertions(+), 73 deletions(-) >>> create mode 100644 qapi/iothrottle.json >>> >>> diff --git a/qapi/block-core.json b/qapi/block-core.json >>> index f85c223..9320974 100644 >>> --- a/qapi/block-core.json >>> +++ b/qapi/block-core.json >>> @@ -6,6 +6,7 @@ >>> >>> # QAPI common definitions >>> { 'include': 'common.json' } >>> +{ 'include': 'iothrottle.json' } >>> >>> ## >>> # @SnapshotInfo: >>> @@ -1761,84 +1762,13 @@ >>> # >>> # @device: Block device name (deprecated, use @id instead) >>> # >>> -# @id: The name or QOM path of the guest device (since: 2.8) >>> -# >>> -# @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) >>> -# >>> -# @bps_rd_max: read throughput limit during bursts, >>> -# in bytes (Since 1.7) >>> -# >>> -# @bps_wr_max: write throughput limit during bursts, >>> -# in bytes (Since 1.7) >>> -# >>> -# @iops_max: total I/O operations per second during bursts, >>> -# in bytes (Since 1.7) >>> -# >>> -# @iops_rd_max: read I/O operations per second during bursts, >>> -# in bytes (Since 1.7) >>> -# >>> -# @iops_wr_max: write I/O operations per second during bursts, >>> -# in bytes (Since 1.7) >>> -# >>> -# @bps_max_length: maximum length of the @bps_max burst >>> -# period, in seconds. It must only >>> -# be set if @bps_max is set as well. >>> -# Defaults to 1. (Since 2.6) >>> -# >>> -# @bps_rd_max_length: maximum length of the @bps_rd_max >>> -# burst period, in seconds. It must only >>> -# be set if @bps_rd_max is set as well. >>> -# Defaults to 1. (Since 2.6) >>> -# >>> -# @bps_wr_max_length: maximum length of the @bps_wr_max >>> -# burst period, in seconds. It must only >>> -# be set if @bps_wr_max is set as well. >>> -# Defaults to 1. (Since 2.6) >>> -# >>> -# @iops_max_length: maximum length of the @iops burst >>> -# period, in seconds. It must only >>> -# be set if @iops_max is set as well. >>> -# Defaults to 1. (Since 2.6) >>> -# >>> -# @iops_rd_max_length: maximum length of the @iops_rd_max >>> -# burst period, in seconds. It must only >>> -# be set if @iops_rd_max is set as well. >>> -# Defaults to 1. (Since 2.6) >>> -# >>> -# @iops_wr_max_length: maximum length of the @iops_wr_max >>> -# burst period, in seconds. It must only >>> -# be set if @iops_wr_max is set as well. >>> -# Defaults to 1. (Since 2.6) >>> -# >>> -# @iops_size: an I/O size in bytes (Since 1.7) >>> -# >>> # @group: throttle group name (Since 2.4) >>> # >>> # Since: 1.1 >>> ## >>> { 'struct': 'BlockIOThrottle', >>> - 'data': { '*device': 'str', '*id': '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', '*group': 'str' } } >>> + 'base': 'IOThrottle', >>> + 'data': { '*device': 'str', '*group': 'str' } } >>> >>> ## >>> # @block-stream: >>> diff --git a/qapi/iothrottle.json b/qapi/iothrottle.json >>> new file mode 100644 >>> index 0000000..0f067c3 >>> --- /dev/null >>> +++ b/qapi/iothrottle.json >>> @@ -0,0 +1,88 @@ >>> +# -*- Mode: Python -*- >>> + >>> +## >>> +# == QAPI IOThrottle definitions >>> +## >>> + >>> +## >>> +# @IOThrottle: >>> +# >>> +# A set of parameters describing IO throttling >>> +# >>> +# @id: The name or QOM path of the guest device (since: 2.8) >>> +# >>> +# @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) >>> +# >>> +# @bps_rd_max: read throughput limit during bursts, >>> +# in bytes (Since 1.7) >>> +# >>> +# @bps_wr_max: write throughput limit during bursts, >>> +# in bytes (Since 1.7) >>> +# >>> +# @iops_max: total I/O operations per second during bursts, >>> +# in bytes (Since 1.7) >>> +# >>> +# @iops_rd_max: read I/O operations per second during bursts, >>> +# in bytes (Since 1.7) >>> +# >>> +# @iops_wr_max: write I/O operations per second during bursts, >>> +# in bytes (Since 1.7) >>> +# >>> +# @bps_max_length: maximum length of the @bps_max burst >>> +# period, in seconds. It must only >>> +# be set if @bps_max is set as well. >>> +# Defaults to 1. (Since 2.6) >>> +# >>> +# @bps_rd_max_length: maximum length of the @bps_rd_max >>> +# burst period, in seconds. It must only >>> +# be set if @bps_rd_max is set as well. >>> +# Defaults to 1. (Since 2.6) >>> +# >>> +# @bps_wr_max_length: maximum length of the @bps_wr_max >>> +# burst period, in seconds. It must only >>> +# be set if @bps_wr_max is set as well. >>> +# Defaults to 1. (Since 2.6) >>> +# >>> +# @iops_max_length: maximum length of the @iops burst >>> +# period, in seconds. It must only >>> +# be set if @iops_max is set as well. >>> +# Defaults to 1. (Since 2.6) >>> +# >>> +# @iops_rd_max_length: maximum length of the @iops_rd_max >>> +# burst period, in seconds. It must only >>> +# be set if @iops_rd_max is set as well. >>> +# Defaults to 1. (Since 2.6) >>> +# >>> +# @iops_wr_max_length: maximum length of the @iops_wr_max >>> +# burst period, in seconds. It must only >>> +# be set if @iops_wr_max is set as well. >>> +# Defaults to 1. (Since 2.6) >>> +# >>> +# @iops_size: an I/O size in bytes (Since 1.7) >>> +# >>> +# >>> +# Since: 2.10 >>> +## >>> +{ 'struct': 'IOThrottle', >>> + 'data': { '*id': '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' } } >> >> Awkward question for a v7, but here goes anyway: why is IOThrottle worth >> its very own .json file? > I feel this is a common throttle structure that is used by block > devices as well as fsdev, so moved to a separate file. I'm not sure that's a good idea. Kevin, Berto, what do you think? > > -Pradeep >> >> Diff can't show the differences (if any), because you factor IOThrottle >> out of BlockIOThrottle and move it in a single patch. I had to extract >> and diff by hand. >>
On Mon 07 Aug 2017 04:48:38 PM CEST, Markus Armbruster wrote: >>> Awkward question for a v7, but here goes anyway: why is IOThrottle >>> worth its very own .json file? >> I feel this is a common throttle structure that is used by block >> devices as well as fsdev, so moved to a separate file. > I'm not sure that's a good idea. Kevin, Berto, what do you think? Mmm... I don't have a very strong opinion, but if there's no actual need to move it to a separate file I'd prefer to leave it where it is. Berto
On 8/8/2017 1:30 PM, Alberto Garcia wrote: > On Mon 07 Aug 2017 04:48:38 PM CEST, Markus Armbruster wrote: >>>> Awkward question for a v7, but here goes anyway: why is IOThrottle >>>> worth its very own .json file? >>> I feel this is a common throttle structure that is used by block >>> devices as well as fsdev, so moved to a separate file. >> I'm not sure that's a good idea. Kevin, Berto, what do you think? > > Mmm... I don't have a very strong opinion, but if there's no actual need > to move it to a separate file I'd prefer to leave it where it is. The segregation is the solid reason. Because throttling is a feature that is used by fsdev, block may many more in future. I do not see moving it back to block does it make any sense? Regards, Pradeep > > Berto >
On Tue 08 Aug 2017 02:30:43 PM CEST, Pradeep Jagadeesh wrote: > On 8/8/2017 1:30 PM, Alberto Garcia wrote: >> On Mon 07 Aug 2017 04:48:38 PM CEST, Markus Armbruster wrote: >>>>> Awkward question for a v7, but here goes anyway: why is IOThrottle >>>>> worth its very own .json file? >>>> I feel this is a common throttle structure that is used by block >>>> devices as well as fsdev, so moved to a separate file. >>> I'm not sure that's a good idea. Kevin, Berto, what do you think? >> >> Mmm... I don't have a very strong opinion, but if there's no actual need >> to move it to a separate file I'd prefer to leave it where it is. > The segregation is the solid reason. Because throttling is a feature > that is used by fsdev, block may many more in future. I do not see > moving it back to block does it make any sense? It's not "moving it back", it's keeping it where it is. But I see no big problem with moving it to a common file either. Berto
Alberto Garcia <berto@igalia.com> writes: > On Tue 08 Aug 2017 02:30:43 PM CEST, Pradeep Jagadeesh wrote: >> On 8/8/2017 1:30 PM, Alberto Garcia wrote: >>> On Mon 07 Aug 2017 04:48:38 PM CEST, Markus Armbruster wrote: >>>>>> Awkward question for a v7, but here goes anyway: why is IOThrottle >>>>>> worth its very own .json file? >>>>> I feel this is a common throttle structure that is used by block >>>>> devices as well as fsdev, so moved to a separate file. >>>> I'm not sure that's a good idea. Kevin, Berto, what do you think? >>> >>> Mmm... I don't have a very strong opinion, but if there's no actual need >>> to move it to a separate file I'd prefer to leave it where it is. >> The segregation is the solid reason. Because throttling is a feature >> that is used by fsdev, block may many more in future. I do not see >> moving it back to block does it make any sense? > > It's not "moving it back", it's keeping it where it is. But I see no big > problem with moving it to a common file either. I'd rather not put every struct shared across subsystem boundaries in its own file. We can keep it right where it is for now. Bonus: more readable diff. If we start sharing more throttle-related material than just a struct, we can reconsider. We could also move it to the existing file for common stuff: qapi/common.json. Not a great fit, though.
On 8/8/2017 5:18 PM, Markus Armbruster wrote: > Alberto Garcia <berto@igalia.com> writes: > >> On Tue 08 Aug 2017 02:30:43 PM CEST, Pradeep Jagadeesh wrote: >>> On 8/8/2017 1:30 PM, Alberto Garcia wrote: >>>> On Mon 07 Aug 2017 04:48:38 PM CEST, Markus Armbruster wrote: >>>>>>> Awkward question for a v7, but here goes anyway: why is IOThrottle >>>>>>> worth its very own .json file? >>>>>> I feel this is a common throttle structure that is used by block >>>>>> devices as well as fsdev, so moved to a separate file. >>>>> I'm not sure that's a good idea. Kevin, Berto, what do you think? >>>> >>>> Mmm... I don't have a very strong opinion, but if there's no actual need >>>> to move it to a separate file I'd prefer to leave it where it is. >>> The segregation is the solid reason. Because throttling is a feature >>> that is used by fsdev, block may many more in future. I do not see >>> moving it back to block does it make any sense? >> >> It's not "moving it back", it's keeping it where it is. But I see no big >> problem with moving it to a common file either. > > I'd rather not put every struct shared across subsystem boundaries in > its own file. > > We can keep it right where it is for now. Bonus: more readable diff. > If we start sharing more throttle-related material than just a struct, > we can reconsider. > > We could also move it to the existing file for common stuff: > qapi/common.json. Not a great fit, though. So, the final conclusion is to move to common.json? Regards, Pradeep >
On 08/10/2017 09:06 AM, Pradeep Jagadeesh wrote: >>> It's not "moving it back", it's keeping it where it is. But I see no big >>> problem with moving it to a common file either. >> >> I'd rather not put every struct shared across subsystem boundaries in >> its own file. >> >> We can keep it right where it is for now. Bonus: more readable diff. >> If we start sharing more throttle-related material than just a struct, >> we can reconsider. >> >> We could also move it to the existing file for common stuff: >> qapi/common.json. Not a great fit, though. > So, the final conclusion is to move to common.json? No. If more than one .json file would benefit by including the definition, then put it in a separate file that both .json include from. But if only one .json file would be including a new file, then just inline the struct directly into that one original file (in this case, block-core.json) instead of creating a separate file (so no to needing iothrottle.json), or putting the code in yet a different file than the one that is using the struct (so no to putting it in common.json).
Eric Blake <eblake@redhat.com> writes: > On 08/10/2017 09:06 AM, Pradeep Jagadeesh wrote: > >>>> It's not "moving it back", it's keeping it where it is. But I see no big >>>> problem with moving it to a common file either. >>> >>> I'd rather not put every struct shared across subsystem boundaries in >>> its own file. >>> >>> We can keep it right where it is for now. Bonus: more readable diff. >>> If we start sharing more throttle-related material than just a struct, >>> we can reconsider. >>> >>> We could also move it to the existing file for common stuff: >>> qapi/common.json. Not a great fit, though. >> >> So, the final conclusion is to move to common.json? > > No. > > If more than one .json file would benefit by including the definition, > then put it in a separate file that both .json include from. This is the case. Your opinion is incompatible with mine, stated above. > But if only one .json file would be including a new file, then just > inline the struct directly into that one original file (in this case, > block-core.json) instead of creating a separate file (so no to needing > iothrottle.json), or putting the code in yet a different file than the > one that is using the struct (so no to putting it in common.json). This is no longer the case. Conclusion: no consensus, yet.
Markus Armbruster <armbru@redhat.com> writes: > Eric Blake <eblake@redhat.com> writes: > >> On 08/10/2017 09:06 AM, Pradeep Jagadeesh wrote: >> >>>>> It's not "moving it back", it's keeping it where it is. But I see no big >>>>> problem with moving it to a common file either. >>>> >>>> I'd rather not put every struct shared across subsystem boundaries in >>>> its own file. >>>> >>>> We can keep it right where it is for now. Bonus: more readable diff. >>>> If we start sharing more throttle-related material than just a struct, >>>> we can reconsider. >>>> >>>> We could also move it to the existing file for common stuff: >>>> qapi/common.json. Not a great fit, though. >>> >>> So, the final conclusion is to move to common.json? >> >> No. >> >> If more than one .json file would benefit by including the definition, >> then put it in a separate file that both .json include from. > > This is the case. > > Your opinion is incompatible with mine, stated above. > >> But if only one .json file would be including a new file, then just >> inline the struct directly into that one original file (in this case, >> block-core.json) instead of creating a separate file (so no to needing >> iothrottle.json), or putting the code in yet a different file than the >> one that is using the struct (so no to putting it in common.json). > > This is no longer the case. > > Conclusion: no consensus, yet. All right, let's start over and try to resolve the impasse and/or misunderstanding. Type BlockIOThrottle lives in qapi/block-core.json, and is used by QMP command block_set_io_throttle. Since 1.1. Pradeep has a use case for throttling in fsdev. Instead of duplicating the relevant parts of BlockIOThrottle, qmp_block_set_io_throttle() and hmp_block_set_io_throttle(), he factors them out smartly, into * [PATCH 2] IOThrottle, base type of BlockIOThrottle * [PATCH 3] throttle_set_io_limits(), called by qmp_block_set_io_throttle() * [PATCH 4] hmp_initialize_io_throttle(), called by hmp_block_set_io_throttle() throttle_set_io_limits() goes into existing util/throttle.c, and hmp_initialize_io_throttle() goes into existing hmp.c. The question is where IOThrottle should go. Pradeep proposes to put it in new qapi/throttle.json. Certainly defensible, but I really don't like putting every little thing shared across subsystem boundaries into its own schema file. Let me step back and discuss why we split the QAPI schema into multiple files in the first place. For me, the one and only reason is MAINTAINERS. If the block folks should continue to maintain IOThrottle, then it should stay put in block-core.json. If somebody else should start maintaining it, it should move. We'd need a suitable entry in MAINTAINERS then. I don't see why maintenance should change, and therefore believe it should stay put. Eric?
On 08/16/2017 11:13 AM, Markus Armbruster wrote: > Markus Armbruster <armbru@redhat.com> writes: > >> >> Conclusion: no consensus, yet. > > All right, let's start over and try to resolve the impasse and/or > misunderstanding. > > Type BlockIOThrottle lives in qapi/block-core.json, and is used by QMP > command block_set_io_throttle. Since 1.1. > > Pradeep has a use case for throttling in fsdev. Instead of duplicating > the relevant parts of BlockIOThrottle, qmp_block_set_io_throttle() and > hmp_block_set_io_throttle(), he factors them out smartly, into > > * [PATCH 2] IOThrottle, base type of BlockIOThrottle > > * [PATCH 3] throttle_set_io_limits(), called by > qmp_block_set_io_throttle() > > * [PATCH 4] hmp_initialize_io_throttle(), called by > hmp_block_set_io_throttle() > > throttle_set_io_limits() goes into existing util/throttle.c, and > hmp_initialize_io_throttle() goes into existing hmp.c. The question is > where IOThrottle should go. Good summary. > > Pradeep proposes to put it in new qapi/throttle.json. Certainly > defensible, but I really don't like putting every little thing shared > across subsystem boundaries into its own schema file. I agree with the dislike of creating new files, if an existing file is adequate. > > Let me step back and discuss why we split the QAPI schema into multiple > files in the first place. For me, the one and only reason is > MAINTAINERS. Indeed, that's a good description of why splits would be appropriate. So the obvious next question is if this is a case that needs a new maintainer. > > If the block folks should continue to maintain IOThrottle, then it > should stay put in block-core.json. I think Manos' work on making throttling a filter driver at the block layer is proof enough that it it is still fine to keep throttling maintained in block-core.json. > > If somebody else should start maintaining it, it should move. We'd need > a suitable entry in MAINTAINERS then. > > I don't see why maintenance should change, and therefore believe it > should stay put. > > Eric? I think we're in violent agreement: don't create a new file, and having the new factored type live in block-core.json is the best fit because we haven't come up with any reasons why it needs to be split.
Eric Blake <eblake@redhat.com> writes: > On 08/16/2017 11:13 AM, Markus Armbruster wrote: >> Markus Armbruster <armbru@redhat.com> writes: >> > >>> >>> Conclusion: no consensus, yet. >> >> All right, let's start over and try to resolve the impasse and/or >> misunderstanding. >> >> Type BlockIOThrottle lives in qapi/block-core.json, and is used by QMP >> command block_set_io_throttle. Since 1.1. >> >> Pradeep has a use case for throttling in fsdev. Instead of duplicating >> the relevant parts of BlockIOThrottle, qmp_block_set_io_throttle() and >> hmp_block_set_io_throttle(), he factors them out smartly, into >> >> * [PATCH 2] IOThrottle, base type of BlockIOThrottle >> >> * [PATCH 3] throttle_set_io_limits(), called by >> qmp_block_set_io_throttle() >> >> * [PATCH 4] hmp_initialize_io_throttle(), called by >> hmp_block_set_io_throttle() >> >> throttle_set_io_limits() goes into existing util/throttle.c, and >> hmp_initialize_io_throttle() goes into existing hmp.c. The question is >> where IOThrottle should go. > > Good summary. > >> >> Pradeep proposes to put it in new qapi/throttle.json. Certainly >> defensible, but I really don't like putting every little thing shared >> across subsystem boundaries into its own schema file. > > I agree with the dislike of creating new files, if an existing file is > adequate. > >> >> Let me step back and discuss why we split the QAPI schema into multiple >> files in the first place. For me, the one and only reason is >> MAINTAINERS. > > Indeed, that's a good description of why splits would be appropriate. > So the obvious next question is if this is a case that needs a new > maintainer. > >> >> If the block folks should continue to maintain IOThrottle, then it >> should stay put in block-core.json. > > I think Manos' work on making throttling a filter driver at the block > layer is proof enough that it it is still fine to keep throttling > maintained in block-core.json. > >> >> If somebody else should start maintaining it, it should move. We'd need >> a suitable entry in MAINTAINERS then. >> >> I don't see why maintenance should change, and therefore believe it >> should stay put. >> >> Eric? > > I think we're in violent agreement: don't create a new file, and having > the new factored type live in block-core.json is the best fit because we > haven't come up with any reasons why it needs to be split. Thanks, Eric. Pradeep, please put IOThrottle right before BlocIOThrottle in block-core.json. Use it in fsdev.json without including block-core.json. Sorry for the delay.
On Aug 16, 2017 11:41 PM, "Markus Armbruster" <armbru@redhat.com> wrote: Eric Blake <eblake@redhat.com> writes: > On 08/16/2017 11:13 AM, Markus Armbruster wrote: >> Markus Armbruster <armbru@redhat.com> writes: >> > >>> >>> Conclusion: no consensus, yet. >> >> All right, let's start over and try to resolve the impasse and/or >> misunderstanding. >> >> Type BlockIOThrottle lives in qapi/block-core.json, and is used by QMP >> command block_set_io_throttle. Since 1.1. >> >> Pradeep has a use case for throttling in fsdev. Instead of duplicating >> the relevant parts of BlockIOThrottle, qmp_block_set_io_throttle() and >> hmp_block_set_io_throttle(), he factors them out smartly, into >> >> * [PATCH 2] IOThrottle, base type of BlockIOThrottle >> >> * [PATCH 3] throttle_set_io_limits(), called by >> qmp_block_set_io_throttle() >> >> * [PATCH 4] hmp_initialize_io_throttle(), called by >> hmp_block_set_io_throttle() >> >> throttle_set_io_limits() goes into existing util/throttle.c, and >> hmp_initialize_io_throttle() goes into existing hmp.c. The question is >> where IOThrottle should go. > > Good summary. > >> >> Pradeep proposes to put it in new qapi/throttle.json. Certainly >> defensible, but I really don't like putting every little thing shared >> across subsystem boundaries into its own schema file. > > I agree with the dislike of creating new files, if an existing file is > adequate. > >> >> Let me step back and discuss why we split the QAPI schema into multiple >> files in the first place. For me, the one and only reason is >> MAINTAINERS. > > Indeed, that's a good description of why splits would be appropriate. > So the obvious next question is if this is a case that needs a new > maintainer. > >> >> If the block folks should continue to maintain IOThrottle, then it >> should stay put in block-core.json. > > I think Manos' work on making throttling a filter driver at the block > layer is proof enough that it it is still fine to keep throttling > maintained in block-core.json. > >> >> If somebody else should start maintaining it, it should move. We'd need >> a suitable entry in MAINTAINERS then. >> >> I don't see why maintenance should change, and therefore believe it >> should stay put. >> >> Eric? > > I think we're in violent agreement: don't create a new file, and having > the new factored type live in block-core.json is the best fit because we > haven't come up with any reasons why it needs to be split. Thanks, Eric. Pradeep, please put IOThrottle right before BlocIOThrottle in block-core.json. Use it in fsdev.json without including block-core.json. Sorry for the delay. Thanks for all the clarifications, sure I will move iothrottle code to block-core.json. Thanks, Pradeep
diff --git a/qapi/block-core.json b/qapi/block-core.json index f85c223..9320974 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -6,6 +6,7 @@ # QAPI common definitions { 'include': 'common.json' } +{ 'include': 'iothrottle.json' } ## # @SnapshotInfo: @@ -1761,84 +1762,13 @@ # # @device: Block device name (deprecated, use @id instead) # -# @id: The name or QOM path of the guest device (since: 2.8) -# -# @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) -# -# @bps_rd_max: read throughput limit during bursts, -# in bytes (Since 1.7) -# -# @bps_wr_max: write throughput limit during bursts, -# in bytes (Since 1.7) -# -# @iops_max: total I/O operations per second during bursts, -# in bytes (Since 1.7) -# -# @iops_rd_max: read I/O operations per second during bursts, -# in bytes (Since 1.7) -# -# @iops_wr_max: write I/O operations per second during bursts, -# in bytes (Since 1.7) -# -# @bps_max_length: maximum length of the @bps_max burst -# period, in seconds. It must only -# be set if @bps_max is set as well. -# Defaults to 1. (Since 2.6) -# -# @bps_rd_max_length: maximum length of the @bps_rd_max -# burst period, in seconds. It must only -# be set if @bps_rd_max is set as well. -# Defaults to 1. (Since 2.6) -# -# @bps_wr_max_length: maximum length of the @bps_wr_max -# burst period, in seconds. It must only -# be set if @bps_wr_max is set as well. -# Defaults to 1. (Since 2.6) -# -# @iops_max_length: maximum length of the @iops burst -# period, in seconds. It must only -# be set if @iops_max is set as well. -# Defaults to 1. (Since 2.6) -# -# @iops_rd_max_length: maximum length of the @iops_rd_max -# burst period, in seconds. It must only -# be set if @iops_rd_max is set as well. -# Defaults to 1. (Since 2.6) -# -# @iops_wr_max_length: maximum length of the @iops_wr_max -# burst period, in seconds. It must only -# be set if @iops_wr_max is set as well. -# Defaults to 1. (Since 2.6) -# -# @iops_size: an I/O size in bytes (Since 1.7) -# # @group: throttle group name (Since 2.4) # # Since: 1.1 ## { 'struct': 'BlockIOThrottle', - 'data': { '*device': 'str', '*id': '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', '*group': 'str' } } + 'base': 'IOThrottle', + 'data': { '*device': 'str', '*group': 'str' } } ## # @block-stream: diff --git a/qapi/iothrottle.json b/qapi/iothrottle.json new file mode 100644 index 0000000..0f067c3 --- /dev/null +++ b/qapi/iothrottle.json @@ -0,0 +1,88 @@ +# -*- Mode: Python -*- + +## +# == QAPI IOThrottle definitions +## + +## +# @IOThrottle: +# +# A set of parameters describing IO throttling +# +# @id: The name or QOM path of the guest device (since: 2.8) +# +# @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) +# +# @bps_rd_max: read throughput limit during bursts, +# in bytes (Since 1.7) +# +# @bps_wr_max: write throughput limit during bursts, +# in bytes (Since 1.7) +# +# @iops_max: total I/O operations per second during bursts, +# in bytes (Since 1.7) +# +# @iops_rd_max: read I/O operations per second during bursts, +# in bytes (Since 1.7) +# +# @iops_wr_max: write I/O operations per second during bursts, +# in bytes (Since 1.7) +# +# @bps_max_length: maximum length of the @bps_max burst +# period, in seconds. It must only +# be set if @bps_max is set as well. +# Defaults to 1. (Since 2.6) +# +# @bps_rd_max_length: maximum length of the @bps_rd_max +# burst period, in seconds. It must only +# be set if @bps_rd_max is set as well. +# Defaults to 1. (Since 2.6) +# +# @bps_wr_max_length: maximum length of the @bps_wr_max +# burst period, in seconds. It must only +# be set if @bps_wr_max is set as well. +# Defaults to 1. (Since 2.6) +# +# @iops_max_length: maximum length of the @iops burst +# period, in seconds. It must only +# be set if @iops_max is set as well. +# Defaults to 1. (Since 2.6) +# +# @iops_rd_max_length: maximum length of the @iops_rd_max +# burst period, in seconds. It must only +# be set if @iops_rd_max is set as well. +# Defaults to 1. (Since 2.6) +# +# @iops_wr_max_length: maximum length of the @iops_wr_max +# burst period, in seconds. It must only +# be set if @iops_wr_max is set as well. +# Defaults to 1. (Since 2.6) +# +# @iops_size: an I/O size in bytes (Since 1.7) +# +# +# Since: 2.10 +## +{ 'struct': 'IOThrottle', + 'data': { '*id': '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' } }