| Message ID | a4bc041444fd76d5da1d8e5321007f416df4ab84.1542384802.git.xiezhide@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fsdev-throttle-qmp: qmp interface for fsdev io throttling | expand |
xiezhide <xiezhide@huawei.com> writes: > Rename the ThrottleLimits member names and modify related code > > Signed-off-by: xiezhide <xiezhide@huawei.com> > --- > qapi/block-core.json | 70 +++++++++++----------- > util/throttle.c | 163 +++++++++++++++++++++++++-------------------------- > 2 files changed, 116 insertions(+), 117 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index d4fe710..4ffaaea 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2240,45 +2240,45 @@ > # transaction. All fields are optional. When setting limits, if a field is > # missing the current value is not changed. > # > -# @iops-total: limit total I/O operations per second > -# @iops-total-max: I/O operations burst > -# @iops-total-max-length: length of the iops-total-max burst period, in seconds > -# It must only be set if @iops-total-max is set as well. > -# @iops-read: limit read operations per second > -# @iops-read-max: I/O operations read burst > -# @iops-read-max-length: length of the iops-read-max burst period, in seconds > -# It must only be set if @iops-read-max is set as well. > -# @iops-write: limit write operations per second > -# @iops-write-max: I/O operations write burst > -# @iops-write-max-length: length of the iops-write-max burst period, in seconds > -# It must only be set if @iops-write-max is set as well. > -# @bps-total: limit total bytes per second > -# @bps-total-max: total bytes burst > -# @bps-total-max-length: length of the bps-total-max burst period, in seconds. > -# It must only be set if @bps-total-max is set as well. > -# @bps-read: limit read bytes per second > -# @bps-read-max: total bytes read burst > -# @bps-read-max-length: length of the bps-read-max burst period, in seconds > -# It must only be set if @bps-read-max is set as well. > -# @bps-write: limit write bytes per second > -# @bps-write-max: total bytes write burst > -# @bps-write-max-length: length of the bps-write-max burst period, in seconds > -# It must only be set if @bps-write-max is set as well. > -# @iops-size: when limiting by iops max size of an I/O in bytes > +# @iops: limit total I/O operations per second > +# @iops_max: I/O operations burst > +# @iops_max_length: length of the iops_total_max burst period, in seconds > +# It must only be set if @iops_total_max is set as well. > +# @iops_rd: limit read operations per second > +# @iops_rd_max: I/O operations read burst > +# @iops_rd_max_length: length of the iops_read_max burst period, in seconds > +# It must only be set if @iops_read_max is set as well. > +# @iops_wr: limit write operations per second > +# @iops_wr_max: I/O operations write burst > +# @iops_wr_max_length: length of the iops_write_max burst period, in seconds > +# It must only be set if @iops_write_max is set as well. > +# @bps: limit total bytes per second > +# @bps_max: total bytes burst > +# @bps_max_length: length of the bps_total_max burst period, in seconds. > +# It must only be set if @bps_total_max is set as well. > +# @bps_rd: limit read bytes per second > +# @bps_rd_max: total bytes read burst > +# @bps_rd_max_length: length of the bps_read_max burst period, in seconds > +# It must only be set if @bps_read_max is set as well. > +# @bps_wr: limit write bytes per second > +# @bps_wr_max: total bytes write burst > +# @bps_wr_max_length: length of the bps_write_max burst period, in seconds > +# It must only be set if @bps_write_max is set as well. > +# @iops_size: when limiting by iops max size of an I/O in bytes > # > # Since: 2.11 > ## > { 'struct': 'ThrottleLimits', > - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int', > - '*iops-total-max-length' : 'int', '*iops-read' : 'int', > - '*iops-read-max' : 'int', '*iops-read-max-length' : 'int', > - '*iops-write' : 'int', '*iops-write-max' : 'int', > - '*iops-write-max-length' : 'int', '*bps-total' : 'int', > - '*bps-total-max' : 'int', '*bps-total-max-length' : 'int', > - '*bps-read' : 'int', '*bps-read-max' : 'int', > - '*bps-read-max-length' : 'int', '*bps-write' : 'int', > - '*bps-write-max' : 'int', '*bps-write-max-length' : 'int', > - '*iops-size' : 'int' } } > + 'data': { '*iops' : 'int', '*iops_max' : 'int', > + '*iops_max_length' : 'int', '*iops_rd' : 'int', > + '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int', > + '*iops_wr' : 'int', '*iops_wr_max' : 'int', > + '*iops_wr_max_length' : 'int', '*bps' : 'int', > + '*bps_max' : 'int', '*bps_max_length' : 'int', > + '*bps_rd' : 'int', '*bps_rd_max' : 'int', > + '*bps_rd_max_length' : 'int', '*bps_wr' : 'int', > + '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int', > + '*iops_size' : 'int' } } Compatibility break. Why is that okay? Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt: Command names, and member names within a type, should be all lower case with words separated by a hyphen. However, some existing older commands and complex types use underscore; when extending such expressions, consistency is preferred over blindly avoiding underscore. The exception doesn't apply here. > > ## > # @block-stream: [...]
On 11/28/18 3:25 AM, Markus Armbruster wrote: > xiezhide <xiezhide@huawei.com> writes: > >> Rename the ThrottleLimits member names and modify related code >> >> Signed-off-by: xiezhide <xiezhide@huawei.com> >> --- >> qapi/block-core.json | 70 +++++++++++----------- >> util/throttle.c | 163 +++++++++++++++++++++++++-------------------------- >> 2 files changed, 116 insertions(+), 117 deletions(-) >> >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index d4fe710..4ffaaea 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> ## >> { 'struct': 'ThrottleLimits', >> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int', >> + 'data': { '*iops' : 'int', '*iops_max' : 'int', >> + '*iops_max_length' : 'int', '*iops_rd' : 'int', >> + '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int', >> + '*iops_wr' : 'int', '*iops_wr_max' : 'int', >> + '*iops_wr_max_length' : 'int', '*bps' : 'int', >> + '*bps_max' : 'int', '*bps_max_length' : 'int', >> + '*bps_rd' : 'int', '*bps_rd_max' : 'int', >> + '*bps_rd_max_length' : 'int', '*bps_wr' : 'int', >> + '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int', >> + '*iops_size' : 'int' } } > > Compatibility break. Why is that okay? Grepping qapi/qapi-introspection.c shows 0 hits for either ThrottleLimits or for iops-total, so there are no QMP commands affected. There might, however, be command line and/or QOM paths affected, which is harder to audit since those don't affect instrospection. > > Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt: > > Command names, and member names within a type, should be all lower > case with words separated by a hyphen. However, some existing older > commands and complex types use underscore; when extending such > expressions, consistency is preferred over blindly avoiding > underscore. > > The exception doesn't apply here. Ah, but it does, because we are refactoring code to share a common QAPI struct in a later patch, where we need this exact naming to avoid breaking that command. So the REAL problem with this commit is that the commit message does not give enough details, either why this is safe (because it does not impact existing QMP commands) or needed (because we will be using it to rewrite an existing QMP command that needs this spelling).
> Subject: Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the > ThrottleLimits member names > > xiezhide <xiezhide@huawei.com> writes: > > > Rename the ThrottleLimits member names and modify related code > > > > Signed-off-by: xiezhide <xiezhide@huawei.com> > > --- > > qapi/block-core.json | 70 +++++++++++----------- > > util/throttle.c | 163 > +++++++++++++++++++++++++-------------------------- > > 2 files changed, 116 insertions(+), 117 deletions(-) > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json index > > d4fe710..4ffaaea 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -2240,45 +2240,45 @@ > > # transaction. All fields are optional. When setting limits, if a > > field is # missing the current value is not changed. > > # > > -# @iops-total: limit total I/O operations per second > > -# @iops-total-max: I/O operations burst > > -# @iops-total-max-length: length of the iops-total-max burst period, in > seconds > > -# It must only be set if @iops-total-max is set > as well. > > -# @iops-read: limit read operations per second > > -# @iops-read-max: I/O operations read burst > > -# @iops-read-max-length: length of the iops-read-max burst period, in > seconds > > -# It must only be set if @iops-read-max is set > as well. > > -# @iops-write: limit write operations per second > > -# @iops-write-max: I/O operations write burst > > -# @iops-write-max-length: length of the iops-write-max burst period, in > seconds > > -# It must only be set if @iops-write-max is > set as well. > > -# @bps-total: limit total bytes per second > > -# @bps-total-max: total bytes burst > > -# @bps-total-max-length: length of the bps-total-max burst period, in > seconds. > > -# It must only be set if @bps-total-max is set > as well. > > -# @bps-read: limit read bytes per second > > -# @bps-read-max: total bytes read burst > > -# @bps-read-max-length: length of the bps-read-max burst period, in > seconds > > -# It must only be set if @bps-read-max is set > as well. > > -# @bps-write: limit write bytes per second > > -# @bps-write-max: total bytes write burst > > -# @bps-write-max-length: length of the bps-write-max burst period, in > seconds > > -# It must only be set if @bps-write-max is set > as well. > > -# @iops-size: when limiting by iops max size of an I/O in > bytes > > +# @iops: limit total I/O operations per second > > +# @iops_max: I/O operations burst > > +# @iops_max_length: length of the iops_total_max burst period, in > seconds > > +# It must only be set if @iops_total_max is > set as well. > > +# @iops_rd: limit read operations per second > > +# @iops_rd_max: I/O operations read burst > > +# @iops_rd_max_length: length of the iops_read_max burst period, in > seconds > > +# It must only be set if @iops_read_max is > set as well. > > +# @iops_wr: limit write operations per second > > +# @iops_wr_max: I/O operations write burst > > +# @iops_wr_max_length: length of the iops_write_max burst period, in > seconds > > +# It must only be set if @iops_write_max is > set as well. > > +# @bps: limit total bytes per second > > +# @bps_max: total bytes burst > > +# @bps_max_length: length of the bps_total_max burst period, in > seconds. > > +# It must only be set if @bps_total_max is > set as well. > > +# @bps_rd: limit read bytes per second > > +# @bps_rd_max: total bytes read burst > > +# @bps_rd_max_length: length of the bps_read_max burst period, in > seconds > > +# It must only be set if @bps_read_max is > set as well. > > +# @bps_wr: limit write bytes per second > > +# @bps_wr_max: total bytes write burst > > +# @bps_wr_max_length: length of the bps_write_max burst period, in > seconds > > +# It must only be set if @bps_write_max is > set as well. > > +# @iops_size: when limiting by iops max size of an I/O in > bytes > > # > > # Since: 2.11 > > ## > > { 'struct': 'ThrottleLimits', > > - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int', > > - '*iops-total-max-length' : 'int', '*iops-read' : 'int', > > - '*iops-read-max' : 'int', '*iops-read-max-length' : 'int', > > - '*iops-write' : 'int', '*iops-write-max' : 'int', > > - '*iops-write-max-length' : 'int', '*bps-total' : 'int', > > - '*bps-total-max' : 'int', '*bps-total-max-length' : 'int', > > - '*bps-read' : 'int', '*bps-read-max' : 'int', > > - '*bps-read-max-length' : 'int', '*bps-write' : 'int', > > - '*bps-write-max' : 'int', '*bps-write-max-length' : 'int', > > - '*iops-size' : 'int' } } > > + 'data': { '*iops' : 'int', '*iops_max' : 'int', > > + '*iops_max_length' : 'int', '*iops_rd' : 'int', > > + '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int', > > + '*iops_wr' : 'int', '*iops_wr_max' : 'int', > > + '*iops_wr_max_length' : 'int', '*bps' : 'int', > > + '*bps_max' : 'int', '*bps_max_length' : 'int', > > + '*bps_rd' : 'int', '*bps_rd_max' : 'int', > > + '*bps_rd_max_length' : 'int', '*bps_wr' : 'int', > > + '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int', > > + '*iops_size' : 'int' } } > > Compatibility break. Why is that okay? > > Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt: > > Command names, and member names within a type, should be all lower > case with words separated by a hyphen. However, some existing older > commands and complex types use underscore; when extending such > expressions, consistency is preferred over blindly avoiding > underscore. > > The exception doesn't apply here. Why did this changing: 1. ThrottleLimits is just use for block device, this change not break compatibility 2. can share same base struct in json file 3. can reuse same code between block and fsdev device for io throttling 4. can keep the same qmp command style for block and fsdev with less codes Block: { "execute": "block_set_io_throttle", "arguments": { "device": "virtio0", "iops": 100, "iops_rd": 0, "iops_wr": 0, "bps": 0, "bps_rd": 0, "bps_wr": 0 } } Fsdev: { "execute": "fsdev-set-io-throttle", "arguments": { "id": "extra-9p-kataTest" "iops": 100, "iops_rd": 0, "iops_wr": 0, "bps": 0, "bps_rd": 0, "bps_wr": 0 } } Thanks xiezhide > > > > > ## > > # @block-stream: > [...]
> Subject: Re: [Qemu-devel] [PATCH v5 2/6] fsdev-throttle-qmp: Rename the > ThrottleLimits member names > > On 11/28/18 3:25 AM, Markus Armbruster wrote: > > xiezhide <xiezhide@huawei.com> writes: > > > >> Rename the ThrottleLimits member names and modify related code > >> > >> Signed-off-by: xiezhide <xiezhide@huawei.com> > >> --- > >> qapi/block-core.json | 70 +++++++++++----------- > >> util/throttle.c | 163 > +++++++++++++++++++++++++-------------------------- > >> 2 files changed, 116 insertions(+), 117 deletions(-) > >> > >> diff --git a/qapi/block-core.json b/qapi/block-core.json index > >> d4fe710..4ffaaea 100644 > >> --- a/qapi/block-core.json > >> +++ b/qapi/block-core.json > > >> ## > >> { 'struct': 'ThrottleLimits', > >> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int', > > >> + 'data': { '*iops' : 'int', '*iops_max' : 'int', > >> + '*iops_max_length' : 'int', '*iops_rd' : 'int', > >> + '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int', > >> + '*iops_wr' : 'int', '*iops_wr_max' : 'int', > >> + '*iops_wr_max_length' : 'int', '*bps' : 'int', > >> + '*bps_max' : 'int', '*bps_max_length' : 'int', > >> + '*bps_rd' : 'int', '*bps_rd_max' : 'int', > >> + '*bps_rd_max_length' : 'int', '*bps_wr' : 'int', > >> + '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int', > >> + '*iops_size' : 'int' } } > > > > Compatibility break. Why is that okay? > > Grepping qapi/qapi-introspection.c shows 0 hits for either ThrottleLimits or for > iops-total, so there are no QMP commands affected. > There might, however, be command line and/or QOM paths affected, which is > harder to audit since those don't affect instrospection. > > > > > Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt: > > > > Command names, and member names within a type, should be all > lower > > case with words separated by a hyphen. However, some existing > older > > commands and complex types use underscore; when extending such > > expressions, consistency is preferred over blindly avoiding > > underscore. > > > > The exception doesn't apply here. > > Ah, but it does, because we are refactoring code to share a common QAPI > struct in a later patch, where we need this exact naming to avoid breaking that > command. > > So the REAL problem with this commit is that the commit message does not > give enough details, either why this is safe (because it does not impact existing > QMP commands) or needed (because we will be using it to rewrite an existing > QMP command that needs this spelling). > @Erick, thanks for your simple but exact explaining for purpose of this patch Thanks xiezhide > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org
Eric Blake <eblake@redhat.com> writes: > On 11/28/18 3:25 AM, Markus Armbruster wrote: >> xiezhide <xiezhide@huawei.com> writes: >> >>> Rename the ThrottleLimits member names and modify related code >>> >>> Signed-off-by: xiezhide <xiezhide@huawei.com> >>> --- >>> qapi/block-core.json | 70 +++++++++++----------- >>> util/throttle.c | 163 +++++++++++++++++++++++++-------------------------- >>> 2 files changed, 116 insertions(+), 117 deletions(-) >>> >>> diff --git a/qapi/block-core.json b/qapi/block-core.json >>> index d4fe710..4ffaaea 100644 >>> --- a/qapi/block-core.json >>> +++ b/qapi/block-core.json > >>> ## >>> { 'struct': 'ThrottleLimits', >>> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int', > >>> + 'data': { '*iops' : 'int', '*iops_max' : 'int', >>> + '*iops_max_length' : 'int', '*iops_rd' : 'int', >>> + '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int', >>> + '*iops_wr' : 'int', '*iops_wr_max' : 'int', >>> + '*iops_wr_max_length' : 'int', '*bps' : 'int', >>> + '*bps_max' : 'int', '*bps_max_length' : 'int', >>> + '*bps_rd' : 'int', '*bps_rd_max' : 'int', >>> + '*bps_rd_max_length' : 'int', '*bps_wr' : 'int', >>> + '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int', >>> + '*iops_size' : 'int' } } >> >> Compatibility break. Why is that okay? > > Grepping qapi/qapi-introspection.c shows 0 hits for either > ThrottleLimits or for iops-total, so there are no QMP commands > affected. I see. > There might, however, be command line and/or QOM paths > affected, which is harder to audit since those don't affect > instrospection. Yet another argument for QAPIfication... Let's check. git-grep ThrottleLimits finds: * block/throttle-groups.c: getter and setter for QOM class "throttle-group" property "limits". This class is user-creatable. Therefore, this is an externally visible interface. The question is whether it's a *stable* interface. The status of QOM properties in that regard is not clear to me. If I remember correctly, we treated it as "just for debugging and such" at least initially. But is that still a defensible position? It ceases to be one as soon as you need QOM properties to do something users ought to be able to do. Like configuring stuff that is meant to be configured. If that's the case, we need to examine the situation, and clarify our ABI promises. Recommend a separate thread. * util/throttle.c: Functions to convert to and from ThrottleConfig. The conversion to ThrottleConfig uses hardcoded member names in error messages. Relevant only insofar we need to remember to change them when we rename members. >> Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt: >> >> Command names, and member names within a type, should be all lower >> case with words separated by a hyphen. However, some existing older >> commands and complex types use underscore; when extending such >> expressions, consistency is preferred over blindly avoiding >> underscore. >> >> The exception doesn't apply here. > > Ah, but it does, because we are refactoring code to share a common > QAPI struct in a later patch, where we need this exact naming to avoid > breaking that command. > > So the REAL problem with this commit is that the commit message does > not give enough details, either why this is safe (because it does not > impact existing QMP commands) or needed (because we will be using it > to rewrite an existing QMP command that needs this spelling). Yes, the commit message needs to make explain why the change is useful, and why it is safe.
> > Eric Blake <eblake@redhat.com> writes: > > > On 11/28/18 3:25 AM, Markus Armbruster wrote: > >> xiezhide <xiezhide@huawei.com> writes: > >> > >>> Rename the ThrottleLimits member names and modify related code > >>> > >>> Signed-off-by: xiezhide <xiezhide@huawei.com> > >>> --- > >>> qapi/block-core.json | 70 +++++++++++----------- > >>> util/throttle.c | 163 > +++++++++++++++++++++++++-------------------------- > >>> 2 files changed, 116 insertions(+), 117 deletions(-) > >>> > >>> diff --git a/qapi/block-core.json b/qapi/block-core.json index > >>> d4fe710..4ffaaea 100644 > >>> --- a/qapi/block-core.json > >>> +++ b/qapi/block-core.json > > > >>> ## > >>> { 'struct': 'ThrottleLimits', > >>> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int', > > > >>> + 'data': { '*iops' : 'int', '*iops_max' : 'int', > >>> + '*iops_max_length' : 'int', '*iops_rd' : 'int', > >>> + '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int', > >>> + '*iops_wr' : 'int', '*iops_wr_max' : 'int', > >>> + '*iops_wr_max_length' : 'int', '*bps' : 'int', > >>> + '*bps_max' : 'int', '*bps_max_length' : 'int', > >>> + '*bps_rd' : 'int', '*bps_rd_max' : 'int', > >>> + '*bps_rd_max_length' : 'int', '*bps_wr' : 'int', > >>> + '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int', > >>> + '*iops_size' : 'int' } } > >> > >> Compatibility break. Why is that okay? > > > > Grepping qapi/qapi-introspection.c shows 0 hits for either > > ThrottleLimits or for iops-total, so there are no QMP commands > > affected. > > I see. > > > There might, however, be command line and/or QOM paths > > affected, which is harder to audit since those don't affect > > instrospection. > > Yet another argument for QAPIfication... > > Let's check. git-grep ThrottleLimits finds: > > * block/throttle-groups.c: getter and setter for QOM class > "throttle-group" property "limits". This class is user-creatable. > Therefore, this is an externally visible interface. > > The question is whether it's a *stable* interface. The status of QOM > properties in that regard is not clear to me. If I remember > correctly, we treated it as "just for debugging and such" at least > initially. But is that still a defensible position? It ceases to be > one as soon as you need QOM properties to do something users ought to > be able to do. Like configuring stuff that is meant to be configured. > If that's the case, we need to examine the situation, and clarify our > ABI promises. Recommend a separate thread. > > * util/throttle.c: Functions to convert to and from ThrottleConfig. The > conversion to ThrottleConfig uses hardcoded member names in error > messages. Relevant only insofar we need to remember to change them > when we rename members. Yes, find few hard coded member names in error message need to change. Will fix it in new version > >> Even if it is, you still run afoul of docs/devel/qapi-code-gen.txt: > >> > >> Command names, and member names within a type, should be all > lower > >> case with words separated by a hyphen. However, some existing > older > >> commands and complex types use underscore; when extending such > >> expressions, consistency is preferred over blindly avoiding > >> underscore. > >> > >> The exception doesn't apply here. > > > > Ah, but it does, because we are refactoring code to share a common > > QAPI struct in a later patch, where we need this exact naming to avoid > > breaking that command. > > > > So the REAL problem with this commit is that the commit message does > > not give enough details, either why this is safe (because it does not > > impact existing QMP commands) or needed (because we will be using it > > to rewrite an existing QMP command that needs this spelling). > > Yes, the commit message needs to make explain why the change is useful, and > why it is safe. Strongly endorse , will with clear and detail commit message next Thanks xiezhide
diff --git a/qapi/block-core.json b/qapi/block-core.json index d4fe710..4ffaaea 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2240,45 +2240,45 @@ # transaction. All fields are optional. When setting limits, if a field is # missing the current value is not changed. # -# @iops-total: limit total I/O operations per second -# @iops-total-max: I/O operations burst -# @iops-total-max-length: length of the iops-total-max burst period, in seconds -# It must only be set if @iops-total-max is set as well. -# @iops-read: limit read operations per second -# @iops-read-max: I/O operations read burst -# @iops-read-max-length: length of the iops-read-max burst period, in seconds -# It must only be set if @iops-read-max is set as well. -# @iops-write: limit write operations per second -# @iops-write-max: I/O operations write burst -# @iops-write-max-length: length of the iops-write-max burst period, in seconds -# It must only be set if @iops-write-max is set as well. -# @bps-total: limit total bytes per second -# @bps-total-max: total bytes burst -# @bps-total-max-length: length of the bps-total-max burst period, in seconds. -# It must only be set if @bps-total-max is set as well. -# @bps-read: limit read bytes per second -# @bps-read-max: total bytes read burst -# @bps-read-max-length: length of the bps-read-max burst period, in seconds -# It must only be set if @bps-read-max is set as well. -# @bps-write: limit write bytes per second -# @bps-write-max: total bytes write burst -# @bps-write-max-length: length of the bps-write-max burst period, in seconds -# It must only be set if @bps-write-max is set as well. -# @iops-size: when limiting by iops max size of an I/O in bytes +# @iops: limit total I/O operations per second +# @iops_max: I/O operations burst +# @iops_max_length: length of the iops_total_max burst period, in seconds +# It must only be set if @iops_total_max is set as well. +# @iops_rd: limit read operations per second +# @iops_rd_max: I/O operations read burst +# @iops_rd_max_length: length of the iops_read_max burst period, in seconds +# It must only be set if @iops_read_max is set as well. +# @iops_wr: limit write operations per second +# @iops_wr_max: I/O operations write burst +# @iops_wr_max_length: length of the iops_write_max burst period, in seconds +# It must only be set if @iops_write_max is set as well. +# @bps: limit total bytes per second +# @bps_max: total bytes burst +# @bps_max_length: length of the bps_total_max burst period, in seconds. +# It must only be set if @bps_total_max is set as well. +# @bps_rd: limit read bytes per second +# @bps_rd_max: total bytes read burst +# @bps_rd_max_length: length of the bps_read_max burst period, in seconds +# It must only be set if @bps_read_max is set as well. +# @bps_wr: limit write bytes per second +# @bps_wr_max: total bytes write burst +# @bps_wr_max_length: length of the bps_write_max burst period, in seconds +# It must only be set if @bps_write_max is set as well. +# @iops_size: when limiting by iops max size of an I/O in bytes # # Since: 2.11 ## { 'struct': 'ThrottleLimits', - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int', - '*iops-total-max-length' : 'int', '*iops-read' : 'int', - '*iops-read-max' : 'int', '*iops-read-max-length' : 'int', - '*iops-write' : 'int', '*iops-write-max' : 'int', - '*iops-write-max-length' : 'int', '*bps-total' : 'int', - '*bps-total-max' : 'int', '*bps-total-max-length' : 'int', - '*bps-read' : 'int', '*bps-read-max' : 'int', - '*bps-read-max-length' : 'int', '*bps-write' : 'int', - '*bps-write-max' : 'int', '*bps-write-max-length' : 'int', - '*iops-size' : 'int' } } + 'data': { '*iops' : 'int', '*iops_max' : 'int', + '*iops_max_length' : 'int', '*iops_rd' : 'int', + '*iops_rd_max' : 'int', '*iops_rd_max_length' : 'int', + '*iops_wr' : 'int', '*iops_wr_max' : 'int', + '*iops_wr_max_length' : 'int', '*bps' : 'int', + '*bps_max' : 'int', '*bps_max_length' : 'int', + '*bps_rd' : 'int', '*bps_rd_max' : 'int', + '*bps_rd_max_length' : 'int', '*bps_wr' : 'int', + '*bps_wr_max' : 'int', '*bps_wr_max_length' : 'int', + '*iops_size' : 'int' } } ## # @block-stream: diff --git a/util/throttle.c b/util/throttle.c index e7db2ad..b421e33 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -496,98 +496,97 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size) void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg, Error **errp) { - if (arg->has_bps_total) { - cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps_total; + if (arg->has_bps) { + cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps; } - if (arg->has_bps_read) { - cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_read; + if (arg->has_bps_rd) { + cfg->buckets[THROTTLE_BPS_READ].avg = arg->bps_rd; } - if (arg->has_bps_write) { - cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_write; + if (arg->has_bps_wr) { + cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr; } - if (arg->has_iops_total) { - cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops_total; + if (arg->has_iops) { + cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops; } - if (arg->has_iops_read) { - cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_read; + if (arg->has_iops_rd) { + cfg->buckets[THROTTLE_OPS_READ].avg = arg->iops_rd; } - if (arg->has_iops_write) { - cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_write; + if (arg->has_iops_wr) { + cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr; } - if (arg->has_bps_total_max) { - cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_total_max; + if (arg->has_bps_max) { + cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max; } - if (arg->has_bps_read_max) { - cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_read_max; + if (arg->has_bps_rd_max) { + cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max; } - if (arg->has_bps_write_max) { - cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_write_max; + if (arg->has_bps_wr_max) { + cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max; } - if (arg->has_iops_total_max) { - cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_total_max; + if (arg->has_iops_max) { + cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max; } - if (arg->has_iops_read_max) { - cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_read_max; + if (arg->has_iops_rd_max) { + cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max; } - if (arg->has_iops_write_max) { - cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_write_max; + if (arg->has_iops_wr_max) { + cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max; } - if (arg->has_bps_total_max_length) { - if (arg->bps_total_max_length > UINT_MAX) { + if (arg->has_bps_max_length) { + if (arg->bps_max_length > UINT_MAX) { error_setg(errp, "bps-total-max-length value must be in" " the range [0, %u]", UINT_MAX); return; } - cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_total_max_length; + cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length; } - if (arg->has_bps_read_max_length) { - if (arg->bps_read_max_length > UINT_MAX) { + if (arg->has_bps_rd_max_length) { + if (arg->bps_rd_max_length > UINT_MAX) { error_setg(errp, "bps-read-max-length value must be in" " the range [0, %u]", UINT_MAX); return; } - cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_read_max_length; + cfg->buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length; } - if (arg->has_bps_write_max_length) { - if (arg->bps_write_max_length > UINT_MAX) { + if (arg->has_bps_wr_max_length) { + if (arg->bps_wr_max_length > UINT_MAX) { error_setg(errp, "bps-write-max-length value must be in" " the range [0, %u]", UINT_MAX); return; } - cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_write_max_length; + cfg->buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length; } - if (arg->has_iops_total_max_length) { - if (arg->iops_total_max_length > UINT_MAX) { + if (arg->has_iops_max_length) { + if (arg->iops_max_length > UINT_MAX) { error_setg(errp, "iops-total-max-length value must be in" " the range [0, %u]", UINT_MAX); return; } - cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_total_max_length; + cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length; } - if (arg->has_iops_read_max_length) { - if (arg->iops_read_max_length > UINT_MAX) { + if (arg->has_iops_rd_max_length) { + if (arg->iops_rd_max_length > UINT_MAX) { error_setg(errp, "iops-read-max-length value must be in" " the range [0, %u]", UINT_MAX); return; } - cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_read_max_length; + cfg->buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length; } - if (arg->has_iops_write_max_length) { - if (arg->iops_write_max_length > UINT_MAX) { + if (arg->has_iops_wr_max_length) { + if (arg->iops_wr_max_length > UINT_MAX) { error_setg(errp, "iops-write-max-length value must be in" " the range [0, %u]", UINT_MAX); return; } - cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_write_max_length; + cfg->buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length; } if (arg->has_iops_size) { cfg->op_size = arg->iops_size; } - throttle_is_valid(cfg, errp); } @@ -598,45 +597,45 @@ void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg, */ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var) { - var->bps_total = cfg->buckets[THROTTLE_BPS_TOTAL].avg; - var->bps_read = cfg->buckets[THROTTLE_BPS_READ].avg; - var->bps_write = cfg->buckets[THROTTLE_BPS_WRITE].avg; - var->iops_total = cfg->buckets[THROTTLE_OPS_TOTAL].avg; - var->iops_read = cfg->buckets[THROTTLE_OPS_READ].avg; - var->iops_write = cfg->buckets[THROTTLE_OPS_WRITE].avg; - var->bps_total_max = cfg->buckets[THROTTLE_BPS_TOTAL].max; - var->bps_read_max = cfg->buckets[THROTTLE_BPS_READ].max; - var->bps_write_max = cfg->buckets[THROTTLE_BPS_WRITE].max; - var->iops_total_max = cfg->buckets[THROTTLE_OPS_TOTAL].max; - var->iops_read_max = cfg->buckets[THROTTLE_OPS_READ].max; - var->iops_write_max = cfg->buckets[THROTTLE_OPS_WRITE].max; - var->bps_total_max_length = cfg->buckets[THROTTLE_BPS_TOTAL].burst_length; - var->bps_read_max_length = cfg->buckets[THROTTLE_BPS_READ].burst_length; - var->bps_write_max_length = cfg->buckets[THROTTLE_BPS_WRITE].burst_length; - var->iops_total_max_length = cfg->buckets[THROTTLE_OPS_TOTAL].burst_length; - var->iops_read_max_length = cfg->buckets[THROTTLE_OPS_READ].burst_length; - var->iops_write_max_length = cfg->buckets[THROTTLE_OPS_WRITE].burst_length; - var->iops_size = cfg->op_size; - - var->has_bps_total = true; - var->has_bps_read = true; - var->has_bps_write = true; - var->has_iops_total = true; - var->has_iops_read = true; - var->has_iops_write = true; - var->has_bps_total_max = true; - var->has_bps_read_max = true; - var->has_bps_write_max = true; - var->has_iops_total_max = true; - var->has_iops_read_max = true; - var->has_iops_write_max = true; - var->has_bps_read_max_length = true; - var->has_bps_total_max_length = true; - var->has_bps_write_max_length = true; - var->has_iops_total_max_length = true; - var->has_iops_read_max_length = true; - var->has_iops_write_max_length = true; - var->has_iops_size = true; + var->bps = cfg->buckets[THROTTLE_BPS_TOTAL].avg; + var->bps_rd = cfg->buckets[THROTTLE_BPS_READ].avg; + var->bps_wr = cfg->buckets[THROTTLE_BPS_WRITE].avg; + var->iops = cfg->buckets[THROTTLE_OPS_TOTAL].avg; + var->iops_rd = cfg->buckets[THROTTLE_OPS_READ].avg; + var->iops_wr = cfg->buckets[THROTTLE_OPS_WRITE].avg; + var->bps_max = cfg->buckets[THROTTLE_BPS_TOTAL].max; + var->bps_rd_max = cfg->buckets[THROTTLE_BPS_READ].max; + var->bps_wr_max = cfg->buckets[THROTTLE_BPS_WRITE].max; + var->iops_max = cfg->buckets[THROTTLE_OPS_TOTAL].max; + var->iops_rd_max = cfg->buckets[THROTTLE_OPS_READ].max; + var->iops_wr_max = cfg->buckets[THROTTLE_OPS_WRITE].max; + var->bps_max_length = cfg->buckets[THROTTLE_BPS_TOTAL].burst_length; + var->bps_rd_max_length = cfg->buckets[THROTTLE_BPS_READ].burst_length; + var->bps_wr_max_length = cfg->buckets[THROTTLE_BPS_WRITE].burst_length; + var->iops_max_length = cfg->buckets[THROTTLE_OPS_TOTAL].burst_length; + var->iops_rd_max_length = cfg->buckets[THROTTLE_OPS_READ].burst_length; + var->iops_wr_max_length = cfg->buckets[THROTTLE_OPS_WRITE].burst_length; + var->iops_size = cfg->op_size; + + var->has_bps = true; + var->has_bps_rd = true; + var->has_bps_wr = true; + var->has_iops = true; + var->has_iops_rd = true; + var->has_iops_wr = true; + var->has_bps_max = true; + var->has_bps_rd_max = true; + var->has_bps_wr_max = true; + var->has_iops_max = true; + var->has_iops_rd_max = true; + var->has_iops_wr_max = true; + var->has_bps_rd_max_length = true; + var->has_bps_max_length = true; + var->has_bps_wr_max_length = true; + var->has_iops_max_length = true; + var->has_iops_rd_max_length = true; + var->has_iops_wr_max_length = true; + var->has_iops_size = true; } /* parse the throttle options
Rename the ThrottleLimits member names and modify related code Signed-off-by: xiezhide <xiezhide@huawei.com> --- qapi/block-core.json | 70 +++++++++++----------- util/throttle.c | 163 +++++++++++++++++++++++++-------------------------- 2 files changed, 116 insertions(+), 117 deletions(-)