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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
> Sent: 29 April 2020 13:50
> To: Jürgen Groß <jgross@xxxxxxxx>; Julien Grall <julien@xxxxxxx>; Julien Grall
> <julien.grall.oss@xxxxxxxxx>
> Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Ian Jackson 
> <ian.jackson@xxxxxxxxxxxxx>; Wei Liu
> <wl@xxxxxxx>; andrew.cooper3@xxxxxxxxxx; Paul Durrant <paul@xxxxxxx>
> Subject: Re: [PATCH] tools/xenstore: don't store domU's mfn of ring page in 
> xensotred
> 
> On 29/04/2020 13:29, Jürgen Groß wrote:
> > On 29.04.20 13:04, Igor Druzhinin wrote:
> >> On 29/04/2020 11:49, Jürgen Groß wrote:
> >>> On 29.04.20 12:39, Igor Druzhinin wrote:
> >>>> On 29/04/2020 10:22, Julien Grall wrote:
> >>>>> Hi Juergen,
> >>>>>
> >>>>> On 29/04/2020 06:51, Jürgen Groß wrote:
> >>>>>>
> >>>>>> 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.
> >>>>
> >>>> Thanks for CC, Julien.
> >>>>
> >>>> We indeed already use this functionality in our toolstack for guest kdump
> >>>> functions. It's not possible in XAPI to adopt libxl model where almost 
> >>>> everything
> >>>> is restarted for a domain entering kdump, so we have to use this message 
> >>>> to
> >>>> rebind xenstore evtchn after soft reset without shutting down backends 
> >>>> and
> >>>> recreating the whole subtree (frontends reconnect fine after that).
> >>>>
> >>>> We obviously only require it for now to be present in oxenstored only.
> >>>> Please don't remove this functionality if possible.
> >>>
> >>> If I read handling in libxl correctly, in the soft reset case XS_RELEASE
> >>> is issued before doing another XS_INTRODUCE. XS_RELEASE will result in
> >>> xenstored throwing away its related struct domain, so XS_INTRODUCE will
> >>> be possible again.
> >>
> >>  From what I remember it was not possible to keep xenstored data for a 
> >> domain
> >> and at the same time perform release-introduce cycle (at least in 
> >> oxenstored).
> >> It also involved firing @releaseDomain which caused havoc in other part of
> >> the toolstack.
> >
> > Wei, Ian, can you please tell me where I'm wrong?
> >
> > A soft reset should restart the domain in a clean state. AFAIK libxl is
> > handling that by doing kind of in-place save-restore, including calling
> > xs_release_domain() and later xs_introduce_domain(). This should result
> > in xenstored throwing away all state it has regarding the domain and
> > then restarting with a new (internal) domain instance.
> >
> > Is XAPI doing soft reset differently? Why should there be a need for
> > keeping xenstored data across a soft reset?
> 
> Yes, XAPI is doing soft reset differently from libxl. You need to keep 
> xenstore
> data to at least keep backends working without the need to reinitialize them
> before entering kdump kernel. We do the same thing while entering crash kernel
> in Windows after BSOD (CC Paul as he recommended this approach).

IIRC I recommended not involving Xen or the toolstack in entering the crash 
kernel... they don't need to know. Windows quite happily enters its crash 
kernel, rebuilds its Xen interfaces and re-attaches to PV backends without any 
external intervention.

  Paul

> There are other reasons to not reset xenstore data.
> 
> I considered XS_INTRODUCE functionality to be part of xenstored ABI and we 
> built
> a lot of infrastructure on top of that. So I don't think it could be easily 
> removed now.
> 
> Igor