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

[Juergen Gross <jgross@xxxxxxxx>]

On 21.03.23 16:02, Anthony PERARD wrote:
> 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.
Okay.

> > +
> >   /* 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?
I can switch the type to unsigned int.

> > +                                        .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.
I'll make console_mfn "unsigned long" just like in the build state.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature