Re: [Qemu-devel] [PATCH v3 3/3] blkdebug: Add latency injection rule type

[Markus Armbruster]

Marc Olson via Qemu-devel <address@hidden> writes:

> On 2/13/19 7:48 AM, Max Reitz wrote:
>> On 12.02.19 22:21, Marc Olson wrote:
>>> On 1/11/19 7:00 AM, Max Reitz wrote:
>>>> On 12.11.18 08:06, Marc Olson wrote:
>> [...]
>>
>>>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>>>> index d4fe710..72f7861 100644
>>>>> --- a/qapi/block-core.json
>>>>> +++ b/qapi/block-core.json
>>>>> @@ -3057,6 +3057,34 @@
>>>>>                '*immediately': 'bool' } }
>>>>>      ##
>>>>> +# @BlkdebugDelayOptions:
>>>>> +#
>>>>> +# Describes a single latency injection for blkdebug.
>>>>> +#
>>>>> +# @event:       trigger event
>>>>> +#
>>>>> +# @state:       the state identifier blkdebug needs to be in to
>>>>> +#               actually trigger the event; defaults to "any"
>>>>> +#
>>>>> +# @latency:     The delay to add to an I/O, in microseconds.
>>>>> +#
>>>>> +# @sector:      specifies the sector index which has to be affected
>>>>> +#               in order to actually trigger the event; defaults to
>>>>> "any
>>>>> +#               sector"
>>>>> +#
>>>>> +# @once:        disables further events after this one has been
>>>>> +#               triggered; defaults to false
>>>>> +#
>>>>> +# Since: 3.1
>>>> Well, 4.0 now, sorry...
>>> Baking version numbers into code is downright silly. Every single
>>> version of this patch has made a comment along the lines of, "oops, it
>>> didn't get reviewed in time for the next version bump, so you have to
>>> resubmit." With a fast moving project, this is nonsense. If you're
>>> looking at the code, you should have access to the git history as well
>>> to determine the version.
>> True, but these comments are used to generate documentation (e.g.
>> docs/interopt/qemu-qmp-ref.7 in the build directory).  So they are used
>> by people who don't have access to the git history.

Exactly.

>> It might be possible to generate that information from git-blame when
>> generating the documentation, but how would trivial fixes be handled
>> that are no functional changes?  For instance, it seems difficult to me
>> to distinguish between a spelling change for some parameter description
>> and a change in behavior.
>>
>> (Then again, we shouldn't have such behavioral changes, hm.)

Gathering "since" information could be partially automated.  Syntactical
changes are visible in introspection.  Keep a persistent map syntactical
element -> first version, and a program to update it for new stuff in
introspection data since the last release.  Commit the map on release.

Automation would be partial, because we can't rule out the need for
documenting a "since" for a change that isn't syntactical.

While there, have the program report any changes that aren't additions.
These are potential compatibility breakers, and should be reviewed.

>> To me personally, the easiest thing would seem to be some convention to
>> write ***INSERT VERSION HERE*** into the code and then the maintainer
>> can just find an replace all instances of that when applying the
>> patches.  But that sounds a bit silly...

Maintainers are our bottleneck.  Shifting work from submitter to
maintainer makes the bottleneck narrower.

>> (I don't have an issue with adjusting the version numbers myself as they
>> are, but it's just as hard for me to find and replace all of them as it
>> is for you.  And since you're probably going to send a v4 anyway...)
>>
>> In the end, it's up to the QAPI maintainers.
>
> IFF I submit by the end of the week, what version number shall I choose?

IFF you want to be taken seriously, cut the hyperbole.

You raised a valid point.  Don't sabotage yourself.

[...]