Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 02/26/2018 06:02 PM, Andrew Cooper wrote:
> On 05/02/18 13:14, George Dunlap wrote:
>> On 02/05/2018 12:56 PM, Jan Beulich wrote:
>>>>>> On 05.02.18 at 12:55, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 02/02/18 08:51, Jan Beulich wrote:
>>>>>>>> On 01.02.18 at 15:38, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>> --- a/docs/misc/xen-command-line.markdown
>>>>>> +++ b/docs/misc/xen-command-line.markdown
>>>>>> @@ -916,6 +916,19 @@ Controls EPT related features.
>>>>>>  
>>>>>>  Specify which console gdbstub should use. See **console**.
>>>>>>  
>>>>>> +### gnttab
>>>>>> +> `= List of [ max_ver:<integer>, transitive=<bool> ]`
>>>>> I realize you don't want to change this as people already use it, but
>>>>> I'd still like to give my usual comment: I'd prefer if we could avoid
>>>>> introducing further underscore-containing (sub)options. I really don't
>>>>> understand why everyone does this: Dashes are easier to type on
>>>>> all keyboards I'm aware of, and there's no need to mimic C identifier
>>>>> names for command line options.
>>>> I can introduce a max-ver alias if you insist, but dropping max_ver here
>>>> is going to break users who took this patch for XSA-226.
>>> Hence the way I've worded my reply - I don't mean to insist on
>>> changing what you have, or the introduction of an alias. I merely
>>> wanted to give the comment, in the hope that it helps to avoid
>>> future underscores in command line option names.
>> FWIW I often end up looking at other options and name things similarly;
>> so making the documentation say "max-ver", but accepting both "max-ver"
>> and "max_ver", would probably make it more likely that future options
>> would start out as having a dash rather than an underscore.
>>
>> But it's just a suggestion; I wouldn't push for it.
> 
> So how to unblock this?  There are no concrete suggestions, and no
> concrete objections to the patch in its current form.

From what I understand, both Jan and I are saying, "We won't block the
patch if you re-submit with max_ver but Jan's other comments addressed;
but we would prefer it if max-ver could be used instead."

Given that you want to keep things compatible with the securty patch, I
see two options forward for you:

1. Re-submit it using only max_ver

2. Make it accept both max-ver and max_ver, but only document  max-ver.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel