Re: [Xen-devel] [PATCH] docs/sphinx: todo/wishlist

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/07/2019 16:30, Juergen Gross wrote:
> On 23.07.19 15:38, Andrew Cooper wrote:
>> On 23/07/2019 05:36, Juergen Gross wrote:
>>> On 22.07.19 21:20, Andrew Cooper wrote:
>>>> diff --git a/docs/misc/wishlist.rst b/docs/misc/wishlist.rst
>>>> new file mode 100644
>>>> index 0000000000..6cdb47d6e7
>>>> --- /dev/null
>>>> +++ b/docs/misc/wishlist.rst
>>>> @@ -0,0 +1,53 @@
>>>> +Development Wishlist
>>>> +====================
>>>> +
>>>> +Remove xenstored's dependencies on unstable interfaces
>>>> +------------------------------------------------------
>>>> +
>>>> +Various xenstored implementations use libxc for two purposes.  It
>>>> would be a
>>>> +substantial advantage to move xenstored onto entirely stable
>>>> interfaces, which
>>>> +disconnects it from the internal of the libxc.
>>>> +
>>>> +1. Foreign mapping of the store ring
>>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> +
>>>> +This is obsolete since :xen-cs:`6a2de353a9` (2012) which allocated
>>>> grant
>>>> +entries instead, to allow xenstored to function as a stub-domain
>>>> without dom0
>>>> +permissions.  :xen-cs:`38eeb3864d` dropped foreign mapping for
>>>> cxenstored.
>>>> +However, there are no OCaml bindings for libxengnttab.
>>>> +
>>>> +Work Items:
>>>> +
>>>> +* Minimal ``tools/ocaml/libs/xg/`` binding for
>>>> ``tools/libs/gnttab/``.
>>>> +* Replicate :xen-cs:`38eeb3864d` for oxenstored as well.
>>>> +
>>>> +
>>>> +2. Figuring out which domain(s) have gone away
>>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> +
>>>> +Currently, the handling of domains is asymmetric.
>>>> +
>>>> +* When a domain is created, the toolstack explicitly sends an
>>>> +  ``XS_INTRODUCE(domid, store mfn, store evtchn)`` message to
>>>> xenstored, to
>>>> +  cause xenstored to connect to the guest ring, and fire the
>>>> +  ``@introduceDomain`` watch.
>>>> +* When a domain is destroyed, Xen fires ``VIRQ_DOM_EXC`` which is
>>>> bound by
>>>> +  xenstored, rather than the toolstack.  xenstored updates its idea
>>>> of the
>>>> +  status of domains, and fires the ``@releaseDomain`` watch.
>>>> +
>>>> +Xenstored uses ``xc_domain_getinfo()``, to work out which domain(s)
>>>> have gone
>>>> +away, and only cares about the shutdown status.
>>>> +
>>>> +Furthermore, ``@releaseDomain`` (like ``VIRQ_DOM_EXC``) is a
>>>> single-bit
>>>> +message, which requires all listeners to evaluate whether the
>>>> message applies
>>>> +to them or not.  This results in a flurry of ``xc_domain_getinfo()``
>>>> calls
>>>> +from multiple entities in the system, which all serialise on the
>>>> domctl lock
>>>> +in Xen.
>>>> +
>>>> +Work Items:
>>>> +
>>>> +* Figure out how shutdown status can be expressed in a stable way
>>>> from Xen.
>>>> +* Figure out if ``VIRQ_DOM_EXC`` and ``@releaseDomain`` can be
>>>> extended to
>>>> +  carry at least a domid, to make domain shutdown scale better.
>>>
>>> @releaseDomain (and @introduceDomain) can't be extended, we'd need to
>>> add another watch path like @domainStatus/<domid>/<newState>. Xenstored
>>> could advertise its capability to raise this watch in /tool/xenstored.
>>
>> I guess I was being a bit fast and loose with terminology.  I didn't
>> intend to imply "literally modify @{introduce,release}Domain", as they
>> are already fixed ABIs, but more to "compatibly build something which is
>> better".
>
> Okay.
>
>>
>> That scheme would work for improved @releaseDomain, but it wouldn't work
>> for an improved introduce.  Introduce needs a single key to watch on,
>> which hands back the domid so you don't need to go searching for it.
>
> Yes, and? Its perfectly fine to set a watch firing if anything below
> @domainStatus is changing.

Hmm - that might work if no other information was put into domainStatus,
but would quickly become a scalability problem otherwise.

>
>>
>>>
>>> As VIRQ_DOM_EXC is just an event I don't see how the domid could be
>>> passed by it. I guess we'd need e.g. a shared memory area which the
>>> domain registered for VIRQ_DOM_EXC could map and which would contain a
>>> bitmap (one bit per domain). The hypervisor would set the bit on a
>>> status change and fire VIRQ_DOM_EXC, xenstored would look for a set
>>> bit, clear it and read the status of the related domain.
>>
>> The point here is to avoid using xc_domain_getinfo() in the first place,
>> so there needs to be no "getting the status of the domain".
>
> I'd guess a single xc_domain_getinfo() in the tools wouldn't be so
> problematic. The caller would know the domid already, so no need to
> query all domains.

Its still a problem when you've got 1000 Qemu's, they all get
@releaseDomain, and try to figure out if it is their own domain which
went away.

This shouldn't require taking the domctl lock in Xen 1000 times to
figure out, seeing as xenstored knows exactly which domain actually went
away.

>
>> DOM_EXC is fired for domain_shutdown() only (but for reasons which
>> escape me, fired twice).  Given that a domid is a 15 bit number, a
>> bitmap of all domains does fit within a single 4k page.
>
> Firing twice is needed: first time for disconnecting all backends
> and the second time for cleaning up when the domain is completely
> gone.

Do we really have pieces of code which count the the @releaseDomain's
for a specific domain?  Please say no...

~Andrew

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