Re: [PATCH v3 2/2] xl: Add escape character argument to xl console

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Jun 22, 2023 at 03:12:48PM +0100, Peter Hoyes wrote:
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index cac641a7eb..c513c39483 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -1958,7 +1958,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,
> +                       char* escape_character);

So, this is changing the libxl API, it's possible, but it's going to be
a bit more complicated than that.

You'll need to:

- Increment LIBXL_API_VERSION, well more like allow a new api version.
  That's adding 0x041800 to the list of allowed value for
  LIBXL_API_VERSION, at around line 793 in "libxl.h", just before
  "#error Unknown LIBXL_API_VERSION".

- Introduce API compatible caller for earlier API version.
  A good example of this done would be
  libxl_retrieve_domain_configuration_0x041200().
  Actually, there's already some changed been made in the past to
  libxl_console_exec() and libxl_primary_console_exec(), so those are
  probably better example and need to modified.

- Introduce LIBXL_HAVE_* macro in libxl.h to advertise the new arg.
  There's already LIBXL_HAVE_CONSOLE_NOTIFY_FD, so I guess the new macro
  could be named LIBXL_HAVE_CONSOLE_ESCAPE_CHARACTER.


>  /* libxl_primary_console_exec finds the domid and console number
>   * corresponding to the primary console of the given vm, then calls
>   * libxl_console_exec with the right arguments (domid might be different
> @@ -1968,9 +1969,12 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, 
> int cons_num,
>   * guests using pygrub.
>   * If notify_fd is not -1, xenconsole will write 0x00 to it to nofity
>   * the caller that it has connected to the guest console.
> + * If escape_character is not NULL, the provided value is used to exit
> + * the guest console.
>   */
>  int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm,
> -                               int notify_fd);
> +                               int notify_fd,
> +                               char* escape_character);
>  
>  #if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x040800
>  
> diff --git a/tools/libs/light/libxl_console.c 
> b/tools/libs/light/libxl_console.c
> index f497be141b..0b7293fe71 100644
> --- a/tools/libs/light/libxl_console.c
> +++ b/tools/libs/light/libxl_console.c
> @@ -75,15 +76,26 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, 
> int cons_num,
>          goto out;
>      }
>  
> +    char *args[] = {
> +        p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
> +        NULL, NULL, NULL, NULL, // start-notify-fd, escape
> +        NULL, // list terminator - do not use
> +    };
> +    char **args_extra = args + 6;
> +
>      if (notify_fd != -1) {
>          notify_fd_s = GCSPRINTF("%d", notify_fd);
> -        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
> -              "--start-notify-fd", notify_fd_s, (void *)NULL);
> -    } else {
> -        execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s,
> -              (void *)NULL);
> +        *args_extra++ = "--start-notify-fd";
> +        *args_extra++ = notify_fd_s;
>      }
>  
> +    if (escape_character) {
> +        *args_extra++ = "--escape";
> +        *args_extra++ = escape_character;
> +    }

There is flexarray_* that could be use, but I guess a preset `*args`
kind of work here.

> +
> +    execv(p, args);
> +
>  out:
>      GC_FREE;
>      return ERROR_FAIL;
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index ccf4d83584..67604e9536 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -141,7 +141,8 @@ const struct cmd_spec cmd_table[] = {
>        "Attach to domain's console",
>        "[options] <Domain>\n"
>        "-t <type>       console type, pv , serial or vuart\n"
> -      "-n <number>     console number"
> +      "-n <number>     console number\n"
> +      "-e <escape>     escape character"

Could you also update "docs/man/xl.1.pod.in" with this new option?

Thanks,

-- 
Anthony PERARD