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: 7452 bytes --]

On Tue, Oct 16, 2018 at 05:53:02PM +0100, Ian Jackson wrote:
> Marek Marczykowski-Górecki writes ("[RFC PATCH v2 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain."):
> > [stuff]
> 
> Thanks for this.  I'm very keen on this approach and keen on getting
> it upstream.  Sorry for the delay reviewing it!  I have been away a
> lot.
> 
> > General idea is to allow freely set device_model_version and
> > device_model_stubdomain_override and choose the right options based on this choice.
> > Also, allow to specific path to stubdomain kernel/ramdisk, for greater flexibility.
> > Right now when qemu-xen in stubdomain is selected, it is assumed it's
> > Linux-based and few decisions are based on it, specifically:
> 
> That sounds sensible.
> 
> >  - qemu args encoding (\x1b as separator, to allow spaces in arguments)
> 
> What if the arguments contain \x1b ? 

Well, here I've changed "What if the arguments contain a space" to "What
if the arguments contain \x1b" ;)

> I think you may need a quoting
> scheme, or to use nul.

The main reason for this over alternatives (including nul) is using
shell script on the stubdomain side to handle this. Handling nul char in
shell is major PITA.

> > It may be a good idea to document "stubdomain API" somewhere. Is there such
> > document for MiniOS one?
> 
> No.
> 
> >  Here is an initial version for Linux one:
> > 
> >     Assumptions about Linux-based stubdomain for qemu-xen:
> >     * qemu command line is stored by toolstack in
> >       /vm/<target-uuid>/image/dmargs xenstore entry, arguments are separated
> >       with \x1b character
> 
> Oh, xenstore.  From docs/misc/xenstore.txt:
> 
> | xenstore values should normally be 7-bit ASCII text strings
> | containing bytes 0x20..0x7f only
> 
> I think you could have separate xenstore keys for the seperate
> arguments, or you could encode it somehow.

What "normally" means in this context? For me it doesn't mean other
characters are prohibited.

> >     * qemu can access saved state (if any) at its FD 3
> >     * qemu can write its state (for save/migration) to its FD 4
> 
> That's a description of the protocol to *qemu*.  Is the toolstack then
> responsible for the protocol across the domain boundary ?  I think it
> would be nice to specify that here too.

Well, toolstack is responsible for qemu command line (and QMP), so it is
part of the stubdomain protocol...

> 
> >     * qemu expects backend for serial console at /dev/hvc3
> >     * disks configured for the target are available as /dev/xvd*, in
> >       configuration order
> >     * qemu can call /etc/qemu-ifup and /etc/qemu-ifdown to connect/disconnect
> >       network interface to appropriate network

Actually, after recent changes in our linux stubdomain, this last point
wouldn't be needed. It's handled internally by observing qmp events.

> Please can we put this in-tree and not in a 00/ cover letter :-).

Sure.

> > Initial version has no QMP support - in initial patches it is completely
> > disabled, which means no suspend/restore. QMP normally would be used for PCI
> > passthrough setup, but it is worked around with MiniOS-like control protocol
> > over xenstore, which then is translated to QMP on stubdomain side.
> 
> How unpleasant.  But better than nothing.
>
> > Some option is to use separate console for that, but that require
> > xenstoled supporting multiple consoles per domain (the goal is to not have qemu
> > in dom0 at all). Also, it would be preferable to evaluate how libxl
> > handle potentially malicious QMP responses.
> 
> We are working on that latter point anyway.
> 
> > Another option is to use xenstore - either translate everything needed to
> > MiniOS-like thing, or write already json-formatted requests to xenstore and
> > watch there for responses. When using separate xenstore dir for that, with
> > per-command entries (command id as a key name?) that would solve concurrency
> > problem.
> 
> That would be fine too.
> 
> > QMP support over separate console: patch "libxl: access QMP socket via console
> > for qemu-in-stubdomain" implements some early PoC of this.
> > Major limitation: only one connection at a time and no means of out of
> > band reset (and re-negotiate). I've tried adding very simple `qmp_reset`
> > command for in-band connection reset, but it is unreliable because of the
> > first limitation - xl process running in background hold this connection open
> > and every other xl instance interfere with it. On the other hand, for libvirt
> > use case one connection could be enough (leaving alone libvirtd restart).
> 
> This is awkward, isn't it.  The Xen PV console protocol has no reset
> mechanism.  Can we use libvchan here and provide qmp vchans ?

That would be an option too. Require (trivial) vchan->unix socket proxy.

> > Xenconsoled patches add support for multiple consoles in xenconsoled, to avoid the need
> > for qemu in dom0 for this to work. Multiple consoles for a stubdomain are used for:
> >  - logs (console 0)
> >  - save + restore (console 1, 2)
> >  - qmp (console 3)
> >  - serial terminal to target domU (console 4)
> > Xenconsoled patches are in fact a separate series, but I'm sending them here to
> > ease dependencies handling (latter libxl patches use that).
> 
> That sounds reasonable.
> 
> > What qmp-libxenstat socket is for?
> > 
> > PCI passthrough support require some more love. Right now, libxl tries to setup
> > pcifront for both target HVM and stubdomain and the former fails (race
> > condition):
> >     xen-pciback pci-259-0: 22 Couldn't locate PCI device (0000:00:1b.0)! perhaps already in-use?
> > Fortunately it isn't needed. There is also a PCI related problem on
> > domain shutdown - it looks like first stubdomain is paused and then libxl waits
> > for pcifront there.
> > Also, MSI doesn't work (qemu output):
> > 
> >     [00:05.0] xen_pt_msgctrl_reg_write: setup MSI (register: 81).
> >     [00:05.0] msi_msix_setup: requested pirq 55 for MSI (vec: 0, entry: 0)
> >     [00:05.0] msi_msix_setup: Error: Mapping of MSI (err: 1, vec: 0, entry 0)
> >     [00:05.0] xen_pt_msgctrl_reg_write: Warning: Can not map MSI (register: 80)!
> >     [00:05.0] Write-back to unknown field 0x44 (partially) inhibited (0x00)
> >     [00:05.0] If the device doesn't work, try enabling permissive mode
> >     [00:05.0] (unsafe) and if it helps report the problem to xen-devel
> 
> I confess I don't understand PCI passthrough but it would be nice for
> this to work.  I think a starting point might be to write down who is
> supposed to do what...
> 
> > This patch series is third (fourth?) attempt to get rid of limitation
> > "if you want to use stubdomain, you're stuck with qemu-traditional", done over
> > years, by many people.
> > I think it should be acceptable plan to gradually add features to
> > qemu-xen+stubdomain configuration - not necessary waiting with committing any
> > of those patches until full feature set of qemu-xen is supported. After all,
> > right now "feature set supported by qemu-xen+stubdom" is empty, so wouldn't be
> > worse.
> 
> Absolutely.
> 
> 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