On 11/28/18 3:25 AM, Markus Armbruster wrote:
xiezhide <xiezh...@huawei.com> writes:
Rename the ThrottleLimits member names and modify related code
Signed-off-by: xiezhide <xiezh...@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).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
