Re: [PATCH] chardev/char-win-stdio: Support VT sequences on Windows 11 host

[Marc-André Lureau]

Hi

On Fri, Jun 16, 2023 at 7:41 AM Zhang Huasen <huasenzhang@foxmail.com> wrote:

From: Huasen Zhang <huasenzhang@foxmail.com>

Hello,

On Thu, 15 Jun 2023 12:57:55 +0200 Marc-André Lureau <marcandre.lureau@redhat.com>
wrote:
> Hi
>
> On Thu, Jun 15, 2023 at 12:36 PM Zhang Huasen <huasenzhang@foxmail.com>
> wrote:
>
> > If the monitor or the serial port use STDIO as backend on Windows 11 host,
> > e.g. -nographic options is used, the monitor or the guest Linux do not
> > response to arrow keys.
> >
> > When Windows creates a console, ENABLE_VIRTUAL_PROCESS_INPUT is disabled
> > by default. Arrow keys cannot be retrieved by ReadFile or ReadConsoleInput
> > functions.
> >
> > Add ENABLE_VIRTUAL_PROCESS_INPUT to the flag which is passed to
> > SetConsoleMode,
> > when opening stdio console.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1674
> >
> > Signed-off-by: Zhang Huasen <huasenzhang@foxmail.com>
> > ---
> >  chardev/char-win-stdio.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/chardev/char-win-stdio.c b/chardev/char-win-stdio.c
> > index eb830eabd9..1a18999e78 100644
> > --- a/chardev/char-win-stdio.c
> > +++ b/chardev/char-win-stdio.c
> > @@ -190,7 +190,7 @@ static void qemu_chr_open_stdio(Chardev *chr,
> >          }
> >      }
> >
> > -    dwMode |= ENABLE_LINE_INPUT;
> > +    dwMode |= ENABLE_LINE_INPUT | ENABLE_VIRTUAL_TERMINAL_INPUT;
> >
>
> I think we should set it only when is_console (although that may not make a
> difference otherwise)

It is okay to set ENABLE_VIRTUAL_TERMINAL_INPUT only when is_console is TRUE.

I do not understand some points of original code.
If the stdin is not a console, i.e. GetConsoleMode fails, we still
call SetConsoleMode and set ENABLE_LINE_INPUT.
Could you please tell what the purpose is?

I have no clue. It has been this way since the beginning (commit db418a0a7).

The code doesn't check SetConsoleMode() return value, so I'd just go with your patch for now.