[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 29.04.20 14:49, Igor Druzhinin wrote:
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 
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).
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.

Nobody wants to remove XS_INTRODUCE. It was just questioned there is a
need to call XS_INTRODUCE multiple times for a domain without a call of
XS_RELEASE in between.

In case there is such a need this means we either should just drop the
test for the mfn to match and recreate the event-channel (which will do
no harm IMO), or we need to keep the mfn in the live-update state record
with the knowledge that it is far from being a good indicator for the
test whether a domain is known already or not (two HVM domains using a
similar configuration with the very same kernel will use the same gfn

As only dom0 is allowed to use XS_INTRODUCE and I'm not aware of any
problems with neither XS_INTRODUCE failing due to illegal calls (no mfn
match), nor with potentially recreating the event channel for a "wrong"
domain, I suggest to just allow XS_INTRODUCE to be called as often as
the toolstack wants to.

In the end that was the primary reason to write this patch: to remove
the need to have the mfn in the live-update state record.





Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.