Re: [Xen-devel] [PATCH] xl console: Add interactive option

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jul 03, 2017 at 08:34:22AM +0200, Felix Schmoll wrote:
> Added an interactive option to the 'xl console'-command that
> forwards the input stream of the console to the underlying pty.
> 
> Made corresponding changes to libxl, xl and xenconsole.
> 
> Signed-off-by: Felix Schmoll <eggi.innovations@xxxxxxxxx>
> ---
>  tools/console/client/main.c |  4 ++++
>  tools/libxl/libxl.h         |  5 +++--
>  tools/libxl/libxl_console.c | 19 ++++++++++++++-----
>  tools/xl/xl_console.c       | 11 +++++++----
>  tools/xl/xl_vmcontrol.c     |  2 +-
>  5 files changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/console/client/main.c b/tools/console/client/main.c
> index 977779f034..a3c4b00835 100644
> --- a/tools/console/client/main.c
> +++ b/tools/console/client/main.c
> @@ -334,6 +334,7 @@ int main(int argc, char **argv)
>               { "num",     1, 0, 'n' },
>               { "help",    0, 0, 'h' },
>               { "start-notify-fd", 1, 0, 's' },
> +             { "pipe", 0, 0, 'p' },
>               { 0 },
>  
>       };
> @@ -370,6 +371,9 @@ int main(int argc, char **argv)
>               case 's':
>                       start_notify_fd = atoi(optarg);
>                       break;
> +        case 'p':
> +            interactive = 1;
> +            break;

Indentation is wrong.

And I believe some more changes are needed. Currently interactive also
changes the terminal attributes via tcsetattr. Since now STDIN isn't
really a tty, we need to skip that.

I think you need to introduce a new flag, not reusing interactive.

>               default:
>                       fprintf(stderr, "Invalid argument\n");
>                       fprintf(stderr, "Try `%s --help' for more 
> information.\n", 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index cf8687aa7e..a55f9b1cc7 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -1499,7 +1499,8 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t 
> domid, int autopass);
>   * the caller that it has connected to the guest console.
>   */
>  int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num,
> -                       libxl_console_type type, int notify_fd);
> +                       libxl_console_type type, int notify_fd,
> +                       int interactive);

If we're to change libxl API, we need to provide compatibility
implementation for older version of this API. See the surrounding code.

And I think using "pipe" is better.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel