Re: [Xen-devel] [PATCH 1/3] xenstore: setup xenstore stubdom console interface properly

[Jürgen Groß <jgross@xxxxxxxx>]

On 11.02.20 21:18, Andrew Cooper wrote:
> On 28/01/2020 14:28, Juergen Gross wrote:
> > In order to be able to get access to the console of Xenstore stubdom
> > we need an appropriate granttab entry. So call xc_dom_gnttab_init()
> > when constructing the domain and preset some information needed
> > for that function in the dom structure.
> >
> > We need to create the event channel for the console, too. Do that and
> > store all necessary data locally.
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   tools/helpers/init-xenstore-domain.c | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> >
> > diff --git a/tools/helpers/init-xenstore-domain.c 
> > b/tools/helpers/init-xenstore-domain.c
> > index adb8408b63..a312bc38b8 100644
> > --- a/tools/helpers/init-xenstore-domain.c
> > +++ b/tools/helpers/init-xenstore-domain.c
> > @@ -24,6 +24,8 @@ static char *param;
> >   static char *name = "Xenstore";
> >   static int memory;
> >   static int maxmem;
> > +static xen_pfn_t console_mfn;
> > +static unsigned int console_evtchn;
> >   
> >   static struct option options[] = {
> >       { "kernel", 1, NULL, 'k' },
> > @@ -113,6 +115,7 @@ static int build(xc_interface *xch)
> >           fprintf(stderr, "xc_domain_setmaxmem failed\n");
> >           goto err;
> >       }
> > +    console_evtchn = xc_evtchn_alloc_unbound(xch, domid, 0);
> Presumably some form of error checking?
Yes.

> Also, while it is probably fine for now, I think this does highlight a
> future issue.  What happens when xenconsoled is also a stubdomain?
>
> I suspect we have a looming chicken&egg problem, where the toolstack is
> going to have to do some careful domid and plumbing to set up build both
> stubdoms in tandem.
Hmm, I'd rather defer console setup in xenstore-stubdom then and do it
later via a XS_CONTROL message. This will even enable a restart of
console-stubdom.

> >       rv = xc_domain_set_memmap_limit(xch, domid, limit_kb);
> >       if ( rv )
> >       {
> > @@ -133,6 +136,9 @@ static int build(xc_interface *xch)
> >           snprintf(cmdline, 512, "--event %d --internal-db", rv);
> >   
> >       dom = xc_dom_allocate(xch, cmdline, NULL);
> Any chance of some error handling, unlikely as it is to go wrong?
Okay.

> > +    dom->container_type = XC_DOM_PV_CONTAINER;
> > +    dom->xenstore_domid = domid;
> > +    dom->console_evtchn = console_evtchn;
> and a newline here?
Okay.

> >       rv = xc_dom_kernel_file(dom, kernel);
> >       if ( rv )
> >       {
> > @@ -186,6 +192,12 @@ static int build(xc_interface *xch)
> >           fprintf(stderr, "xc_dom_boot_image failed\n");
> >           goto err;
> >       }
> > +    rv = xc_dom_gnttab_init(dom);
> > +    if ( rv )
> > +    {
> > +        fprintf(stderr, "xc_dom_gnttab_init failed\n");
> > +        goto err;
> > +    }
> >   
> >       rv = xc_domain_set_virq_handler(xch, domid, VIRQ_DOM_EXC);
> >       if ( rv )
> > @@ -201,6 +213,7 @@ static int build(xc_interface *xch)
> >       }
> >   
> >       rv = 0;
> > +    console_mfn = xc_dom_p2m(dom, dom->console_pfn);
> This doesn't appear to be used.
Oh, the usage is in patch 2. Probably I should move this addition to
that patch then.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel