> 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 <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).
>
@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
