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

[Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

---- On Wed, 16 Aug 2023 05:55:07 -0400 George Dunlap  wrote ---

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

With all due respect George but deprecated and broken are fundamentally 
different. Deprecation means that an interface is being retired in the future 
for any number of reasons and that it can continue to be used until its 
retirement without risk of unintended consequences. Labeling an interface as 
broken, which has been acceptable to do so in other much larger communities, is 
a stronger sentiment that the interface should stop being used immediately 
because it can lead to unintended consequences, not because it will be retired 
in some distant future.

 > [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.

In general I would concur here, except that these are externally consumed files 
for which the consuming projects just need to be told don't use this interface. 
The why is not so important to them as it is to the Xen community. But for the 
latter, any one working on Xen code can do a git blame on the comment line to 
find the originating commit and read the commit message for the details as to 
why.

v/r,
dps