Re: [PATCH v3 02/10] xen/version: Introduce non-truncating deterministically-signed XENVER_* subops

[George Dunlap <george.dunlap@xxxxxxxxx>]

On Wed, Aug 16, 2023 at 3:58 AM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:

On 16/08/2023 1:19 am, Stefano Stabellini wrote:
> On Tue, 15 Aug 2023, Andrew Cooper wrote:

>> diff --git a/xen/include/public/version.h b/xen/include/public/version.h
>> index cbc4ef7a46e6..0dd6bbcb43cc 100644
>> --- a/xen/include/public/version.h
>> +++ b/xen/include/public/version.h
>> @@ -19,12 +19,20 @@
>>  /* arg == NULL; returns major:minor (16:16). */
>>  #define XENVER_version      0
>> 
>> -/* arg == xen_extraversion_t. */
>> +/*
>> + * arg == xen_extraversion_t.
>> + *
>> + * This API/ABI is broken.  Use XENVER_extraversion2 where possible.
> Like Jan and Julien I also don't like the word "broken" especially
> without explanation of why it is broken next to it.

The word "broken" is an accurate and appropriate word in the English
language to describe the situation.  I'm sorry you don't like it, but
unless any of you can articulate why you think it is inappropriate
phraseology, the complaint is unactionable.

Specifically ...

> Instead, I would say:
>
> "XENVER_extraversion is deprecated. Please use XENVER_extraversion2."

... depreciated is misleading.

Deprecated means, "It is the official position of the developers of the project that for the moment, you *can* use it, but you *shouldn't*"; it also implies that support for it might go away at some point.  The fact that we're saying "you shouldn't use it" by itself implies that it is lacking somehow.  It's a factual statement that gives you useful information.

Neither "broken" nor "has problems" actually tell you anything above "deprecated", other than the opinion of the developer writing the documentation; and they are both (to different levels) emotionally charged.  You don't deprecate things unless there's something wrong with them.  In this particular case, I don't think anyone has an emotional attachment to the existing hypercall, so nobody is going to be insulted; but it's a good habit to avoid it.  (See [1] for more about this.)

[1] http://xenbits.xenproject.org/governance/communication-practice.html , the "Avoid inflammatory and negatively charged language" section

If I have a backlog of a million things to do, how do I prioritize switching to and testing extraversion2?  The situation as I understand it is: "If it works for you now with the current value you're fine, but it may break later when you change it, in a way that's obvious".  In that situation, you can safely put off fixing it until your build breaks.  If, on the other hand, the situation is "It may randomly work or not work with any given build", or "It may seem to work for you now but is actually failing in a not-very-obvious way", then you probably need to prioritize fixing it.

Saying "May cause truncation" gives you some the information you need to know. "Will silently truncate past N characters" gives you even more.  

 

We should at very least say it's deprecated.  If we can summarize the issues briefly, then that would be helpful.

 -George