Re: [PATCH] xen/console: Handle true dom0less case when switching serial input

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.03.2023 08:51, Michal Orzel wrote:
> On 16/03/2023 08:24, Jan Beulich wrote:
>> On 16.03.2023 02:49, Stefano Stabellini wrote:
>>> On Wed, 15 Mar 2023, Michal Orzel wrote:
>>>> On 15/03/2023 14:11, Jan Beulich wrote:
>>>>> On 15.03.2023 13:34, Michal Orzel wrote:
>>>>>> On 14/03/2023 16:17, Jan Beulich wrote:
>>>>>>> On 14.03.2023 15:27, Michal Orzel wrote:
>>>>>>>> --- a/xen/drivers/char/console.c
>>>>>>>> +++ b/xen/drivers/char/console.c
>>>>>>>> @@ -491,6 +491,14 @@ static void switch_serial_input(void)
>>>>>>>>      else
>>>>>>>>      {
>>>>>>>>          console_rx++;
>>>>>>>> +
>>>>>>>> +        /*
>>>>>>>> +         * Skip switching serial input to hardware domain if it does 
>>>>>>>> not exist
>>>>>>>> +         * (i.e. true dom0less mode).
>>>>>>>> +         */
>>>>>>>> +        if ( !hardware_domain && (console_rx == 1) )
>>>>>>>> +            console_rx++;
>>>>>>>
>>>>>>> The consumers of this variable aren't really serialized with this
>>>>>>> updating. That's probably okay-ish prior to your change, but now
>>>>>>> there can be two updates in rapid succession. I think it would be
>>>>>>> better if the variable was written only once here.
>>>>>> ok, makes sense.
>>>>>>
>>>>>>>
>>>>>>>>          printk("*** Serial input to DOM%d", console_rx - 1);
>>>>>>>
>>>>>>> When invoked from console_endboot() this will now switch to Dom1,
>>>>>>> i.e. that domain becomes kind of "preferred", which I think is
>>>>>>> wrong. Instead I think in such a case we should direct input to
>>>>>>> Xen by default.
>>>>>> Switching serial input to the first usable domain is the major 
>>>>>> motivation behind this patch.
>>>>>> The number of times I got pinged by users with *apparent* Xen issue on 
>>>>>> true dom0less
>>>>>> just because input was directed to dom0 which was not there (not 
>>>>>> everyone seems to read the
>>>>>> boot logs) forced me to create this patch and manifests that this is not 
>>>>>> the behavior user wants.
>>>>>> Switching to Xen console would not help at all. Also, we already have a 
>>>>>> way to set switch code to 'x'
>>>>>> to default serial input to Xen.
>>>>>> So I think what I did (switching to the first existing domain) should be 
>>>>>> the default behavior (just like it was done for dom0).
>>>>>
>>>>> Well, I'm not going to stand in the way, but if one of several supposedly
>>>>> equal domains is to be "preferred" in some way, then I for one would
>>>>> expect justification for doing so. If that's the route to go, then the
>>>>> patch snippet you provided looks good to me.
>>>> Well, the explanation is that we are directing input to the first existing 
>>>> domain
>>>> which also is the first domain created (this is what users expect at least 
>>>> by default).
>>>> This for now creates simplest/cleanest solution that matches the current 
>>>> behavior
>>>> with dom0 and solves the users inconveniences I mentioned earlier.
>>>> There is no other real preference apart from being first domain created 
>>>> and to help keep the order simple.
>>>
>>> My two cents. Given the feedback we are getting from users, I think it
>>> is important that the input goes to a real valid domain (not Xen, not
>>> Dom0 that doesn't exist). "Which dom0less domain?" is a valid question,
>>> and I don't know what would be the best answer. But any of those domains
>>> would be better than what we have today.
>>
>> Could boot time configuration perhaps indicate which domain to "prefer"?
>> Otherwise I'm pretty inclined to suggest to make it actually random ...
> Random is not great because in a system with e.g. 20 domUs and directing to 
> e.g. 10th domU
> user would have to go through a lot of domains executing CTRL-AAA several 
> times. Also, for me
> it would be difficult to reason about as such approach is definitely not 
> something users expect.
> 
> May I ask so that we proceed with a patch I proposed to start from the first 
> usable domain
> to match the current behavior and to help users?. In the meantime I will 
> start working on
> a support for indicating which domain to prefer.

So long as the description states clearly that the choice is arbitrary, I
wouldn't (further) object to such a change.

> All in all, the new patch would touch console_endboot() and not 
> switch_serial_input().

Of course.

Jan