Re: [PATCH] tools: use libxenlight for writing xenstore-stubdom console nodes

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Mar 15, 2023 at 04:42:26PM +0100, Juergen Gross wrote:
> @@ -1982,6 +1987,13 @@ int libxl_console_get_tty(libxl_ctx *ctx, uint32_t 
> domid, int cons_num,
>   */
>  int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char 
> **path);
>  
> +/* libxl_console_add_xenstore writes the Xenstore entries for a domain's
> + * primary console based on domid, event channel port and the guest frame
> + * number of the PV console's ring page.
> + */
> +int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t 
> backend,
> +                               int evtch, uint64_t gfn);

Could you make this function async, by adding "const libxl_asyncop_how
*ao_how" parameter in last position?

You can follow libxl_domain_pause() example has to write an async
function that is actually synchronous (AO_CREATE, libxl__ao_complete,
AO_INPROGRESS, AO_CREATE_FAIL).

Adding the async capability now means that we won't need to change the
API if the function actually need to be async one day.

> +
>  /* May be called with info_r == NULL to check for domain's existence.
>   * Returns ERROR_DOMAIN_NOTFOUND if domain does not exist (used to return
>   * ERROR_INVAL for this scenario). */
> diff --git a/tools/libs/light/libxl_console.c 
> b/tools/libs/light/libxl_console.c
> index d8b2bc5465..ce3de100cc 100644
> --- a/tools/libs/light/libxl_console.c
> +++ b/tools/libs/light/libxl_console.c
> @@ -346,6 +346,26 @@ out:
>      return rc;
>  }
>  
> +int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t 
> backend,
> +                               int evtch, uint64_t gfn)
> +{
> +    GC_INIT(ctx);
> +    int rc;
> +    libxl__device_console console = { .backend_domid = backend,
> +                                      .output = "pty",
> +                                      .consback = 
> LIBXL__CONSOLE_BACKEND_XENCONSOLED,
> +                                    };
> +    libxl__domain_build_state state = { .console_port = evtch,

`console_port` is "uint32_t", shouldn't `evtchn` be the same type, or at
least also be unsigned?

> +                                        .console_mfn = gfn,

I wonder if we should check if `gfn` fit in `console_mfn`, as it could
be smaller when building the toolstack on 32bit platform.


Thanks,

-- 
Anthony PERARD