Re: [PATCH] tools/xenstore: don't store domU's mfn of ring page in xensotred

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 29/04/2020 06:51, Jürgen Groß wrote:
> On 28.04.20 22:55, Julien Grall wrote:
> > Hi Juergen,
> >
> > On Tue, 28 Apr 2020 at 16:53, Juergen Gross <jgross@xxxxxxxx> wrote:
> > > The XS_INTRODUCE command has two parameters: the mfn (or better: gfn)
> > > of the domain's xenstore ring page and the event channel of the
> > > domain for communicating with Xenstore.
> > >
> > > The gfn is not really needed. It is stored in the per-domain struct
> > > in xenstored and in case of another XS_INTRODUCE for the domain it
> > > is tested to match the original value. If it doesn't match the
> > > command is aborted via EINVAL.
> > >
> > > Today there shouldn't be multiple XS_INTRODUCE requests for the same
> > > domain issued, so the mfn/gfn can just be ignored and multiple
> > > XS_INTRODUCE commands can be rejected without testing the mfn/gfn.
> > So there is a comment in the else part:
> >
> > /* Use XS_INTRODUCE for recreating the xenbus event-channel. */
> >
> >  From the commit message this is not entirely clear why we want to
> > prevent recreating the event-channel. Can you expand it?
> Recreating the event channel would be fine, but I don't see why it
> would ever be needed. And XS_INTRODUCE is called only at domain creation
> time today, so there is obviously no need for repeating this call.
>
> Maybe the idea was to do this after sending a XS_RESUME command, which
> isn't used anywhere in Xen and is another part of Xenstore which doesn't
> make any sense today.
Commit f6cc37ea8ac71385b60507c034519f304da75f4c "tools/oxenstored: port 
XS_INTRODUCE evtchn rebind function from cxenstored" added the exact 
same behavior in the OCaml XenStored last year.
This was introduced 12 years after C XenStored, so surely someone think 
this is useful. We should check why this was introduced in OCaml 
XenStored (I have CCed the author of the patch).
If we still think this is not useful, then you should add an explanation 
in the commit message why the two implementations diverge and possibly 
update the spec.
Cheers,

--
Julien Grall