Re: [RFC PATCH v2 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain.

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4707 bytes --]

On Thu, Nov 01, 2018 at 04:57:18PM +0000, Ian Jackson wrote:
> Marek Marczykowski-Górecki writes ("Re: [RFC PATCH v2 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain."):
> > All the xenconsoled stuff is unchanged (and unlikely to change), so it
> > should be safe to review it. Patches 06 and 07 also shouldn't change.
> 
> Sorry, I missed this reply.  I will go on to review those.
> 
> > The thing that will change is qemu cmdline and qmp handling. TBH I'm not sure
> > if its better to touch qmp now, or after reworked qmp handling by
> > Anthony will be merged. There will definitely be some conflicts and it
> > may even affects what underlying mechanism is chosen for qmp transport.
> > Based on discussion here, and in libxl__ev_qmp_* thread, I see two
> > viable options:
> > 
> > 1. libvchan
> >   pros:
> >    - out of band reset support, so qmp capabilities negotiation can be
> >      handled gracefully
> >   cons:
> >    - more work, require patching qemu (or adding vchan->socket proxy),
> >      adds dependency on libvchan to libxl (probably not a problem)
> >    - possibly more complex libxl__ev_qmp_* handling, or at least needs
> >      separate handling of send/receive for stubdomain case
> 
> I think that the changes to libxl__ev_qmp_* will be relatively
> self-contained, particularly after Anthony's async rework.  There's
> one place where the communication occurs.
> 
> Does libvchan offer asynchronous connection (ie, connect/disconnect
> calls which cannot be stalled by the peer, but which instead allow
> poll/select-based async) ?  I think it may not, in which case we need
> a vchan to socket proxy anyway.

libxenvchan_server_init is asynchronous. libxenvchan_client_init is too,
but it fails if called before server is ready. I have a
wrapper[1] around libxenvchan_client_init in Qubes code which solve this
problem with xs_watch. "libvchan: create xenstore entries in one
transaction" patch is related to that wrapper.

Maybe it should be also added to libxenvchan? Right now it only waits
(synchronously) for server to appear (using while(...) xs_read_watch()).
This is slightly more complex, as it also handle remote domain death
before establishing connection as well as save+restore local domain.
But it can be provided as a separate function like
libxenvchan_client_wait_for_server or such.

Providing a function that could be used in libxl would be more complex,
as it needs to integrate with libxl async API. Maybe it could use
good old trick with separate thread + pipe() for signaling readiness?
This way, the libxenvchan_client_wait_for_server would start separate
thread (using own xenstore connection) and return fd that libxl can wait
on. No need to convert libxenvchan_client_wait_for_server into callback
hell...

[1] https://github.com/QubesOS/qubes-core-vchan-xen/blob/master/vchan/init.c#L58-L168

> Aside from that the libxl dependency is untroublesome.
> 
> > 2. pv console
> >   pros:
> >    - no qemu modifications
> >    - same read()/write() on libxl side
> >   cons:
> >    - no out of band reset, needs libxl handling for that (skipping
> >      negotiation)
> 
> Doesn't this potentially mean that the qmp console connection can
> become irrecoverably desynchronised ?  I don't know how you would
> recover from the situation where another libxl process had got halfway
> through some qmp stuff and been terminated (for whatever reason; maybe
> the calling toolstack crashed).

That's right, it could result in irrecoverably desynchronised
connection. So, we need out of band reset.

> >    - possibly other problems from that (events filling up some buffers
> >      when no one is listening?)
> 
> xenconsole drops things in this situation.  I think that may result in
> desynchronisation.
> 
> > BTW Does libxl listed for qmp events?
> 
> Not right now.  It may want to in future.  Anthony's qmp series
> discards events.
> 
> > There is also third option: xenstore, but that would probably require
> > totally separate libxl__ev_qmp_* implementation, so I'd rule it out.
> 
> That's not a terrible idea but I can't see it being popular with qemu
> upstream, so you'd end up writing a kind of bidirectional
> qmp<->xenstore proxy.  Urgh.

Well, I do that already (for pci-ins). In bash.

> > If problems with pv console could be solved, I'd go this way. But
> > otherwise libvchan seems like a good alternative.
> 
> Yes.
> 
> Ian.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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