* [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain
@ 2020-04-28 4:04 Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol Jason Andryuk
` (23 more replies)
0 siblings, 24 replies; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Jason Andryuk,
Andrew Cooper, George Dunlap, Marek Marczykowski-Górecki,
Simon Gaiser, Jan Beulich, Samuel Thibault, Anthony PERARD,
Ian Jackson, Eric Shelton
Hi,
In coordination with Marek, I'm making a submission of his patches for Linux
stubdomain device-model support. I made a few of my own additions, but Marek
did the heavy lifting. Thank you, Marek.
Below is mostly the v4 cover leter with a few additions.
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.
First two patches add documentation about expected toolstack-stubdomain-qemu
interface, both for MiniOS stubdomain and Linux stubdomain.
Initial version has no QMP support - in initial patches it is completely
disabled, which means no suspend/restore and no PCI passthrough.
Later patches add QMP over libvchan connection support. The actual connection
is made in a separate process. As discussed on Xen Summit 2019, this allows to
apply some basic checks and/or filtering (not part of this series), to limit
libxl exposure for potentially malicious stubdomain.
Jason's additions ensure the qmp-proxy (vchan-socket-proxy) processes and
sockets are cleaned up and add some documentation.
The actual stubdomain implementation is here:
https://github.com/marmarek/qubes-vmm-xen-stubdom-linux
(branch for-upstream, tag for-upstream-v3)
See readme there for build instructions. Marek's version requires dracut. I
have hacked up a version usable install with initramfs-tools:
https://github.com/jandryuk/qubes-vmm-xen-stubdom-linux
(branch initramfs-tools)
Few comments/questions about the stubdomain code:
1. There are extra patches for qemu that are necessary to run it in stubdomain.
While it is desirable to upstream them, I think it can be done after merging
libxl part. Stubdomain's qemu build will in most cases be separate anyway, to
limit qemu's dependencies (so the stubdomain size).
2. By default Linux hvc-xen console frontend is unreliable for data transfer
(qemu state save/restore) - it drops data sent faster than client is reading
it. To fix it, console device needs to be switched into raw mode (`stty raw
/dev/hvc1`). Especially for restoring qemu state it is tricky, as it would need
to be done before opening the device, but stty (obviously) needs to open the
device first. To solve this problem, for now the repository contains kernel
patch which changes the default for all hvc consoles. Again, this isn't
practical problem, as the kernel for stubdomain is built separately. But it
would be nice to have something working with vanilla kernel. I see those
options:
- convert it to kernel cmdline parameter (hvc_console_raw=1 ?)
- use channels instead of consoles (and on the kernel side change the default
to "raw" only for channels); while in theory better design, libxl part will
be more complex, as channels can be connected to sockets but not files, so
libxl would need to read/write to it exactly when qemu write/read the data,
not before/after as it is done now
3. Mini-OS stubdoms use dmargs xenstore key as a string. Linux stubdoms use
dmargs as a directory for numbered entries. Should they be different names?
Remaining parts for eliminating dom0's instance of qemu:
- do not force QDISK backend for CDROM
- multiple consoles support in xenconsoled
Changes in v2:
- apply review comments by Jason Andryuk
Changes in v3:
- rework qemu arguments handling (separate xenstore keys, instead of \x1b separator)
- add QMP over libvchan, instead of console
- add protocol documentation
- a lot of minor changes, see individual patches for full changes list
- split xenconsoled patches into separate series
Changes in v4:
- extract vchan connection into a separate process
- rebase on master
- various fixes
Changes in v5:
- Marek: apply review comments from Jason Andryuk
- Jason: Clean up qmp-proxy processes and sockets
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Cc: Simon Gaiser <simon@invisiblethingslab.com>
Cc: Eric Shelton <eshelton@pobox.com>
Cc: Ian Jackson <ian.jackson@citrix.com>
Cc: Wei Liu <wl@xen.org>
Eric Shelton (1):
libxl: Handle Linux stubdomain specific QEMU options.
Jason Andryuk (5):
docs: Add device-model-domid to xenstore-paths
libxl: Check stubdomain kernel & ramdisk presence
libxl: Refactor kill_device_model to libxl__kill_xs_path
libxl: Kill vchan-socket-proxy when cleaning up qmp
tools: Clean up vchan-socket-proxy socket
Marek Marczykowski-Górecki (15):
Document ioemu MiniOS stubdomain protocol
Document ioemu Linux stubdomain protocol
libxl: fix qemu-trad cmdline for no sdl/vnc case
libxl: Allow running qemu-xen in stubdomain
libxl: write qemu arguments into separate xenstore keys
xl: add stubdomain related options to xl config parser
tools/libvchan: notify server when client is connected
libxl: add save/restore support for qemu-xen in stubdomain
tools: add missing libxenvchan cflags
tools: add simple vchan-socket-proxy
libxl: use vchan for QMP access with Linux stubdomain
Regenerate autotools files
libxl: require qemu in dom0 even if stubdomain is in use
libxl: ignore emulated IDE disks beyond the first 4
libxl: consider also qemu in stubdomain in libxl__dm_active check
.gitignore | 1 +
configure | 14 +-
docs/configure | 14 +-
docs/man/xl.cfg.5.pod.in | 27 +-
docs/misc/stubdom.txt | 103 ++++++
docs/misc/xenstore-paths.pandoc | 5 +
stubdom/configure | 14 +-
tools/Rules.mk | 2 +-
tools/config.h.in | 3 +
tools/configure | 46 ++-
tools/configure.ac | 9 +
tools/libvchan/Makefile | 8 +-
tools/libvchan/init.c | 3 +
tools/libvchan/vchan-socket-proxy.c | 500 ++++++++++++++++++++++++++++
tools/libxl/libxl_aoutils.c | 32 ++
tools/libxl/libxl_create.c | 46 ++-
tools/libxl/libxl_dm.c | 484 +++++++++++++++++++++------
tools/libxl/libxl_domain.c | 7 +
tools/libxl/libxl_internal.h | 22 ++
tools/libxl/libxl_mem.c | 6 +-
tools/libxl/libxl_qmp.c | 27 +-
tools/libxl/libxl_types.idl | 3 +
tools/xl/xl_parse.c | 7 +
23 files changed, 1205 insertions(+), 178 deletions(-)
create mode 100644 tools/libvchan/vchan-socket-proxy.c
--
2.20.1
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:08 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 02/21] Document ioemu Linux " Jason Andryuk
` (22 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Jason Andryuk,
Andrew Cooper, Marek Marczykowski-Górecki, George Dunlap,
Jan Beulich, Ian Jackson
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Add documentation based on reverse-engineered toolstack-ioemu stubdomain
protocol.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
docs/misc/stubdom.txt | 53 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
index 882a18cab4..64c77d9b64 100644
--- a/docs/misc/stubdom.txt
+++ b/docs/misc/stubdom.txt
@@ -23,6 +23,59 @@ and https://wiki.xen.org/wiki/Device_Model_Stub_Domains for more
information on device model stub domains
+Toolstack to MiniOS ioemu stubdomain protocol
+---------------------------------------------
+
+This section describe communication protocol between toolstack and
+qemu-traditional running in MiniOS stubdomain. The protocol include
+expectations of both qemu and stubdomain itself.
+
+Setup (done by toolstack, expected by stubdomain):
+ - Block devices for target domain are connected as PV disks to stubdomain,
+ according to configuration order, starting with xvda
+ - Network devices for target domain are connected as PV nics to stubdomain,
+ according to configuration order, starting with 0
+ - if graphics output is expected, VFB and VKB devices are set for stubdomain
+ (its backend is responsible for exposing them using appropriate protocol
+ like VNC or Spice)
+ - other target domain's devices are not connected at this point to stubdomain
+ (may be hot-plugged later)
+ - QEMU command line (space separated arguments) is stored in
+ /vm/<target-uuid>/image/dmargs xenstore path
+ - target domain id is stored in /local/domain/<stubdom-id>/target xenstore path
+?? - bios type is stored in /local/domain/<target-id>/hvmloader/bios
+ - stubdomain's console 0 is connected to qemu log file
+ - stubdomain's console 1 is connected to qemu save file (for saving state)
+ - stubdomain's console 2 is connected to qemu save file (for restoring state)
+ - next consoles are connected according to target guest's serial console configuration
+
+Startup:
+1. PV stubdomain is started with ioemu-stubdom.gz kernel and no initrd
+2. stubdomain initialize relevant devices
+3. stubdomain signal readiness by writing "running" to /local/domain/<stubdom-id>/device-model/<target-id>/state xenstore path
+4. now stubdomain is considered running
+
+Runtime control (hotplug etc):
+Toolstack can issue command through xenstore. The sequence is (from toolstack POV):
+1. Write parameter to /local/domain/<stubdom-id>/device-model/<target-id>/parameter.
+2. Write command to /local/domain/<stubdom-id>/device-model/<target-id>/command.
+3. Wait for command result in /local/domain/<stubdom-id>/device-model/<target-id>/state (command specific value).
+4. Write "running" back to /local/domain/<stubdom-id>/device-model/<target-id>/state.
+
+Defined commands:
+ - "pci-ins" - PCI hot plug, results:
+ - "pci-inserted" - success
+ - "pci-insert-failed" - failure
+ - "pci-rem" - PCI hot remove, results:
+ - "pci-removed" - success
+ - ??
+ - "save" - save domain state to console 1, results:
+ - "paused" - success
+ - "continue" - resume domain execution, after loading state from console 2 (require -loadvm command argument), results:
+ - "running" - success
+
+
+
PV-GRUB
=======
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 02/21] Document ioemu Linux stubdomain protocol
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:08 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 03/21] libxl: fix qemu-trad cmdline for no sdl/vnc case Jason Andryuk
` (21 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Jason Andryuk,
Andrew Cooper, Marek Marczykowski-Górecki, George Dunlap,
Jan Beulich, Ian Jackson
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Add documentation for upcoming Linux stubdomain for qemu-upstream.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
docs/misc/stubdom.txt | 50 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
index 64c77d9b64..bb5e87dfb9 100644
--- a/docs/misc/stubdom.txt
+++ b/docs/misc/stubdom.txt
@@ -75,6 +75,56 @@ Defined commands:
- "running" - success
+Toolstack to Linux ioemu stubdomain protocol
+--------------------------------------------
+
+This section describe communication protocol between toolstack and
+qemu-upstream running in Linux stubdomain. The protocol include
+expectations of both stubdomain, and qemu.
+
+Setup (done by toolstack, expected by stubdomain):
+ - Block devices for target domain are connected as PV disks to stubdomain,
+ according to configuration order, starting with xvda
+ - Network devices for target domain are connected as PV nics to stubdomain,
+ according to configuration order, starting with 0
+ - [not implemented] if graphics output is expected, VFB and VKB devices are set for stubdomain
+ (its backend is responsible for exposing them using appropriate protocol
+ like VNC or Spice)
+ - other target domain's devices are not connected at this point to stubdomain
+ (may be hot-plugged later)
+ - QEMU command line is stored in
+ /vm/<target-uuid>/image/dmargs xenstore dir, each argument as separate key
+ in form /vm/<target-uuid>/image/dmargs/NNN, where NNN is 0-padded argument
+ number
+ - target domain id is stored in /local/domain/<stubdom-id>/target xenstore path
+?? - bios type is stored in /local/domain/<target-id>/hvmloader/bios
+ - stubdomain's console 0 is connected to qemu log file
+ - stubdomain's console 1 is connected to qemu save file (for saving state)
+ - stubdomain's console 2 is connected to qemu save file (for restoring state)
+ - next consoles are connected according to target guest's serial console configuration
+
+Environment exposed by stubdomain to qemu (needed to construct appropriate qemu command line and later interact with qmp):
+ - target domain's disks are available as /dev/xvd[a-z]
+ - console 2 (incoming domain state) is connected with FD 3
+ - console 1 (saving domain state) is added over QMP to qemu as "fdset-id 1" (done by stubdomain, toolstack doesn't need to care about it)
+ - nics are connected to relevant stubdomain PV vifs when available (qemu -netdev should specify ifname= explicitly)
+
+Startup:
+1. toolstack starts PV stubdomain with stubdom-linux-kernel kernel and stubdom-linux-initrd initrd
+2. stubdomain initialize relevant devices
+3. stubdomain starts qemu with requested command line, plus few stubdomain specific ones - including local qmp access options
+4. stubdomain starts vchan server on /local/domain/<stubdom-id>/device-model/<target-id>/qmp-vchan, exposing qmp socket to the toolstack
+5. qemu signal readiness by writing "running" to /local/domain/<stubdom-id>/device-model/<target-id>/state xenstore path
+6. now device model is considered running
+
+QEMU can be controlled using QMP over vchan at /local/domain/<stubdom-id>/device-model/<target-id>/qmp-vchan. Only one simultaneous connection is supported and toolstack needs to ensure that.
+
+Limitations:
+ - PCI passthrough require permissive mode
+ - only one nic is supported
+ - at most 26 emulated disks are supported (more are still available as PV disks)
+ - graphics output (VNC/SDL/Spice) not supported
+
PV-GRUB
=======
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 03/21] libxl: fix qemu-trad cmdline for no sdl/vnc case
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 02/21] Document ioemu Linux " Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 04/21] libxl: Allow running qemu-xen in stubdomain Jason Andryuk
` (20 subsequent siblings)
23 siblings, 0 replies; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Wei Liu, Wei Liu, Jason Andryuk, Marek Marczykowski-Górecki,
Anthony PERARD, Ian Jackson
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
When qemu is running in stubdomain, any attempt to initialize vnc/sdl
there will crash it (on failed attempt to load a keymap from a file). If
vfb is present, all those cases are skipped. But since
b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do not start dom0 qemu
for stubdomain when not needed" it is possible to create a stubdomain
without vfb and contrary to the comment -vnc none do trigger VNC
initialization code (just skips exposing it externally).
Change the implicit SDL avoiding method to -nographics option, used when
none of SDL or VNC is enabled.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v2:
- typo in qemu option
Changes in v3:
- add missing { }
---
tools/libxl/libxl_dm.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f4007bbe50..b91e63db6f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -734,14 +734,15 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
if (libxl_defbool_val(vnc->findunused)) {
flexarray_append(dm_args, "-vncunused");
}
- } else
+ } else if (!sdl) {
/*
* VNC is not enabled by default by qemu-xen-traditional,
- * however passing -vnc none causes SDL to not be
- * (unexpectedly) enabled by default. This is overridden by
- * explicitly passing -sdl below as required.
+ * however skipping -vnc causes SDL to be
+ * (unexpectedly) enabled by default. If undesired, disable graphics at
+ * all.
*/
- flexarray_append_pair(dm_args, "-vnc", "none");
+ flexarray_append(dm_args, "-nographic");
+ }
if (sdl) {
flexarray_append(dm_args, "-sdl");
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 04/21] libxl: Allow running qemu-xen in stubdomain
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (2 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 03/21] libxl: fix qemu-trad cmdline for no sdl/vnc case Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:10 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options Jason Andryuk
` (19 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Do not prohibit anymore using stubdomain with qemu-xen.
To help distingushing MiniOS and Linux stubdomain, add helper inline
functions libxl__stubdomain_is_linux() and
libxl__stubdomain_is_linux_running(). Those should be used where really
the difference is about MiniOS/Linux, not qemu-xen/qemu-xen-traditional.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v3:
- new patch, instead of "libxl: Add "stubdomain_version" to
domain_build_info"
- helper functions as suggested by Ian Jackson
---
tools/libxl/libxl_create.c | 9 ---------
tools/libxl/libxl_internal.h | 17 +++++++++++++++++
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e7cb2dbc2b..7423ee8e57 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -171,15 +171,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
}
}
- if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
- b_info->device_model_version !=
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL &&
- libxl_defbool_val(b_info->device_model_stubdomain)) {
- LOG(ERROR,
- "device model stubdomains require \"qemu-xen-traditional\"");
- return ERROR_INVAL;
- }
-
if (!b_info->max_vcpus)
b_info->max_vcpus = 1;
if (!b_info->avail_vcpus.size) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5f39e44cb9..ebbf926897 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2320,6 +2320,23 @@ _hidden int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
/* Return the system-wide default device model */
_hidden libxl_device_model_version libxl__default_device_model(libxl__gc *gc);
+static inline
+bool libxl__stubdomain_is_linux_running(libxl__gc *gc, uint32_t domid)
+{
+ /* same logic as in libxl__stubdomain_is_linux */
+ return libxl__device_model_version_running(gc, domid)
+ == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
+
+static inline
+bool libxl__stubdomain_is_linux(libxl_domain_build_info *b_info)
+{
+ /* right now qemu-tranditional implies MiniOS stubdomain and qemu-xen
+ * implies Linux stubdomain */
+ return libxl_defbool_val(b_info->device_model_stubdomain) &&
+ b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
+
#define DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, fmt, _a...) \
libxl__sprintf(gc, "/local/domain/%u/device-model/%u" fmt, dm_domid, \
domid, ##_a)
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options.
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (3 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 04/21] libxl: Allow running qemu-xen in stubdomain Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:19 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys Jason Andryuk
` (18 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Wei Liu, Jason Andryuk, Marek Marczykowski-Górecki,
Simon Gaiser, Anthony PERARD, Ian Jackson, Eric Shelton
From: Eric Shelton <eshelton@pobox.com>
This patch creates an appropriate command line for the QEMU instance
running in a Linux-based stubdomain.
NOTE: a number of items are not currently implemented for Linux-based
stubdomains, such as:
- save/restore
- QMP socket
- graphics output (e.g., VNC)
Signed-off-by: Eric Shelton <eshelton@pobox.com>
Simon:
* fix disk path
* fix cdrom path and "format"
Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
[drop Qubes-specific parts]
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Allow setting stubdomain_ramdisk independently from stubdomain_kernel
Add a qemu- prefix for qemu-stubdom-linux-{kernel,rootfs} since stubdom
doesn't convey device-model. Use qemu- since this code is qemu specific.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v2:
- fix serial specified with serial=[ ... ] syntax
- error out on multiple consoles (incompatible with stubdom)
- drop erroneous chunk about cdrom
Changes in v3:
- change to use libxl__stubdomain_is_linux instead of
b_info->stubdomain_version
- drop libxl__stubdomain_version_running, prefer
libxl__stubdomain_is_linux_running introduced by previous patch
- drop ifup/ifdown script - stubdomain will handle that with qemu
events itself
- slightly simplify -serial argument
- add support for multiple serial consoles, do not ignore
b_info.u.serial(_list)
- add error checking for more than 26 emulated disks ("/dev/xvd%c"
format string)
Changes in v5:
- commit message fixup to match patch contents - Marek
- file names are now qemu-stubdom-linux-{kernel,rootfs} - Jason
- allow setting ramdisk independently of kernel - Jason
---
tools/libxl/libxl_create.c | 45 +++++++++
tools/libxl/libxl_dm.c | 190 ++++++++++++++++++++++++-----------
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_mem.c | 6 +-
tools/libxl/libxl_types.idl | 3 +
5 files changed, 184 insertions(+), 61 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 7423ee8e57..3b5535f2c8 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -171,6 +171,40 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
}
}
+ if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
+ libxl_defbool_val(b_info->device_model_stubdomain)) {
+ if (!b_info->stubdomain_kernel) {
+ switch (b_info->device_model_version) {
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+ b_info->stubdomain_kernel =
+ libxl__abs_path(NOGC, "ioemu-stubdom.gz", libxl__xenfirmwaredir_path());
+ break;
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+ b_info->stubdomain_kernel =
+ libxl__abs_path(NOGC,
+ "qemu-stubdom-linux-kernel",
+ libxl__xenfirmwaredir_path());
+ break;
+ default:
+ abort();
+ }
+ }
+ if (!b_info->stubdomain_ramdisk) {
+ switch (b_info->device_model_version) {
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+ break;
+ case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+ b_info->stubdomain_ramdisk =
+ libxl__abs_path(NOGC,
+ "qemu-stubdom-linux-rootfs",
+ libxl__xenfirmwaredir_path());
+ break;
+ default:
+ abort();
+ }
+ }
+ }
+
if (!b_info->max_vcpus)
b_info->max_vcpus = 1;
if (!b_info->avail_vcpus.size) {
@@ -206,6 +240,17 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
b_info->target_memkb = b_info->max_memkb;
+ if (b_info->stubdomain_memkb == LIBXL_MEMKB_DEFAULT) {
+ if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+ if (libxl__stubdomain_is_linux(b_info))
+ b_info->stubdomain_memkb = LIBXL_LINUX_STUBDOM_MEM * 1024;
+ else
+ b_info->stubdomain_memkb = 28 * 1024; // MiniOS
+ } else {
+ b_info->stubdomain_memkb = 0; // no stubdomain
+ }
+ }
+
libxl_defbool_setdefault(&b_info->claim_mode, false);
libxl_defbool_setdefault(&b_info->localtime, false);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b91e63db6f..5a7d55686f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1188,6 +1188,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
int i, connection, devid;
uint64_t ram_size;
const char *path, *chardev;
+ bool is_stubdom = libxl_defbool_val(b_info->device_model_stubdomain);
dm_args = flexarray_make(gc, 16, 1);
dm_envs = flexarray_make(gc, 16, 1);
@@ -1197,39 +1198,42 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_vappend(dm_args, dm,
"-xen-domid",
GCSPRINTF("%d", guest_domid), NULL);
+ flexarray_append(dm_args, "-no-shutdown");
- flexarray_append(dm_args, "-chardev");
- if (state->dm_monitor_fd >= 0) {
- flexarray_append(dm_args,
- GCSPRINTF("socket,id=libxl-cmd,fd=%d,server,nowait",
- state->dm_monitor_fd));
+ /* There is currently no way to access the QMP socket in the stubdom */
+ if (!is_stubdom) {
+ flexarray_append(dm_args, "-chardev");
+ if (state->dm_monitor_fd >= 0) {
+ flexarray_append(dm_args,
+ GCSPRINTF("socket,id=libxl-cmd,fd=%d,server,nowait",
+ state->dm_monitor_fd));
- /*
- * Start QEMU with its "CPU" paused, it will not start any emulation
- * until the QMP command "cont" is used. This also prevent QEMU from
- * writing "running" to the "state" xenstore node so we only use this
- * flag when we have the QMP based startup notification.
- * */
- flexarray_append(dm_args, "-S");
- } else {
- flexarray_append(dm_args,
- GCSPRINTF("socket,id=libxl-cmd,"
- "path=%s,server,nowait",
- libxl__qemu_qmp_path(gc, guest_domid)));
- }
+ /*
+ * Start QEMU with its "CPU" paused, it will not start any emulation
+ * until the QMP command "cont" is used. This also prevent QEMU from
+ * writing "running" to the "state" xenstore node so we only use this
+ * flag when we have the QMP based startup notification.
+ * */
+ flexarray_append(dm_args, "-S");
+ } else {
+ flexarray_append(dm_args,
+ GCSPRINTF("socket,id=libxl-cmd,"
+ "path=%s,server,nowait",
+ libxl__qemu_qmp_path(gc, guest_domid)));
+ }
- flexarray_append(dm_args, "-no-shutdown");
- flexarray_append(dm_args, "-mon");
- flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
+ flexarray_append(dm_args, "-mon");
+ flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
- flexarray_append(dm_args, "-chardev");
- flexarray_append(dm_args,
- GCSPRINTF("socket,id=libxenstat-cmd,"
- "path=%s/qmp-libxenstat-%d,server,nowait",
- libxl__run_dir_path(), guest_domid));
+ flexarray_append(dm_args, "-chardev");
+ flexarray_append(dm_args,
+ GCSPRINTF("socket,id=libxenstat-cmd,"
+ "path=%s/qmp-libxenstat-%d,server,nowait",
+ libxl__run_dir_path(), guest_domid));
- flexarray_append(dm_args, "-mon");
- flexarray_append(dm_args, "chardev=libxenstat-cmd,mode=control");
+ flexarray_append(dm_args, "-mon");
+ flexarray_append(dm_args, "chardev=libxenstat-cmd,mode=control");
+ }
for (i = 0; i < guest_config->num_channels; i++) {
connection = guest_config->channels[i].connection;
@@ -1273,7 +1277,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_vappend(dm_args, "-name", c_info->name, NULL);
}
- if (vnc) {
+ if (vnc && !is_stubdom) {
char *vncarg = NULL;
flexarray_append(dm_args, "-vnc");
@@ -1312,7 +1316,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
}
flexarray_append(dm_args, vncarg);
- } else
+ } else if (!is_stubdom)
/*
* Ensure that by default no vnc server is created.
*/
@@ -1324,7 +1328,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
*/
flexarray_append_pair(dm_args, "-display", "none");
- if (sdl) {
+ if (sdl && !is_stubdom) {
flexarray_append(dm_args, "-sdl");
if (sdl->display)
flexarray_append_pair(dm_envs, "DISPLAY", sdl->display);
@@ -1366,18 +1370,34 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
{
LOGD(ERROR, guest_domid, "Both serial and serial_list set");
return ERROR_INVAL;
- }
- if (b_info->u.hvm.serial) {
- flexarray_vappend(dm_args,
- "-serial", b_info->u.hvm.serial, NULL);
- } else if (b_info->u.hvm.serial_list) {
- char **p;
- for (p = b_info->u.hvm.serial_list;
- *p;
- p++) {
- flexarray_vappend(dm_args,
- "-serial",
- *p, NULL);
+ } else {
+ if (b_info->u.hvm.serial) {
+ if (is_stubdom) {
+ /* see spawn_stub_launch_dm() for connecting STUBDOM_CONSOLE_SERIAL */
+ flexarray_vappend(dm_args,
+ "-serial",
+ GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL),
+ NULL);
+ } else {
+ flexarray_vappend(dm_args,
+ "-serial", b_info->u.hvm.serial, NULL);
+ }
+ } else if (b_info->u.hvm.serial_list) {
+ char **p;
+ /* see spawn_stub_launch_dm() for connecting STUBDOM_CONSOLE_SERIAL */
+ for (p = b_info->u.hvm.serial_list, i = 0;
+ *p;
+ p++, i++) {
+ if (is_stubdom)
+ flexarray_vappend(dm_args,
+ "-serial",
+ GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL + i),
+ NULL);
+ else
+ flexarray_vappend(dm_args,
+ "-serial",
+ *p, NULL);
+ }
}
}
}
@@ -1386,7 +1406,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_append(dm_args, "-nographic");
}
- if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
+ if (libxl_defbool_val(b_info->u.hvm.spice.enable) && !is_stubdom) {
const libxl_spice_info *spice = &b_info->u.hvm.spice;
char *spiceoptions = dm_spice_options(gc, spice);
if (!spiceoptions)
@@ -1813,7 +1833,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
* If qemu isn't doing the interpreting, the parameter is
* always raw
*/
- if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK)
+ if (libxl_defbool_val(b_info->device_model_stubdomain))
+ format = "host_device";
+ else if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK)
format = libxl__qemu_disk_format_string(disks[i].format);
else
format = libxl__qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
@@ -1824,6 +1846,16 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
disks[i].vdev);
continue;
}
+ } else if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+ if (disk > 'z' - 'a') {
+ LOGD(WARN, guest_domid,
+ "Emulation of only first %d disks is supported with qemu-xen in stubdomain.\n"
+ "Disk %d will be available via PV drivers but not as an emulated disk.",
+ 'z' - 'a',
+ disk);
+ continue;
+ }
+ target_path = GCSPRINTF("/dev/xvd%c", 'a' + disk);
} else {
if (format == NULL) {
LOGD(WARN, guest_domid,
@@ -1964,7 +1996,7 @@ static int libxl__build_device_model_args(libxl__gc *gc,
char ***args, char ***envs,
const libxl__domain_build_state *state,
int *dm_state_fd)
-/* dm_state_fd may be NULL iff caller knows we are using old stubdom
+/* dm_state_fd may be NULL iff caller knows we are using stubdom
* and therefore will be passing a filename rather than a fd. */
{
switch (guest_config->b_info.device_model_version) {
@@ -1974,8 +2006,10 @@ static int libxl__build_device_model_args(libxl__gc *gc,
args, envs,
state);
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
- assert(dm_state_fd != NULL);
- assert(*dm_state_fd < 0);
+ if (!libxl_defbool_val(guest_config->b_info.device_model_stubdomain)) {
+ assert(dm_state_fd != NULL);
+ assert(*dm_state_fd < 0);
+ }
return libxl__build_device_model_args_new(gc, dm,
guest_domid, guest_config,
args, envs,
@@ -2080,6 +2114,16 @@ retry_transaction:
return 0;
}
+static int libxl__store_libxl_entry(libxl__gc *gc, uint32_t domid,
+ const char *name, const char *value)
+{
+ char *path = NULL;
+
+ path = libxl__xs_libxl_path(gc, domid);
+ path = libxl__sprintf(gc, "%s/%s", path, name);
+ return libxl__xs_printf(gc, XBT_NULL, path, "%s", value);
+}
+
static void dmss_init(libxl__dm_spawn_state *dmss)
{
libxl__ev_qmp_init(&dmss->qmp);
@@ -2138,10 +2182,14 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
dmss_init(&sdss->pvqemu);
libxl__xswait_init(&sdss->xswait);
- if (guest_config->b_info.device_model_version !=
- LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
- ret = ERROR_INVAL;
- goto out;
+ assert(libxl_defbool_val(guest_config->b_info.device_model_stubdomain));
+
+ if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
+ if (d_state->saved_state) {
+ LOG(ERROR, "Save/Restore not supported yet with Linux Stubdom.");
+ ret = -1;
+ goto out;
+ }
}
sdss->pvqemu.guest_domid = INVALID_DOMID;
@@ -2163,8 +2211,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
dm_config->b_info.shadow_memkb = 0;
dm_config->b_info.max_vcpus = 1;
- dm_config->b_info.max_memkb = 28 * 1024 +
- guest_config->b_info.video_memkb;
+ dm_config->b_info.max_memkb = guest_config->b_info.stubdomain_memkb;
+ dm_config->b_info.max_memkb += guest_config->b_info.video_memkb;
dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
dm_config->b_info.max_grant_frames = guest_config->b_info.max_grant_frames;
@@ -2203,10 +2251,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
dm_config->num_vkbs = 1;
}
- stubdom_state->pv_kernel.path
- = libxl__abs_path(gc, "ioemu-stubdom.gz", libxl__xenfirmwaredir_path());
- stubdom_state->pv_cmdline = GCSPRINTF(" -d %d", guest_domid);
- stubdom_state->pv_ramdisk.path = "";
+ stubdom_state->pv_kernel.path = guest_config->b_info.stubdomain_kernel;
+ stubdom_state->pv_ramdisk.path = guest_config->b_info.stubdomain_ramdisk;
/* fixme: this function can leak the stubdom if it fails */
ret = libxl__domain_make(gc, dm_config, stubdom_state,
@@ -2226,6 +2272,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
goto out;
}
+ libxl__store_libxl_entry(gc, guest_domid, "dm-version",
+ libxl_device_model_version_to_string(dm_config->b_info.device_model_version));
libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
libxl__xs_printf(gc, XBT_NULL,
GCSPRINTF("%s/image/device-model-domid",
@@ -2235,6 +2283,15 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
GCSPRINTF("%s/target",
libxl__xs_get_dompath(gc, dm_domid)),
"%d", guest_domid);
+ if (guest_config->b_info.device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+ /* qemu-xen is used as a dm in the stubdomain, so we set the bios
+ * accroding to this */
+ libxl__xs_printf(gc, XBT_NULL,
+ libxl__sprintf(gc, "%s/hvmloader/bios",
+ libxl__xs_get_dompath(gc, guest_domid)),
+ "%s",
+ libxl_bios_type_to_string(guest_config->b_info.u.hvm.bios));
+ }
ret = xc_domain_set_target(ctx->xch, dm_domid, guest_domid);
if (ret<0) {
LOGED(ERROR, guest_domid, "setting target domain %d -> %d",
@@ -2316,6 +2373,11 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
if (guest_config->b_info.u.hvm.serial)
num_console++;
+ else if (guest_config->b_info.u.hvm.serial_list) {
+ char **serial = guest_config->b_info.u.hvm.serial_list;
+ while (*(serial++))
+ num_console++;
+ }
console = libxl__calloc(gc, num_console, sizeof(libxl__device_console));
@@ -2349,8 +2411,18 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
console[i].output =
GCSPRINTF("pipe:%s", d_state->saved_state);
break;
+ case STUBDOM_CONSOLE_SERIAL:
+ if (guest_config->b_info.u.hvm.serial) {
+ console[i].output = guest_config->b_info.u.hvm.serial;
+ break;
+ }
+ /* fall-through */
default:
- console[i].output = "pty";
+ /* Serial_list is set, as otherwise num_consoles would be
+ * smaller and consoles 0-2 are handled above. */
+ assert(guest_config->b_info.u.hvm.serial_list);
+ console[i].output = guest_config->b_info.u.hvm.serial_list[
+ i-STUBDOM_CONSOLE_SERIAL];
break;
}
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ebbf926897..a8f0eed945 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -119,6 +119,7 @@
#define STUBDOM_CONSOLE_RESTORE 2
#define STUBDOM_CONSOLE_SERIAL 3
#define STUBDOM_SPECIAL_CONSOLES 3
+#define LIBXL_LINUX_STUBDOM_MEM 128
#define TAP_DEVICE_SUFFIX "-emu"
#define DOMID_XS_PATH "domid"
#define PVSHIM_BASENAME "xen-shim"
diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
index bc7b95aa74..e52a9624ea 100644
--- a/tools/libxl/libxl_mem.c
+++ b/tools/libxl/libxl_mem.c
@@ -459,8 +459,10 @@ int libxl__domain_need_memory_calculate(libxl__gc *gc,
case LIBXL_DOMAIN_TYPE_PVH:
case LIBXL_DOMAIN_TYPE_HVM:
*need_memkb += LIBXL_HVM_EXTRA_MEMORY;
- if (libxl_defbool_val(b_info->device_model_stubdomain))
- *need_memkb += 32 * 1024;
+ if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+ *need_memkb += b_info->stubdomain_memkb;
+ *need_memkb += b_info->video_memkb;
+ }
break;
case LIBXL_DOMAIN_TYPE_PV:
*need_memkb += LIBXL_PV_EXTRA_MEMORY;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f7c473be74..9d3f05f399 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -518,6 +518,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
("device_model_version", libxl_device_model_version),
("device_model_stubdomain", libxl_defbool),
+ ("stubdomain_memkb", MemKB),
+ ("stubdomain_kernel", string),
+ ("stubdomain_ramdisk", string),
# if you set device_model you must set device_model_version too
("device_model", string),
("device_model_ssidref", uint32),
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (4 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:25 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 07/21] xl: add stubdomain related options to xl config parser Jason Andryuk
` (17 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
This allows using arguments with spaces, like -append, without
nominating any special "separator" character.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v3:
- previous version of this patch "libxl: use \x1b to separate qemu
arguments for linux stubdomain" used specific non-printable
separator, but it was rejected as xenstore doesn't cope well with
non-printable chars
---
tools/libxl/libxl_dm.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5a7d55686f..bdc23554eb 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2065,6 +2065,40 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
return 0;
}
+static int libxl__write_stub_linux_dmargs(libxl__gc *gc,
+ int dm_domid, int guest_domid,
+ char **args)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ int i;
+ char *vm_path;
+ char *path;
+ struct xs_permissions roperm[2];
+ xs_transaction_t t;
+
+ roperm[0].id = 0;
+ roperm[0].perms = XS_PERM_NONE;
+ roperm[1].id = dm_domid;
+ roperm[1].perms = XS_PERM_READ;
+
+ vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/vm", guest_domid));
+ path = GCSPRINTF("%s/image/dmargs", vm_path);
+
+retry_transaction:
+ t = xs_transaction_start(ctx->xsh);
+ xs_write(ctx->xsh, t, path, "", 0);
+ xs_set_permissions(ctx->xsh, t, path, roperm, ARRAY_SIZE(roperm));
+ i = 1;
+ for (i=1; args[i] != NULL; i++)
+ xs_write(ctx->xsh, t, GCSPRINTF("%s/%03d", path, i), args[i], strlen(args[i]));
+
+ xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path), roperm, ARRAY_SIZE(roperm));
+ if (!xs_transaction_end(ctx->xsh, t, 0))
+ if (errno == EAGAIN)
+ goto retry_transaction;
+ return 0;
+}
+
static int libxl__write_stub_dmargs(libxl__gc *gc,
int dm_domid, int guest_domid,
char **args)
@@ -2274,7 +2308,10 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
libxl__store_libxl_entry(gc, guest_domid, "dm-version",
libxl_device_model_version_to_string(dm_config->b_info.device_model_version));
- libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
+ if (libxl__stubdomain_is_linux(&guest_config->b_info))
+ libxl__write_stub_linux_dmargs(gc, dm_domid, guest_domid, args);
+ else
+ libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
libxl__xs_printf(gc, XBT_NULL,
GCSPRINTF("%s/image/device-model-domid",
libxl__xs_get_dompath(gc, guest_domid)),
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 07/21] xl: add stubdomain related options to xl config parser
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (5 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:26 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 08/21] tools/libvchan: notify server when client is connected Jason Andryuk
` (16 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
docs/man/xl.cfg.5.pod.in | 27 +++++++++++++++++++++++----
tools/xl/xl_parse.c | 7 +++++++
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 0e9e58a41a..c9bc181a95 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2733,10 +2733,29 @@ model which they were installed with.
=item B<device_model_override="PATH">
-Override the path to the binary to be used as the device-model. The
-binary provided here MUST be consistent with the
-B<device_model_version> which you have specified. You should not
-normally need to specify this option.
+Override the path to the binary to be used as the device-model running in
+toolstack domain. The binary provided here MUST be consistent with the
+B<device_model_version> which you have specified. You should not normally need
+to specify this option.
+
+=item B<stubdomain_kernel="PATH">
+
+Override the path to the kernel image used as device-model stubdomain.
+The binary provided here MUST be consistent with the
+B<device_model_version> which you have specified.
+In case of B<qemu-xen-traditional> it is expected to be MiniOS-based stubdomain
+image, in case of B<qemu-xen> it is expected to be Linux-based stubdomain
+kernel.
+
+=item B<stubdomain_ramdisk="PATH">
+
+Override the path to the ramdisk image used as device-model stubdomain.
+The binary provided here is to be used by a kernel pointed by B<stubdomain_kernel>.
+It is known to be used only by Linux-based stubdomain kernel.
+
+=item B<stubdomain_memory=MBYTES>
+
+Start the stubdomain with MBYTES megabytes of RAM. Default is 128.
=item B<device_model_stubdomain_override=BOOLEAN>
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 4450d59f16..61b4ef7b7e 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2525,6 +2525,13 @@ skip_usbdev:
xlu_cfg_replace_string(config, "device_model_user",
&b_info->device_model_user, 0);
+ xlu_cfg_replace_string (config, "stubdomain_kernel",
+ &b_info->stubdomain_kernel, 0);
+ xlu_cfg_replace_string (config, "stubdomain_ramdisk",
+ &b_info->stubdomain_ramdisk, 0);
+ if (!xlu_cfg_get_long (config, "stubdomain_memory", &l, 0))
+ b_info->stubdomain_memkb = l * 1024;
+
#define parse_extra_args(type) \
e = xlu_cfg_get_list_as_string_list(config, "device_model_args"#type, \
&b_info->extra##type, 0); \
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 08/21] tools/libvchan: notify server when client is connected
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (6 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 07/21] xl: add stubdomain related options to xl config parser Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:27 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain Jason Andryuk
` (15 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, Marek Marczykowski-Górecki, Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Let the server know when the client is connected. Otherwise server will
notice only when client send some data.
This change does not break existing clients, as libvchan user should
handle spurious notifications anyway (for example acknowledge of remote
side reading the data).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Replace spaces with tabs to match the file's whitespace.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Marek: I had this patch in Qubes for a long time and totally forgot it
wasn't upstream thing...
---
tools/libvchan/init.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
index 180833dc2f..ad4b64fbe3 100644
--- a/tools/libvchan/init.c
+++ b/tools/libvchan/init.c
@@ -447,6 +447,9 @@ struct libxenvchan *libxenvchan_client_init(struct xentoollog_logger *logger,
ctrl->ring->cli_live = 1;
ctrl->ring->srv_notify = VCHAN_NOTIFY_WRITE;
+ /* wake up the server */
+ xenevtchn_notify(ctrl->event, ctrl->event_port);
+
out:
if (xs)
xs_daemon_close(xs);
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (7 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 08/21] tools/libvchan: notify server when client is connected Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:35 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 10/21] tools: add missing libxenvchan cflags Jason Andryuk
` (14 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
relevant consoles.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Address TODO in dm_state_save_to_fdset: Only remove savefile for
non-stubdom.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v3:
- adjust for qmp_ev*
- assume specific fdset id in qemu set in stubdomain
Changes in v5:
- Only remove savefile for non-stubdom
---
tools/libxl/libxl_dm.c | 23 +++++++++++------------
tools/libxl/libxl_qmp.c | 27 +++++++++++++++++++++++++--
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index bdc23554eb..45d0dd56f5 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1744,10 +1744,17 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
}
if (state->saved_state) {
- /* This file descriptor is meant to be used by QEMU */
- *dm_state_fd = open(state->saved_state, O_RDONLY);
- flexarray_append(dm_args, "-incoming");
- flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
+ if (is_stubdom) {
+ /* Linux stubdomain connects specific FD to STUBDOM_CONSOLE_RESTORE
+ */
+ flexarray_append(dm_args, "-incoming");
+ flexarray_append(dm_args, "fd:3");
+ } else {
+ /* This file descriptor is meant to be used by QEMU */
+ *dm_state_fd = open(state->saved_state, O_RDONLY);
+ flexarray_append(dm_args, "-incoming");
+ flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
+ }
}
for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
flexarray_append(dm_args, b_info->extra[i]);
@@ -2218,14 +2225,6 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
assert(libxl_defbool_val(guest_config->b_info.device_model_stubdomain));
- if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
- if (d_state->saved_state) {
- LOG(ERROR, "Save/Restore not supported yet with Linux Stubdom.");
- ret = -1;
- goto out;
- }
- }
-
sdss->pvqemu.guest_domid = INVALID_DOMID;
libxl_domain_create_info_init(&dm_config->c_info);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index efaba91086..c394000ea9 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -962,6 +962,7 @@ static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
const libxl__json_object *response, int rc);
static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
const libxl__json_object *response, int rc);
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int fdset);
static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
const libxl__json_object *response, int rc);
@@ -994,10 +995,17 @@ static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
EGC_GC;
libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
const char *const filename = dsps->dm_savefile;
+ uint32_t dm_domid = libxl_get_stubdom_id(CTX, dsps->domid);
if (rc)
goto error;
+ if (dm_domid) {
+ /* see Linux stubdom interface in docs/stubdom.txt */
+ dm_state_save_to_fdset(egc, ev, 1);
+ return;
+ }
+
ev->payload_fd = open(filename, O_WRONLY | O_CREAT, 0600);
if (ev->payload_fd < 0) {
LOGED(ERROR, ev->domid,
@@ -1028,7 +1036,6 @@ static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
EGC_GC;
int fdset;
const libxl__json_object *o;
- libxl__json_object *args = NULL;
libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
close(ev->payload_fd);
@@ -1043,6 +1050,21 @@ static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
goto error;
}
fdset = libxl__json_object_get_integer(o);
+ dm_state_save_to_fdset(egc, ev, fdset);
+ return;
+
+error:
+ assert(rc);
+ libxl__remove_file(gc, dsps->dm_savefile);
+ dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int fdset)
+{
+ EGC_GC;
+ int rc;
+ libxl__json_object *args = NULL;
+ libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
ev->callback = dm_state_saved;
@@ -1060,7 +1082,8 @@ static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
error:
assert(rc);
- libxl__remove_file(gc, dsps->dm_savefile);
+ if (!libxl_get_stubdom_id(CTX, dsps->domid))
+ libxl__remove_file(gc, dsps->dm_savefile);
dsps->callback_device_model_done(egc, dsps, rc);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 10/21] tools: add missing libxenvchan cflags
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (8 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:35 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 11/21] tools: add simple vchan-socket-proxy Jason Andryuk
` (13 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, Marek Marczykowski-Górecki, Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
libxenvchan.h include xenevtchn.h and xengnttab.h, so applications built
with it needs applicable -I in CFLAGS too.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/Rules.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 9bac15c8d1..078eb7230a 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -159,7 +159,7 @@ SHDEPS_libxenstat = $(SHLIB_libxenctrl) $(SHLIB_libxenstore)
LDLIBS_libxenstat = $(SHDEPS_libxenstat) $(XEN_LIBXENSTAT)/libxenstat$(libextension)
SHLIB_libxenstat = $(SHDEPS_libxenstat) -Wl,-rpath-link=$(XEN_LIBXENSTAT)
-CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
+CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
SHDEPS_libxenvchan = $(SHLIB_libxentoollog) $(SHLIB_libxenstore) $(SHLIB_libxenevtchn) $(SHLIB_libxengnttab)
LDLIBS_libxenvchan = $(SHDEPS_libxenvchan) $(XEN_LIBVCHAN)/libxenvchan$(libextension)
SHLIB_libxenvchan = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 11/21] tools: add simple vchan-socket-proxy
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (9 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 10/21] tools: add missing libxenvchan cflags Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:37 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 12/21] libxl: use vchan for QMP access with Linux stubdomain Jason Andryuk
` (12 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, Marek Marczykowski-Górecki, Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Add a simple proxy for tunneling socket connection over vchan. This is
based on existing vchan-node* applications, but extended with socket
support. vchan-socket-proxy serves both as a client and as a server,
depending on parameters. It can be used to transparently communicate
with an application in another domian that normally expose UNIX socket
interface. Specifically, it's written to communicate with qemu running
within stubdom.
Server mode listens for vchan connections and when one is opened,
connects to a pointed UNIX socket. Client mode listens on UNIX
socket and when someone connects, opens a vchan connection. Only
a single connection at a time is supported.
Additionally, socket can be provided as a number - in which case it's
interpreted as already open FD (in case of UNIX listening socket -
listen() needs to be already called). Or "-" meaning stdin/stdout - in
which case it is reduced to vchan-node2 functionality.
Example usage:
1. (in dom0) vchan-socket-proxy --mode=client <DOMID>
/local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
2. (in DOMID) vchan-socket-proxy --mode=server 0
/local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
This will listen on /run/qemu.(DOMID) in dom0 and whenever connection is
made, it will connect to DOMID, where server process will connect to
/run/qemu.(DOMID) there. When client disconnects, vchan connection is
terminated and server vchan-socket-proxy process also disconnects from
qemu.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes on v5:
- Ensure bindir directory is present
- String and comment fixes
---
.gitignore | 1 +
tools/libvchan/Makefile | 8 +-
tools/libvchan/vchan-socket-proxy.c | 478 ++++++++++++++++++++++++++++
3 files changed, 486 insertions(+), 1 deletion(-)
create mode 100644 tools/libvchan/vchan-socket-proxy.c
diff --git a/.gitignore b/.gitignore
index 4ca679ddbc..1760e54464 100644
--- a/.gitignore
+++ b/.gitignore
@@ -368,6 +368,7 @@ tools/misc/xenwatchdogd
tools/misc/xen-hvmcrash
tools/misc/xen-lowmemd
tools/libvchan/vchan-node[12]
+tools/libvchan/vchan-socket-proxy
tools/ocaml/*/.ocamldep.make
tools/ocaml/*/*.cm[ixao]
tools/ocaml/*/*.cmxa
diff --git a/tools/libvchan/Makefile b/tools/libvchan/Makefile
index 7892750c3e..913bcc8884 100644
--- a/tools/libvchan/Makefile
+++ b/tools/libvchan/Makefile
@@ -13,6 +13,7 @@ LIBVCHAN_PIC_OBJS = $(patsubst %.o,%.opic,$(LIBVCHAN_OBJS))
LIBVCHAN_LIBS = $(LDLIBS_libxenstore) $(LDLIBS_libxengnttab) $(LDLIBS_libxenevtchn)
$(LIBVCHAN_OBJS) $(LIBVCHAN_PIC_OBJS): CFLAGS += $(CFLAGS_libxenstore) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
$(NODE_OBJS) $(NODE2_OBJS): CFLAGS += $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
+vchan-socket-proxy.o: CFLAGS += $(CFLAGS_libxenstore) $(CFLAGS_libxenctrl) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
MAJOR = 4.14
MINOR = 0
@@ -39,7 +40,7 @@ $(PKG_CONFIG_LOCAL): PKG_CONFIG_LIBDIR = $(CURDIR)
$(PKG_CONFIG_LOCAL): PKG_CONFIG_CFLAGS_LOCAL = $(CFLAGS_xeninclude)
.PHONY: all
-all: libxenvchan.so vchan-node1 vchan-node2 libxenvchan.a $(PKG_CONFIG_INST) $(PKG_CONFIG_LOCAL)
+all: libxenvchan.so vchan-node1 vchan-node2 vchan-socket-proxy libxenvchan.a $(PKG_CONFIG_INST) $(PKG_CONFIG_LOCAL)
libxenvchan.so: libxenvchan.so.$(MAJOR)
ln -sf $< $@
@@ -59,13 +60,18 @@ vchan-node1: $(NODE_OBJS) libxenvchan.so
vchan-node2: $(NODE2_OBJS) libxenvchan.so
$(CC) $(LDFLAGS) -o $@ $(NODE2_OBJS) $(LDLIBS_libxenvchan) $(APPEND_LDFLAGS)
+vchan-socket-proxy: vchan-socket-proxy.o libxenvchan.so
+ $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+
.PHONY: install
install: all
$(INSTALL_DIR) $(DESTDIR)$(libdir)
$(INSTALL_DIR) $(DESTDIR)$(includedir)
+ $(INSTALL_DIR) $(DESTDIR)$(bindir)
$(INSTALL_PROG) libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)
ln -sf libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)/libxenvchan.so.$(MAJOR)
ln -sf libxenvchan.so.$(MAJOR) $(DESTDIR)$(libdir)/libxenvchan.so
+ $(INSTALL_PROG) vchan-socket-proxy $(DESTDIR)$(bindir)
$(INSTALL_DATA) libxenvchan.h $(DESTDIR)$(includedir)
$(INSTALL_DATA) libxenvchan.a $(DESTDIR)$(libdir)
$(INSTALL_DATA) xenvchan.pc $(DESTDIR)$(PKG_INSTALLDIR)
diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
new file mode 100644
index 0000000000..13700c5d67
--- /dev/null
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -0,0 +1,478 @@
+/**
+ * @file
+ * @section AUTHORS
+ *
+ * Copyright (C) 2010 Rafal Wojtczuk <rafal@invisiblethingslab.com>
+ *
+ * Authors:
+ * Rafal Wojtczuk <rafal@invisiblethingslab.com>
+ * Daniel De Graaf <dgdegra@tycho.nsa.gov>
+ * Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
+ *
+ * @section LICENSE
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @section DESCRIPTION
+ *
+ * This is a vchan to unix socket proxy. Vchan server is set, and on client
+ * connection, local socket connection is established. Communication is bidirectional.
+ * One client is served at a time, clients needs to coordinate this themselves.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <getopt.h>
+
+#include <xenstore.h>
+#include <xenctrl.h>
+#include <libxenvchan.h>
+
+static void usage(char** argv)
+{
+ fprintf(stderr, "usage:\n"
+ "\t%s [options] domainid nodepath [socket-path|file-no|-]\n"
+ "\n"
+ "options:\n"
+ "\t-m, --mode=client|server - vchan connection mode (client by default)\n"
+ "\t-s, --state-path=path - xenstore path where write \"running\" to \n"
+ "\t at startup\n"
+ "\t-v, --verbose - verbose logging\n"
+ "\n"
+ "client: client of a vchan connection, fourth parameter can be:\n"
+ "\tsocket-path: listen on a UNIX socket at this path and connect to vchan\n"
+ "\t whenever new connection is accepted;\n"
+ "\t handle multiple _subsequent_ connections, until terminated\n"
+ "\n"
+ "\tfile-no: except open FD of a socket in listen mode;\n"
+ "\t otherwise similar to socket-path\n"
+ "\n"
+ "\t-: open vchan connection immediately and pass the data\n"
+ "\t from stdin/stdout; terminate when vchan connection\n"
+ "\t is closed\n"
+ "\n"
+ "server: server of a vchan connection, fourth parameter can be:\n"
+ "\tsocket-path: connect to this UNIX socket when new vchan connection\n"
+ "\t is accepted;\n"
+ "\t handle multiple _subsequent_ connections, until terminated\n"
+ "\n"
+ "\tfile-no: pass data to/from this FD; terminate when vchan connection\n"
+ "\t is closed\n"
+ "\n"
+ "\t-: pass data to/from stdin/stdout; terminate when vchan\n"
+ "\t connection is closed\n",
+ argv[0]);
+ exit(1);
+}
+
+#define BUFSIZE 8192
+char inbuf[BUFSIZE];
+char outbuf[BUFSIZE];
+int insiz = 0;
+int outsiz = 0;
+int verbose = 0;
+
+static void vchan_wr(struct libxenvchan *ctrl) {
+ int ret;
+
+ if (!insiz)
+ return;
+ ret = libxenvchan_write(ctrl, inbuf, insiz);
+ if (ret < 0) {
+ fprintf(stderr, "vchan write failed\n");
+ exit(1);
+ }
+ if (verbose)
+ fprintf(stderr, "wrote %d bytes to vchan\n", ret);
+ if (ret > 0) {
+ insiz -= ret;
+ memmove(inbuf, inbuf + ret, insiz);
+ }
+}
+
+static void socket_wr(int output_fd) {
+ int ret;
+
+ if (!outsiz)
+ return;
+ ret = write(output_fd, outbuf, outsiz);
+ if (ret < 0 && errno != EAGAIN)
+ exit(1);
+ if (ret > 0) {
+ outsiz -= ret;
+ memmove(outbuf, outbuf + ret, outsiz);
+ }
+}
+
+static int set_nonblocking(int fd, int nonblocking) {
+ int flags = fcntl(fd, F_GETFL);
+ if (flags == -1)
+ return -1;
+
+ if (nonblocking)
+ flags |= O_NONBLOCK;
+ else
+ flags &= ~O_NONBLOCK;
+
+ if (fcntl(fd, F_SETFL, flags) == -1)
+ return -1;
+
+ return 0;
+}
+
+static int connect_socket(const char *path_or_fd) {
+ int fd;
+ char *endptr;
+ struct sockaddr_un addr;
+
+ fd = strtoll(path_or_fd, &endptr, 0);
+ if (*endptr == '\0') {
+ set_nonblocking(fd, 1);
+ return fd;
+ }
+
+ fd = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (fd == -1)
+ return -1;
+
+ addr.sun_family = AF_UNIX;
+ strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
+ if (connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
+ close(fd);
+ return -1;
+ }
+
+ set_nonblocking(fd, 1);
+
+ return fd;
+}
+
+static int listen_socket(const char *path_or_fd) {
+ int fd;
+ char *endptr;
+ struct sockaddr_un addr;
+
+ fd = strtoll(path_or_fd, &endptr, 0);
+ if (*endptr == '\0') {
+ return fd;
+ }
+
+ /* if not a number, assume a socket path */
+ fd = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (fd == -1)
+ return -1;
+
+ addr.sun_family = AF_UNIX;
+ strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
+ if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
+ close(fd);
+ return -1;
+ }
+ if (listen(fd, 5) != 0) {
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+
+static struct libxenvchan *connect_vchan(int domid, const char *path) {
+ struct libxenvchan *ctrl = NULL;
+ struct xs_handle *xs = NULL;
+ xc_interface *xc = NULL;
+ xc_dominfo_t dominfo;
+ char **watch_ret;
+ unsigned int watch_num;
+ int ret;
+
+ xs = xs_open(XS_OPEN_READONLY);
+ if (!xs) {
+ perror("xs_open");
+ goto out;
+ }
+ xc = xc_interface_open(NULL, NULL, XC_OPENFLAG_NON_REENTRANT);
+ if (!xc) {
+ perror("xc_interface_open");
+ goto out;
+ }
+ /* wait for vchan server to create *path* */
+ xs_watch(xs, path, "path");
+ xs_watch(xs, "@releaseDomain", "release");
+ while ((watch_ret = xs_read_watch(xs, &watch_num))) {
+ /* don't care about exact which fired the watch */
+ free(watch_ret);
+ ctrl = libxenvchan_client_init(NULL, domid, path);
+ if (ctrl)
+ break;
+
+ ret = xc_domain_getinfo(xc, domid, 1, &dominfo);
+ /* break the loop if domain is definitely not there anymore, but
+ * continue if it is or the call failed (like EPERM) */
+ if (ret == -1 && errno == ESRCH)
+ break;
+ if (ret == 1 && (dominfo.domid != (uint32_t)domid || dominfo.dying))
+ break;
+ }
+
+out:
+ if (xc)
+ xc_interface_close(xc);
+ if (xs)
+ xs_close(xs);
+ return ctrl;
+}
+
+
+static void discard_buffers(struct libxenvchan *ctrl) {
+ /* discard local buffers */
+ insiz = 0;
+ outsiz = 0;
+
+ /* discard remaining incoming data */
+ while (libxenvchan_data_ready(ctrl)) {
+ if (libxenvchan_read(ctrl, inbuf, BUFSIZE) == -1) {
+ perror("vchan read");
+ exit(1);
+ }
+ }
+}
+
+int data_loop(struct libxenvchan *ctrl, int input_fd, int output_fd)
+{
+ int ret;
+ int libxenvchan_fd;
+ int max_fd;
+
+ libxenvchan_fd = libxenvchan_fd_for_select(ctrl);
+ for (;;) {
+ fd_set rfds;
+ fd_set wfds;
+ FD_ZERO(&rfds);
+ FD_ZERO(&wfds);
+
+ max_fd = -1;
+ if (input_fd != -1 && insiz != BUFSIZE) {
+ FD_SET(input_fd, &rfds);
+ if (input_fd > max_fd)
+ max_fd = input_fd;
+ }
+ if (output_fd != -1 && outsiz) {
+ FD_SET(output_fd, &wfds);
+ if (output_fd > max_fd)
+ max_fd = output_fd;
+ }
+ FD_SET(libxenvchan_fd, &rfds);
+ if (libxenvchan_fd > max_fd)
+ max_fd = libxenvchan_fd;
+ ret = select(max_fd + 1, &rfds, &wfds, NULL, NULL);
+ if (ret < 0) {
+ perror("select");
+ exit(1);
+ }
+ if (FD_ISSET(libxenvchan_fd, &rfds)) {
+ libxenvchan_wait(ctrl);
+ if (!libxenvchan_is_open(ctrl)) {
+ if (verbose)
+ fprintf(stderr, "vchan client disconnected\n");
+ while (outsiz)
+ socket_wr(output_fd);
+ close(output_fd);
+ close(input_fd);
+ discard_buffers(ctrl);
+ break;
+ }
+ vchan_wr(ctrl);
+ }
+
+ if (FD_ISSET(input_fd, &rfds)) {
+ ret = read(input_fd, inbuf + insiz, BUFSIZE - insiz);
+ if (ret < 0 && errno != EAGAIN)
+ exit(1);
+ if (verbose)
+ fprintf(stderr, "from-unix: %.*s\n", ret, inbuf + insiz);
+ if (ret == 0) {
+ /* EOF on socket, write everything in the buffer and close the
+ * input_fd socket */
+ while (insiz) {
+ vchan_wr(ctrl);
+ libxenvchan_wait(ctrl);
+ }
+ close(input_fd);
+ input_fd = -1;
+ /* TODO: maybe signal the vchan client somehow? */
+ break;
+ }
+ if (ret)
+ insiz += ret;
+ vchan_wr(ctrl);
+ }
+ if (FD_ISSET(output_fd, &wfds))
+ socket_wr(output_fd);
+ while (libxenvchan_data_ready(ctrl) && outsiz < BUFSIZE) {
+ ret = libxenvchan_read(ctrl, outbuf + outsiz, BUFSIZE - outsiz);
+ if (ret < 0)
+ exit(1);
+ if (verbose)
+ fprintf(stderr, "from-vchan: %.*s\n", ret, outbuf + outsiz);
+ outsiz += ret;
+ socket_wr(output_fd);
+ }
+ }
+ return 0;
+}
+
+/**
+ Simple libxenvchan application, both client and server.
+ Both sides may write and read, both from the libxenvchan and from
+ stdin/stdout (just like netcat).
+*/
+
+static struct option options[] = {
+ { "mode", required_argument, NULL, 'm' },
+ { "verbose", no_argument, NULL, 'v' },
+ { "state-path", required_argument, NULL, 's' },
+ { }
+};
+
+int main(int argc, char **argv)
+{
+ int is_server = 0;
+ int socket_fd = -1;
+ int input_fd, output_fd;
+ struct libxenvchan *ctrl = NULL;
+ const char *socket_path;
+ int domid;
+ const char *vchan_path;
+ const char *state_path = NULL;
+ int opt;
+
+ while ((opt = getopt_long(argc, argv, "m:vs:", options, NULL)) != -1) {
+ switch (opt) {
+ case 'm':
+ if (strcmp(optarg, "server") == 0)
+ is_server = 1;
+ else if (strcmp(optarg, "client") == 0)
+ is_server = 0;
+ else {
+ fprintf(stderr, "invalid argument for --mode: %s\n", optarg);
+ usage(argv);
+ return 1;
+ }
+ break;
+ case 'v':
+ verbose = 1;
+ break;
+ case 's':
+ state_path = optarg;
+ break;
+ case '?':
+ usage(argv);
+ }
+ }
+
+ if (argc-optind != 3)
+ usage(argv);
+
+ domid = atoi(argv[optind]);
+ vchan_path = argv[optind+1];
+ socket_path = argv[optind+2];
+
+ if (is_server) {
+ ctrl = libxenvchan_server_init(NULL, domid, vchan_path, 0, 0);
+ if (!ctrl) {
+ perror("libxenvchan_server_init");
+ exit(1);
+ }
+ } else {
+ if (strcmp(socket_path, "-") == 0) {
+ input_fd = 0;
+ output_fd = 1;
+ } else {
+ socket_fd = listen_socket(socket_path);
+ if (socket_fd == -1) {
+ perror("listen socket");
+ return 1;
+ }
+ }
+ }
+
+ if (state_path) {
+ struct xs_handle *xs;
+
+ xs = xs_open(0);
+ if (!xs) {
+ perror("xs_open");
+ return 1;
+ }
+ if (!xs_write(xs, XBT_NULL, state_path, "running", strlen("running"))) {
+ perror("xs_write");
+ return 1;
+ }
+ xs_close(xs);
+ }
+
+ for (;;) {
+ if (is_server) {
+ /* wait for vchan connection */
+ while (libxenvchan_is_open(ctrl) != 1)
+ libxenvchan_wait(ctrl);
+ /* vchan client connected, setup local FD if needed */
+ if (strcmp(socket_path, "-") == 0) {
+ input_fd = 0;
+ output_fd = 1;
+ } else {
+ input_fd = output_fd = connect_socket(socket_path);
+ }
+ if (input_fd == -1) {
+ perror("connect socket");
+ return 1;
+ }
+ if (data_loop(ctrl, input_fd, output_fd) != 0)
+ break;
+ /* keep it running only when get UNIX socket path */
+ if (socket_path[0] != '/')
+ break;
+ } else {
+ /* wait for local socket connection */
+ if (strcmp(socket_path, "-") != 0)
+ input_fd = output_fd = accept(socket_fd, NULL, NULL);
+ if (input_fd == -1) {
+ perror("accept");
+ return 1;
+ }
+ set_nonblocking(input_fd, 1);
+ set_nonblocking(output_fd, 1);
+ ctrl = connect_vchan(domid, vchan_path);
+ if (!ctrl) {
+ perror("vchan client init");
+ return 1;
+ }
+ if (data_loop(ctrl, input_fd, output_fd) != 0)
+ break;
+ /* don't reconnect if output was stdout */
+ if (strcmp(socket_path, "-") == 0)
+ break;
+
+ libxenvchan_close(ctrl);
+ ctrl = NULL;
+ }
+ }
+ return 0;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 12/21] libxl: use vchan for QMP access with Linux stubdomain
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (10 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 11/21] tools: add simple vchan-socket-proxy Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:39 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 13/21] Regenerate autotools files Jason Andryuk
` (11 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Access to QMP of QEMU in Linux stubdomain is possible over vchan
connection. Handle the actual vchan connection in a separate process
(vchan-socket-proxy). This simplified integration with QMP (already
quite complex), but also allows preliminary filtering of (potentially
malicious) QMP input.
Since only one client can be connected to vchan server at the same time
and it is not enforced by the libxenvchan itself, additional client-side
locking is needed. It is implicitly implemented by vchan-socket-proxy,
as it handle only one connection at a time. Note that qemu supports only
one simultaneous client on a control socket anyway (but in UNIX socket
case, it enforce it server-side), so it doesn't add any extra
limitation.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Changes in v4:
- new patch, in place of both "libxl: use vchan for QMP access ..."
Changes in v5:
- Use device-model/%u/qmp-proxy-state xenstore path
- Rephrase comment
---
tools/configure.ac | 9 ++
tools/libxl/libxl_dm.c | 163 +++++++++++++++++++++++++++++++++--
tools/libxl/libxl_internal.h | 1 +
3 files changed, 165 insertions(+), 8 deletions(-)
diff --git a/tools/configure.ac b/tools/configure.ac
index ea0272766f..1c0ed4c3b1 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -192,6 +192,15 @@ AC_SUBST(qemu_xen)
AC_SUBST(qemu_xen_path)
AC_SUBST(qemu_xen_systemd)
+AC_ARG_WITH([stubdom-qmp-proxy],
+ AC_HELP_STRING([--stubdom-qmp-proxy@<:@=PATH@:>@],
+ [Use supplied binary PATH as a QMP proxy into stubdomain]),[
+ stubdom_qmp_proxy="$withval"
+],[
+ stubdom_qmp_proxy="$bindir/vchan-socket-proxy"
+])
+AC_DEFINE_UNQUOTED([STUBDOM_QMP_PROXY_PATH], ["$stubdom_qmp_proxy"], [QMP proxy path])
+
AC_ARG_WITH([system-seabios],
AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@],
[Use system supplied seabios PATH instead of building and installing
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 45d0dd56f5..e420c3fc7b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1200,7 +1200,11 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
GCSPRINTF("%d", guest_domid), NULL);
flexarray_append(dm_args, "-no-shutdown");
- /* There is currently no way to access the QMP socket in the stubdom */
+ /*
+ * QMP access to qemu running in stubdomain is done over vchan. The
+ * stubdomain init script adds the appropriate monitor options for
+ * vchan-socket-proxy.
+ */
if (!is_stubdom) {
flexarray_append(dm_args, "-chardev");
if (state->dm_monitor_fd >= 0) {
@@ -2194,6 +2198,23 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc,
static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
int rc, const char *p);
+static void spawn_qmp_proxy(libxl__egc *egc,
+ libxl__stub_dm_spawn_state *sdss);
+
+static void qmp_proxy_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+ const char *xsdata);
+
+static void qmp_proxy_startup_failed(libxl__egc *egc,
+ libxl__spawn_state *spawn,
+ int rc);
+
+static void qmp_proxy_detached(libxl__egc *egc,
+ libxl__spawn_state *spawn);
+
+static void qmp_proxy_spawn_outcome(libxl__egc *egc,
+ libxl__stub_dm_spawn_state *sdss,
+ int rc);
+
char *libxl__stub_dm_name(libxl__gc *gc, const char *guest_name)
{
return GCSPRINTF("%s-dm", guest_name);
@@ -2476,24 +2497,150 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
goto out;
}
+ sdss->qmp_proxy_spawn.ao = ao;
+ if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
+ spawn_qmp_proxy(egc, sdss);
+ } else {
+ qmp_proxy_spawn_outcome(egc, sdss, 0);
+ }
+
+ return;
+
+out:
+ assert(ret);
+ qmp_proxy_spawn_outcome(egc, sdss, ret);
+}
+
+static void spawn_qmp_proxy(libxl__egc *egc,
+ libxl__stub_dm_spawn_state *sdss)
+{
+ STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
+ const uint32_t guest_domid = sdss->dm.guest_domid;
+ const uint32_t dm_domid = sdss->pvqemu.guest_domid;
+ const char *dom_path = libxl__xs_get_dompath(gc, dm_domid);
+ char **args;
+ int nr = 0;
+ int rc, logfile_w, null;
+
+ if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) {
+ LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid);
+ sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path);
+ sdss->qmp_proxy_spawn.xspath = DEVICE_MODEL_XS_PATH(gc, LIBXL_TOOLSTACK_DOMID,
+ dm_domid, "/qmp-proxy-state");
+ sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+ sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid;
+ sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm;
+ sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed;
+ sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached;
+
+ const int arraysize = 6;
+ GCNEW_ARRAY(args, arraysize);
+ args[nr++] = STUBDOM_QMP_PROXY_PATH;
+ args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath);
+ args[nr++] = GCSPRINTF("%u", dm_domid);
+ args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid);
+ args[nr++] = (char*)libxl__qemu_qmp_path(gc, guest_domid);
+ args[nr++] = NULL;
+ assert(nr == arraysize);
+
+ logfile_w = libxl__create_qemu_logfile(gc, GCSPRINTF("qmp-proxy-%s",
+ sdss->dm_config.c_info.name));
+ if (logfile_w < 0) {
+ rc = logfile_w;
+ goto out;
+ }
+ null = open("/dev/null", O_RDWR);
+ if (null < 0) {
+ LOGED(ERROR, guest_domid, "unable to open /dev/null");
+ rc = ERROR_FAIL;
+ goto out_close;
+ }
+
+ rc = libxl__spawn_spawn(egc, &sdss->qmp_proxy_spawn);
+ if (rc < 0)
+ goto out_close;
+ if (!rc) { /* inner child */
+ setsid();
+ libxl__exec(gc, null, null, logfile_w, STUBDOM_QMP_PROXY_PATH, args, NULL);
+ /* unreachable */
+ }
+
+ rc = 0;
+
+out_close:
+ if (logfile_w >= 0)
+ close(logfile_w);
+ if (null >= 0)
+ close(null);
+out:
+ if (rc)
+ qmp_proxy_spawn_outcome(egc, sdss, rc);
+}
+
+static void qmp_proxy_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+ const char *xsdata)
+{
+ STATE_AO_GC(spawn->ao);
+
+ if (!xsdata)
+ return;
+
+ if (strcmp(xsdata, "running"))
+ return;
+
+ libxl__spawn_initiate_detach(gc, spawn);
+}
+
+static void qmp_proxy_startup_failed(libxl__egc *egc,
+ libxl__spawn_state *spawn,
+ int rc)
+{
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(spawn, *sdss, qmp_proxy_spawn);
+ qmp_proxy_spawn_outcome(egc, sdss, rc);
+}
+
+static void qmp_proxy_detached(libxl__egc *egc,
+ libxl__spawn_state *spawn)
+{
+ libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(spawn, *sdss, qmp_proxy_spawn);
+ qmp_proxy_spawn_outcome(egc, sdss, 0);
+}
+
+static void qmp_proxy_spawn_outcome(libxl__egc *egc,
+ libxl__stub_dm_spawn_state *sdss,
+ int rc)
+{
+ STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
+ int need_pvqemu = libxl__need_xenpv_qemu(gc, &sdss->dm_config);
+
+ if (rc) goto out;
+
+ if (need_pvqemu < 0) {
+ rc = need_pvqemu;
+ goto out;
+ }
+
sdss->pvqemu.spawn.ao = ao;
- sdss->pvqemu.guest_domid = dm_domid;
sdss->pvqemu.guest_config = &sdss->dm_config;
sdss->pvqemu.build_state = &sdss->dm_state;
sdss->pvqemu.callback = spawn_stubdom_pvqemu_cb;
-
- if (!need_qemu) {
+ if (need_pvqemu) {
+ libxl__spawn_local_dm(egc, &sdss->pvqemu);
+ } else {
/* If dom0 qemu not needed, do not launch it */
spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, 0);
- } else {
- libxl__spawn_local_dm(egc, &sdss->pvqemu);
}
return;
out:
- assert(ret);
- spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
+ assert(rc);
+ spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, rc);
}
static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a8f0eed945..00cfbe1fac 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4159,6 +4159,7 @@ typedef struct {
libxl__destroy_domid_state dis;
libxl__multidev multidev;
libxl__xswait_state xswait;
+ libxl__spawn_state qmp_proxy_spawn;
} libxl__stub_dm_spawn_state;
_hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 13/21] Regenerate autotools files
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (11 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 12/21] libxl: use vchan for QMP access with Linux stubdomain Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:41 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use Jason Andryuk
` (10 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Ian Jackson, Jason Andryuk, Marek Marczykowski-Górecki,
Wei Liu, Samuel Thibault
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Since we have those generated files committed to the repo (why?!),
update them after changing configure.ac.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
configure | 14 +-------------
docs/configure | 14 +-------------
stubdom/configure | 14 +-------------
tools/config.h.in | 3 +++
tools/configure | 46 ++++++++++++++++++++++++++++------------------
5 files changed, 34 insertions(+), 57 deletions(-)
diff --git a/configure b/configure
index 9da3970cef..8af54e8a5a 100755
--- a/configure
+++ b/configure
@@ -644,7 +644,6 @@ infodir
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -723,7 +722,6 @@ datadir='${datarootdir}'
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -976,15 +974,6 @@ do
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
- -runstatedir | --runstatedir | --runstatedi | --runstated \
- | --runstate | --runstat | --runsta | --runst | --runs \
- | --run | --ru | --r)
- ac_prev=runstatedir ;;
- -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
- | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
- | --run=* | --ru=* | --r=*)
- runstatedir=$ac_optarg ;;
-
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1122,7 +1111,7 @@ fi
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
- libdir localedir mandir runstatedir
+ libdir localedir mandir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
@@ -1275,7 +1264,6 @@ Fine tuning of the installation directories:
--sysconfdir=DIR read-only single-machine data [PREFIX/etc]
--sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
- --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIR object code libraries [EPREFIX/lib]
--includedir=DIR C header files [PREFIX/include]
--oldincludedir=DIR C header files for non-gcc [/usr/include]
diff --git a/docs/configure b/docs/configure
index 9e3ed60462..93e9dcf404 100755
--- a/docs/configure
+++ b/docs/configure
@@ -634,7 +634,6 @@ infodir
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -711,7 +710,6 @@ datadir='${datarootdir}'
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -964,15 +962,6 @@ do
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
- -runstatedir | --runstatedir | --runstatedi | --runstated \
- | --runstate | --runstat | --runsta | --runst | --runs \
- | --run | --ru | --r)
- ac_prev=runstatedir ;;
- -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
- | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
- | --run=* | --ru=* | --r=*)
- runstatedir=$ac_optarg ;;
-
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1110,7 +1099,7 @@ fi
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
- libdir localedir mandir runstatedir
+ libdir localedir mandir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
@@ -1263,7 +1252,6 @@ Fine tuning of the installation directories:
--sysconfdir=DIR read-only single-machine data [PREFIX/etc]
--sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
- --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIR object code libraries [EPREFIX/lib]
--includedir=DIR C header files [PREFIX/include]
--oldincludedir=DIR C header files for non-gcc [/usr/include]
diff --git a/stubdom/configure b/stubdom/configure
index da03da535a..f7604a37f7 100755
--- a/stubdom/configure
+++ b/stubdom/configure
@@ -661,7 +661,6 @@ infodir
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -751,7 +750,6 @@ datadir='${datarootdir}'
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1004,15 +1002,6 @@ do
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
- -runstatedir | --runstatedir | --runstatedi | --runstated \
- | --runstate | --runstat | --runsta | --runst | --runs \
- | --run | --ru | --r)
- ac_prev=runstatedir ;;
- -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
- | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
- | --run=* | --ru=* | --r=*)
- runstatedir=$ac_optarg ;;
-
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1150,7 +1139,7 @@ fi
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
- libdir localedir mandir runstatedir
+ libdir localedir mandir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
@@ -1303,7 +1292,6 @@ Fine tuning of the installation directories:
--sysconfdir=DIR read-only single-machine data [PREFIX/etc]
--sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
- --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIR object code libraries [EPREFIX/lib]
--includedir=DIR C header files [PREFIX/include]
--oldincludedir=DIR C header files for non-gcc [/usr/include]
diff --git a/tools/config.h.in b/tools/config.h.in
index 5a5944ebe1..5abf6092de 100644
--- a/tools/config.h.in
+++ b/tools/config.h.in
@@ -123,6 +123,9 @@
/* Define to 1 if you have the ANSI C header files. */
#undef STDC_HEADERS
+/* QMP proxy path */
+#undef STUBDOM_QMP_PROXY_PATH
+
/* Enable large inode numbers on Mac OS X 10.5. */
#ifndef _DARWIN_USE_64_BIT_INODE
# define _DARWIN_USE_64_BIT_INODE 1
diff --git a/tools/configure b/tools/configure
index 4fa5f7b937..fef684f0be 100755
--- a/tools/configure
+++ b/tools/configure
@@ -770,7 +770,6 @@ infodir
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -811,6 +810,7 @@ with_linux_backend_modules
enable_qemu_traditional
enable_rombios
with_system_qemu
+with_stubdom_qmp_proxy
with_system_seabios
with_system_ovmf
enable_ipxe
@@ -896,7 +896,6 @@ datadir='${datarootdir}'
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1149,15 +1148,6 @@ do
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
- -runstatedir | --runstatedir | --runstatedi | --runstated \
- | --runstate | --runstat | --runsta | --runst | --runs \
- | --run | --ru | --r)
- ac_prev=runstatedir ;;
- -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
- | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
- | --run=* | --ru=* | --r=*)
- runstatedir=$ac_optarg ;;
-
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1295,7 +1285,7 @@ fi
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
- libdir localedir mandir runstatedir
+ libdir localedir mandir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
@@ -1448,7 +1438,6 @@ Fine tuning of the installation directories:
--sysconfdir=DIR read-only single-machine data [PREFIX/etc]
--sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
- --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIR object code libraries [EPREFIX/lib]
--includedir=DIR C header files [PREFIX/include]
--oldincludedir=DIR C header files for non-gcc [/usr/include]
@@ -1531,6 +1520,9 @@ Optional Packages:
Use system supplied qemu PATH or qemu (taken from
$PATH) as qemu-xen device model instead of building
and installing our own version
+ --stubdom-qmp-proxy[=PATH]
+ Use supplied binary PATH as a QMP proxy into
+ stubdomain
--with-system-seabios[=PATH]
Use system supplied seabios PATH instead of building
and installing our own version
@@ -3378,7 +3370,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3424,7 +3416,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3448,7 +3440,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3493,7 +3485,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -3517,7 +3509,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -4519,6 +4511,24 @@ _ACEOF
+# Check whether --with-stubdom-qmp-proxy was given.
+if test "${with_stubdom_qmp_proxy+set}" = set; then :
+ withval=$with_stubdom_qmp_proxy;
+ stubdom_qmp_proxy="$withval"
+
+else
+
+ stubdom_qmp_proxy="$bindir/vchan-socket-proxy"
+
+fi
+
+
+cat >>confdefs.h <<_ACEOF
+#define STUBDOM_QMP_PROXY_PATH "$stubdom_qmp_proxy"
+_ACEOF
+
+
+
# Check whether --with-system-seabios was given.
if test "${with_system_seabios+set}" = set; then :
withval=$with_system_seabios;
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (12 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 13/21] Regenerate autotools files Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:42 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 15/21] libxl: ignore emulated IDE disks beyond the first 4 Jason Andryuk
` (9 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Until xenconsoled learns how to handle multiple consoles, this is needed
for save/restore support (qemu state is transferred over secondary
consoles).
Additionally, Linux-based stubdomain waits for all the backends to
initialize during boot. Lack of some console backends results in
stubdomain startup timeout.
This is a temporary patch until xenconsoled will be improved.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/libxl/libxl_dm.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index e420c3fc7b..5e5e7a27b3 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2484,7 +2484,11 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
}
}
- need_qemu = libxl__need_xenpv_qemu(gc, dm_config);
+ /*
+ * Until xenconsoled learns how to handle multiple consoles, require qemu
+ * in dom0 to serve consoles for a stubdomain - it require at least 3 of them.
+ */
+ need_qemu = 1 || libxl__need_xenpv_qemu(gc, &sdss->dm_config);
for (i = 0; i < num_console; i++) {
libxl__device device;
@@ -2616,7 +2620,11 @@ static void qmp_proxy_spawn_outcome(libxl__egc *egc,
int rc)
{
STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
- int need_pvqemu = libxl__need_xenpv_qemu(gc, &sdss->dm_config);
+ /*
+ * Until xenconsoled learns how to handle multiple consoles, require qemu
+ * in dom0 to serve consoles for a stubdomain - it require at least 3 of them.
+ */
+ int need_pvqemu = 1 || libxl__need_xenpv_qemu(gc, &sdss->dm_config);
if (rc) goto out;
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 15/21] libxl: ignore emulated IDE disks beyond the first 4
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (13 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:43 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 16/21] libxl: consider also qemu in stubdomain in libxl__dm_active check Jason Andryuk
` (8 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Qemu supports only 4 emulated IDE disks, when given more (or with higher
indexes), it will fail to start. Since the disks can still be accessible
using PV interface, just ignore emulated path and log a warning, instead
of rejecting the configuration altogether.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/libxl/libxl_dm.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5e5e7a27b3..03d7a38f1f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1891,6 +1891,13 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
}
if (disks[i].is_cdrom) {
+ if (disk > 4) {
+ LOGD(WARN, guest_domid, "Emulated CDROM can be only one of the first 4 disks.\n"
+ "Disk %s will be available via PV drivers but not as an "
+ "emulated disk.",
+ disks[i].vdev);
+ continue;
+ }
drive = libxl__sprintf(gc,
"if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
disk, dev_number);
@@ -1968,6 +1975,10 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
&disks[i],
colo_mode);
} else {
+ LOGD(WARN, guest_domid, "Only 4 emulated IDE disks are supported.\n"
+ "Disk %s will be available via PV drivers but not as an "
+ "emulated disk.",
+ disks[i].vdev);
continue; /* Do not emulate this disk */
}
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 16/21] libxl: consider also qemu in stubdomain in libxl__dm_active check
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (14 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 15/21] libxl: ignore emulated IDE disks beyond the first 4 Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:43 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 17/21] docs: Add device-model-domid to xenstore-paths Jason Andryuk
` (7 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki,
Wei Liu, Jason Andryuk
From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Since qemu-xen can now run in stubdomain too, handle this case when
checking it's state too.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/libxl/libxl_dm.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 03d7a38f1f..5d61da1de8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -3749,12 +3749,18 @@ out:
int libxl__dm_active(libxl__gc *gc, uint32_t domid)
{
- char *pid, *path;
+ char *pid, *dm_domid, *path;
path = GCSPRINTF("/local/domain/%d/image/device-model-pid", domid);
pid = libxl__xs_read(gc, XBT_NULL, path);
- return pid != NULL;
+ if (pid)
+ return true;
+
+ path = GCSPRINTF("/local/domain/%d/image/device-model-domid", domid);
+ dm_domid = libxl__xs_read(gc, XBT_NULL, path);
+
+ return dm_domid != NULL;
}
int libxl__dm_check_start(libxl__gc *gc, libxl_domain_config *d_config,
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 17/21] docs: Add device-model-domid to xenstore-paths
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (15 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 16/21] libxl: consider also qemu in stubdomain in libxl__dm_active check Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:44 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 18/21] libxl: Check stubdomain kernel & ramdisk presence Jason Andryuk
` (6 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Jason Andryuk,
Andrew Cooper, George Dunlap, Jan Beulich, Ian Jackson
Document device-model-domid for when using a device model stubdomain.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
docs/misc/xenstore-paths.pandoc | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc
index a152f5ea68..766e8008dc 100644
--- a/docs/misc/xenstore-paths.pandoc
+++ b/docs/misc/xenstore-paths.pandoc
@@ -148,6 +148,11 @@ The domain's own ID.
The process ID of the device model associated with this domain, if it
has one.
+#### ~/image/device-model-domid = INTEGER [INTERNAL]
+
+The domain ID of the device model stubdomain associated with this domain,
+if it has one.
+
#### ~/cpu/[0-9]+/availability = ("online"|"offline") [PV]
One node for each virtual CPU up to the guest's configured
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 18/21] libxl: Check stubdomain kernel & ramdisk presence
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (16 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 17/21] docs: Add device-model-domid to xenstore-paths Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:45 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 19/21] libxl: Refactor kill_device_model to libxl__kill_xs_path Jason Andryuk
` (5 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu, Jason Andryuk
Just out of context is the following comment for libxl__domain_make:
/* fixme: this function can leak the stubdom if it fails */
When the stubdomain kernel or ramdisk is not present, the domid and
stubdomain name will indeed be leaked. Avoid the leak by checking the
file presence and erroring out when absent. It doesn't fix all cases,
but it avoids a big one when using a linux device model stubdomain.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/libxl/libxl_dm.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5d61da1de8..a57c13bdf4 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2316,6 +2316,22 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
dm_config->num_vkbs = 1;
}
+ if (guest_config->b_info.stubdomain_kernel &&
+ access(guest_config->b_info.stubdomain_kernel, R_OK) != 0) {
+ LOGED(ERROR, guest_domid, "could not access stubdomain kernel %s",
+ guest_config->b_info.stubdomain_kernel);
+ ret = ERROR_INVAL;
+ goto out;
+ }
+
+ if (guest_config->b_info.stubdomain_ramdisk &&
+ access(guest_config->b_info.stubdomain_ramdisk, R_OK) != 0) {
+ LOGED(ERROR, guest_domid, "could not access stubdomain ramdisk %s",
+ guest_config->b_info.stubdomain_ramdisk);
+ ret = ERROR_INVAL;
+ goto out;
+ }
+
stubdom_state->pv_kernel.path = guest_config->b_info.stubdomain_kernel;
stubdom_state->pv_ramdisk.path = guest_config->b_info.stubdomain_ramdisk;
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 19/21] libxl: Refactor kill_device_model to libxl__kill_xs_path
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (17 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 18/21] libxl: Check stubdomain kernel & ramdisk presence Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:45 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 20/21] libxl: Kill vchan-socket-proxy when cleaning up qmp Jason Andryuk
` (4 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu, Jason Andryuk
Move kill_device_model to libxl__kill_xs_path so we have a helper to
kill a process from a pid stored in xenstore. We'll be using it to kill
vchan-qmp-proxy.
libxl__kill_xs_path takes a "what" string for use in printing error
messages. kill_device_model is retained in libxl_dm.c to provide the
string.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/libxl/libxl_aoutils.c | 32 ++++++++++++++++++++++++++++++++
tools/libxl/libxl_dm.c | 27 +--------------------------
tools/libxl/libxl_internal.h | 3 +++
3 files changed, 36 insertions(+), 26 deletions(-)
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 1be858c93c..c4c095a5ba 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -626,6 +626,38 @@ void libxl__kill(libxl__gc *gc, pid_t pid, int sig, const char *what)
what, (unsigned long)pid, sig);
}
+/* Generic function to signal (HUP) a pid stored in xenstore */
+int libxl__kill_xs_path(libxl__gc *gc, const char *xs_path_pid,
+ const char *what)
+{
+ const char *xs_pid;
+ int ret, pid;
+
+ ret = libxl__xs_read_checked(gc, XBT_NULL, xs_path_pid, &xs_pid);
+ if (ret || !xs_pid) {
+ LOG(ERROR, "unable to find %s pid in %s", what, xs_path_pid);
+ ret = ret ? : ERROR_FAIL;
+ goto out;
+ }
+ pid = atoi(xs_pid);
+
+ ret = kill(pid, SIGHUP);
+ if (ret < 0 && errno == ESRCH) {
+ LOG(ERROR, "%s already exited", what);
+ ret = 0;
+ } else if (ret == 0) {
+ LOG(DEBUG, "%s signaled", what);
+ ret = 0;
+ } else {
+ LOGE(ERROR, "failed to kill %s [%d]", what, pid);
+ ret = ERROR_FAIL;
+ goto out;
+ }
+
+out:
+ return ret;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a57c13bdf4..10ca4226ba 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -3397,32 +3397,7 @@ out:
/* Generic function to signal a Qemu instance to exit */
static int kill_device_model(libxl__gc *gc, const char *xs_path_pid)
{
- const char *xs_pid;
- int ret, pid;
-
- ret = libxl__xs_read_checked(gc, XBT_NULL, xs_path_pid, &xs_pid);
- if (ret || !xs_pid) {
- LOG(ERROR, "unable to find device model pid in %s", xs_path_pid);
- ret = ret ? : ERROR_FAIL;
- goto out;
- }
- pid = atoi(xs_pid);
-
- ret = kill(pid, SIGHUP);
- if (ret < 0 && errno == ESRCH) {
- LOG(ERROR, "Device Model already exited");
- ret = 0;
- } else if (ret == 0) {
- LOG(DEBUG, "Device Model signaled");
- ret = 0;
- } else {
- LOGE(ERROR, "failed to kill Device Model [%d]", pid);
- ret = ERROR_FAIL;
- goto out;
- }
-
-out:
- return ret;
+ return libxl__kill_xs_path(gc, xs_path_pid, "Device Model");
}
/* Helper to destroy a Qdisk backend */
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 00cfbe1fac..32aa797714 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2707,6 +2707,9 @@ int libxl__async_exec_start(libxl__async_exec_state *aes);
bool libxl__async_exec_inuse(const libxl__async_exec_state *aes);
_hidden void libxl__kill(libxl__gc *gc, pid_t pid, int sig, const char *what);
+/* kill SIGHUP a pid stored in xenstore */
+_hidden int libxl__kill_xs_path(libxl__gc *gc, const char *xs_path_pid,
+ const char *what);
/*----- device addition/removal -----*/
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 20/21] libxl: Kill vchan-socket-proxy when cleaning up qmp
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (18 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 19/21] libxl: Refactor kill_device_model to libxl__kill_xs_path Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:47 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 21/21] tools: Clean up vchan-socket-proxy socket Jason Andryuk
` (3 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel; +Cc: Anthony PERARD, Ian Jackson, Wei Liu, Jason Andryuk
We need to kill the vchan-socket-proxy so we don't leak the daemonized
processes. libxl__stubdomain_is_linux_running works against the
guest_domid, but the xenstore path is beneath the stubdomain. This
leads to the use of libxl_is_stubdom in addition to
libxl__stubdomain_is_linux_running so that the stubdomain calls kill for
the qmp-proxy
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
libxl__qmp_cleanup was considered, but it is not called for guests with
a stubdomain.
---
tools/libxl/libxl_domain.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index fef2cd4e13..3b66e25aa7 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1260,10 +1260,17 @@ static void dm_destroy_cb(libxl__egc *egc,
libxl__destroy_domid_state *dis = CONTAINER_OF(ddms, *dis, ddms);
STATE_AO_GC(dis->ao);
uint32_t domid = dis->domid;
+ uint32_t target_domid;
if (rc < 0)
LOGD(ERROR, domid, "libxl__destroy_device_model failed");
+ if (libxl_is_stubdom(CTX, domid, &target_domid) &&
+ libxl__stubdomain_is_linux_running(gc, target_domid)) {
+ char *path = GCSPRINTF("/local/domain/%d/image/qmp-proxy-pid", domid);
+ libxl__kill_xs_path(gc, path, "QMP Proxy");
+ }
+
dis->drs.ao = ao;
dis->drs.domid = domid;
dis->drs.callback = devices_destroy_cb;
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH v5 21/21] tools: Clean up vchan-socket-proxy socket
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (19 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 20/21] libxl: Kill vchan-socket-proxy when cleaning up qmp Jason Andryuk
@ 2020-04-28 4:04 ` Jason Andryuk
2020-05-14 16:48 ` Ian Jackson
2020-05-11 20:19 ` [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (2 subsequent siblings)
23 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-04-28 4:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Wei Liu, Jason Andryuk
To avoid socket files lingering in /run/xen, have vchan-socket-proxy
clean up the sockets it creates. Use a signal handler as well as atexit
to handle both means of termination.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
tools/libvchan/vchan-socket-proxy.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 13700c5d67..0fb42964b5 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -33,6 +33,7 @@
#include <stdlib.h>
#include <stdio.h>
+#include <signal.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
@@ -88,6 +89,22 @@ char outbuf[BUFSIZE];
int insiz = 0;
int outsiz = 0;
int verbose = 0;
+char *cleanup_socket;
+
+static void cleanup(void)
+{
+ if (cleanup_socket) {
+ unlink(cleanup_socket);
+ free(cleanup_socket);
+ cleanup_socket = NULL;
+ }
+}
+
+static void cleanup_exit(int signum)
+{
+ cleanup();
+ exit(0);
+}
static void vchan_wr(struct libxenvchan *ctrl) {
int ret;
@@ -394,6 +411,9 @@ int main(int argc, char **argv)
vchan_path = argv[optind+1];
socket_path = argv[optind+2];
+ signal(SIGHUP, cleanup_exit);
+ signal(SIGTERM, cleanup_exit);
+
if (is_server) {
ctrl = libxenvchan_server_init(NULL, domid, vchan_path, 0, 0);
if (!ctrl) {
@@ -410,6 +430,8 @@ int main(int argc, char **argv)
perror("listen socket");
return 1;
}
+ cleanup_socket = strdup(socket_path);
+ atexit(cleanup);
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (20 preceding siblings ...)
2020-04-28 4:04 ` [PATCH v5 21/21] tools: Clean up vchan-socket-proxy socket Jason Andryuk
@ 2020-05-11 20:19 ` Jason Andryuk
2020-05-14 16:07 ` Ian Jackson
2020-05-14 16:55 ` Ian Jackson
23 siblings, 0 replies; 55+ messages in thread
From: Jason Andryuk @ 2020-05-11 20:19 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
George Dunlap, Marek Marczykowski-Górecki, Simon Gaiser,
Jan Beulich, Samuel Thibault, Anthony PERARD, Ian Jackson,
Eric Shelton
Ping?
-Jason
On Tue, Apr 28, 2020 at 12:05 AM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> Hi,
>
> In coordination with Marek, I'm making a submission of his patches for Linux
> stubdomain device-model support. I made a few of my own additions, but Marek
> did the heavy lifting. Thank you, Marek.
>
> Below is mostly the v4 cover leter with a few additions.
>
> 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.
>
> First two patches add documentation about expected toolstack-stubdomain-qemu
> interface, both for MiniOS stubdomain and Linux stubdomain.
>
> Initial version has no QMP support - in initial patches it is completely
> disabled, which means no suspend/restore and no PCI passthrough.
>
> Later patches add QMP over libvchan connection support. The actual connection
> is made in a separate process. As discussed on Xen Summit 2019, this allows to
> apply some basic checks and/or filtering (not part of this series), to limit
> libxl exposure for potentially malicious stubdomain.
>
> Jason's additions ensure the qmp-proxy (vchan-socket-proxy) processes and
> sockets are cleaned up and add some documentation.
>
> The actual stubdomain implementation is here:
>
> https://github.com/marmarek/qubes-vmm-xen-stubdom-linux
> (branch for-upstream, tag for-upstream-v3)
>
> See readme there for build instructions. Marek's version requires dracut. I
> have hacked up a version usable install with initramfs-tools:
>
> https://github.com/jandryuk/qubes-vmm-xen-stubdom-linux
> (branch initramfs-tools)
>
> Few comments/questions about the stubdomain code:
>
> 1. There are extra patches for qemu that are necessary to run it in stubdomain.
> While it is desirable to upstream them, I think it can be done after merging
> libxl part. Stubdomain's qemu build will in most cases be separate anyway, to
> limit qemu's dependencies (so the stubdomain size).
>
> 2. By default Linux hvc-xen console frontend is unreliable for data transfer
> (qemu state save/restore) - it drops data sent faster than client is reading
> it. To fix it, console device needs to be switched into raw mode (`stty raw
> /dev/hvc1`). Especially for restoring qemu state it is tricky, as it would need
> to be done before opening the device, but stty (obviously) needs to open the
> device first. To solve this problem, for now the repository contains kernel
> patch which changes the default for all hvc consoles. Again, this isn't
> practical problem, as the kernel for stubdomain is built separately. But it
> would be nice to have something working with vanilla kernel. I see those
> options:
> - convert it to kernel cmdline parameter (hvc_console_raw=1 ?)
> - use channels instead of consoles (and on the kernel side change the default
> to "raw" only for channels); while in theory better design, libxl part will
> be more complex, as channels can be connected to sockets but not files, so
> libxl would need to read/write to it exactly when qemu write/read the data,
> not before/after as it is done now
>
> 3. Mini-OS stubdoms use dmargs xenstore key as a string. Linux stubdoms use
> dmargs as a directory for numbered entries. Should they be different names?
>
> Remaining parts for eliminating dom0's instance of qemu:
> - do not force QDISK backend for CDROM
> - multiple consoles support in xenconsoled
>
> Changes in v2:
> - apply review comments by Jason Andryuk
> Changes in v3:
> - rework qemu arguments handling (separate xenstore keys, instead of \x1b separator)
> - add QMP over libvchan, instead of console
> - add protocol documentation
> - a lot of minor changes, see individual patches for full changes list
> - split xenconsoled patches into separate series
> Changes in v4:
> - extract vchan connection into a separate process
> - rebase on master
> - various fixes
> Changes in v5:
> - Marek: apply review comments from Jason Andryuk
> - Jason: Clean up qmp-proxy processes and sockets
>
> Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Cc: Simon Gaiser <simon@invisiblethingslab.com>
> Cc: Eric Shelton <eshelton@pobox.com>
> Cc: Ian Jackson <ian.jackson@citrix.com>
> Cc: Wei Liu <wl@xen.org>
>
> Eric Shelton (1):
> libxl: Handle Linux stubdomain specific QEMU options.
>
> Jason Andryuk (5):
> docs: Add device-model-domid to xenstore-paths
> libxl: Check stubdomain kernel & ramdisk presence
> libxl: Refactor kill_device_model to libxl__kill_xs_path
> libxl: Kill vchan-socket-proxy when cleaning up qmp
> tools: Clean up vchan-socket-proxy socket
>
> Marek Marczykowski-Górecki (15):
> Document ioemu MiniOS stubdomain protocol
> Document ioemu Linux stubdomain protocol
> libxl: fix qemu-trad cmdline for no sdl/vnc case
> libxl: Allow running qemu-xen in stubdomain
> libxl: write qemu arguments into separate xenstore keys
> xl: add stubdomain related options to xl config parser
> tools/libvchan: notify server when client is connected
> libxl: add save/restore support for qemu-xen in stubdomain
> tools: add missing libxenvchan cflags
> tools: add simple vchan-socket-proxy
> libxl: use vchan for QMP access with Linux stubdomain
> Regenerate autotools files
> libxl: require qemu in dom0 even if stubdomain is in use
> libxl: ignore emulated IDE disks beyond the first 4
> libxl: consider also qemu in stubdomain in libxl__dm_active check
>
> .gitignore | 1 +
> configure | 14 +-
> docs/configure | 14 +-
> docs/man/xl.cfg.5.pod.in | 27 +-
> docs/misc/stubdom.txt | 103 ++++++
> docs/misc/xenstore-paths.pandoc | 5 +
> stubdom/configure | 14 +-
> tools/Rules.mk | 2 +-
> tools/config.h.in | 3 +
> tools/configure | 46 ++-
> tools/configure.ac | 9 +
> tools/libvchan/Makefile | 8 +-
> tools/libvchan/init.c | 3 +
> tools/libvchan/vchan-socket-proxy.c | 500 ++++++++++++++++++++++++++++
> tools/libxl/libxl_aoutils.c | 32 ++
> tools/libxl/libxl_create.c | 46 ++-
> tools/libxl/libxl_dm.c | 484 +++++++++++++++++++++------
> tools/libxl/libxl_domain.c | 7 +
> tools/libxl/libxl_internal.h | 22 ++
> tools/libxl/libxl_mem.c | 6 +-
> tools/libxl/libxl_qmp.c | 27 +-
> tools/libxl/libxl_types.idl | 3 +
> tools/xl/xl_parse.c | 7 +
> 23 files changed, 1205 insertions(+), 178 deletions(-)
> create mode 100644 tools/libvchan/vchan-socket-proxy.c
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (21 preceding siblings ...)
2020-05-11 20:19 ` [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
@ 2020-05-14 16:07 ` Ian Jackson
2020-05-14 16:55 ` Ian Jackson
23 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:07 UTC (permalink / raw)
To: Jason Andryuk
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
George Dunlap, Marek Marczykowski-Górecki, Simon Gaiser,
Jan Beulich, Samuel Thibault, Anthony Perard, xen-devel,
Eric Shelton
Jason Andryuk writes ("[PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain"):
> In coordination with Marek, I'm making a submission of his patches for Linux
> stubdomain device-model support. I made a few of my own additions, but Marek
> did the heavy lifting. Thank you, Marek.
Hi. Thanks very much for this contribution. Sorry it has taken me so
long to get to review it.
> Later patches add QMP over libvchan connection support. The actual connection
> is made in a separate process. As discussed on Xen Summit 2019, this allows to
> apply some basic checks and/or filtering (not part of this series), to limit
> libxl exposure for potentially malicious stubdomain.
OK.
> Few comments/questions about the stubdomain code:
>
> 1. There are extra patches for qemu that are necessary to run it in stubdomain.
> While it is desirable to upstream them, I think it can be done after merging
> libxl part. Stubdomain's qemu build will in most cases be separate anyway, to
> limit qemu's dependencies (so the stubdomain size).
Yes.
> 2. By default Linux hvc-xen console frontend is unreliable for data transfer
> (qemu state save/restore) - it drops data sent faster than client is reading
> it. To fix it, console device needs to be switched into raw mode (`stty raw
> /dev/hvc1`). Especially for restoring qemu state it is tricky, as it would need
> to be done before opening the device, but stty (obviously) needs to open the
> device first. To solve this problem, for now the repository contains kernel
> patch which changes the default for all hvc consoles. Again, this isn't
> practical problem, as the kernel for stubdomain is built separately. But it
> would be nice to have something working with vanilla kernel. I see those
> options:
> - convert it to kernel cmdline parameter (hvc_console_raw=1 ?)
> - use channels instead of consoles (and on the kernel side change the default
> to "raw" only for channels); while in theory better design, libxl part will
> be more complex, as channels can be connected to sockets but not files, so
> libxl would need to read/write to it exactly when qemu write/read the data,
> not before/after as it is done now
What a mess. Thanks for trying to tackle it!
Would it be possible to add a rendenzvous to the console ? Eg, the
guest could write a "ready" byte after it has set the mode.
I'm not sure I understand the problem with libxl and channels. Maybe
a helper process (perhaps existing only during migration) could help ?
Or, libxl has the "datacopier" async thing in it which you can spawn
one of and hopefully forget about. You could teach it channels, or
make a thing like it that uses channels, or something.
> 3. Mini-OS stubdoms use dmargs xenstore key as a string. Linux stubdoms use
> dmargs as a directory for numbered entries. Should they be different names?
Yes, I think so. That way if there's a version mismatch you get
ENOENT rather than an empty argument list...
I'll go and look at the patches now.
Regards,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol
2020-04-28 4:04 ` [PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol Jason Andryuk
@ 2020-05-14 16:08 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:08 UTC (permalink / raw)
To: Jason Andryuk
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
George Dunlap, Marek Marczykowski-Górecki, Jan Beulich,
xen-devel
Jason Andryuk writes ("[PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Add documentation based on reverse-engineered toolstack-ioemu stubdomain
> protocol.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
NB I have not reviewed this for correctness. I don't think I know the
protocol well enough ...
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 02/21] Document ioemu Linux stubdomain protocol
2020-04-28 4:04 ` [PATCH v5 02/21] Document ioemu Linux " Jason Andryuk
@ 2020-05-14 16:08 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:08 UTC (permalink / raw)
To: Jason Andryuk
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
George Dunlap, Marek Marczykowski-Górecki, Jan Beulich,
xen-devel
Jason Andryuk writes ("[PATCH v5 02/21] Document ioemu Linux stubdomain protocol"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Add documentation for upcoming Linux stubdomain for qemu-upstream.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
This probably shouldn't be committed except with the implmeentation
:-).
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 04/21] libxl: Allow running qemu-xen in stubdomain
2020-04-28 4:04 ` [PATCH v5 04/21] libxl: Allow running qemu-xen in stubdomain Jason Andryuk
@ 2020-05-14 16:10 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:10 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 04/21] libxl: Allow running qemu-xen in stubdomain"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Do not prohibit anymore using stubdomain with qemu-xen.
> To help distingushing MiniOS and Linux stubdomain, add helper inline
> functions libxl__stubdomain_is_linux() and
> libxl__stubdomain_is_linux_running(). Those should be used where really
> the difference is about MiniOS/Linux, not qemu-xen/qemu-xen-traditional.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options.
2020-04-28 4:04 ` [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options Jason Andryuk
@ 2020-05-14 16:19 ` Ian Jackson
2020-05-17 13:55 ` Jason Andryuk
0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:19 UTC (permalink / raw)
To: Jason Andryuk
Cc: Wei Liu, Marek Marczykowski-Górecki, Simon Gaiser,
Anthony Perard, xen-devel, Eric Shelton
Jason Andryuk writes ("[PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options."):
> From: Eric Shelton <eshelton@pobox.com>
>
> This patch creates an appropriate command line for the QEMU instance
> running in a Linux-based stubdomain.
>
> NOTE: a number of items are not currently implemented for Linux-based
> stubdomains, such as:
> - save/restore
> - QMP socket
> - graphics output (e.g., VNC)
>
> Signed-off-by: Eric Shelton <eshelton@pobox.com>
>
> Simon:
> * fix disk path
> * fix cdrom path and "format"
>
> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
> [drop Qubes-specific parts]
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Nice work all.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
This is despite me spotting three tiny style nits:
> @@ -1312,7 +1316,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
> }
>
> flexarray_append(dm_args, vncarg);
> - } else
> + } else if (!is_stubdom)
> /*
> * Ensure that by default no vnc server is created.
> */
While you are here it would be nice to regularise the { }. (libxl
CODING_STYLE says that all branches of an if should have { }, if any
of them do.)
> @@ -1974,8 +2006,10 @@ static int libxl__build_device_model_args(libxl__gc *gc,
> args, envs,
> state);
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> - assert(dm_state_fd != NULL);
> - assert(*dm_state_fd < 0);
> + if (!libxl_defbool_val(guest_config->b_info.device_model_stubdomain)) {
> + assert(dm_state_fd != NULL);
> + assert(*dm_state_fd < 0);
> + }
This } seems to be misindented ?
> if (guest_config->b_info.u.hvm.serial)
> num_console++;
> + else if (guest_config->b_info.u.hvm.serial_list) {
> + char **serial = guest_config->b_info.u.hvm.serial_list;
> + while (*(serial++))
> + num_console++;
> + }
You should add the { } areound the if block too.
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys
2020-04-28 4:04 ` [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys Jason Andryuk
@ 2020-05-14 16:25 ` Ian Jackson
2020-05-17 14:29 ` Jason Andryuk
0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:25 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys"):
> +static int libxl__write_stub_linux_dmargs(libxl__gc *gc,
> + int dm_domid, int guest_domid,
> + char **args)
> +{
...
> + vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/vm", guest_domid));
> + path = GCSPRINTF("%s/image/dmargs", vm_path);
> +
> +retry_transaction:
> + t = xs_transaction_start(ctx->xsh);
> + xs_write(ctx->xsh, t, path, "", 0);
> + xs_set_permissions(ctx->xsh, t, path, roperm, ARRAY_SIZE(roperm));
This wants to be libxl__xs_mknod I think ?
> + i = 1;
> + for (i=1; args[i] != NULL; i++)
> + xs_write(ctx->xsh, t, GCSPRINTF("%s/%03d", path, i), args[i], strlen(args[i]));
Can you do this with libxl__xs_transaction_* please, and a loop rather
than a goto ?
Why not use libxl__xs_write_checked ?
> + xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path), roperm, ARRAY_SIZE(roperm));
This line seems out of place. At least, it is not mentioned in the
commit message. If it's needed, can you please split it out - and, of
course, then, provide an explanation :-).
> + if (!xs_transaction_end(ctx->xsh, t, 0))
> + if (errno == EAGAIN)
> + goto retry_transaction;
> + return 0;
Thanks,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 07/21] xl: add stubdomain related options to xl config parser
2020-04-28 4:04 ` [PATCH v5 07/21] xl: add stubdomain related options to xl config parser Jason Andryuk
@ 2020-05-14 16:26 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:26 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 07/21] xl: add stubdomain related options to xl config parser"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 08/21] tools/libvchan: notify server when client is connected
2020-04-28 4:04 ` [PATCH v5 08/21] tools/libvchan: notify server when client is connected Jason Andryuk
@ 2020-05-14 16:27 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:27 UTC (permalink / raw)
To: Jason Andryuk
Cc: xen-devel, Daniel De Graaf, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 08/21] tools/libvchan: notify server when client is connected"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Let the server know when the client is connected. Otherwise server will
> notice only when client send some data.
> This change does not break existing clients, as libvchan user should
> handle spurious notifications anyway (for example acknowledge of remote
> side reading the data).
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Replace spaces with tabs to match the file's whitespace.
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
> Marek: I had this patch in Qubes for a long time and totally forgot it
> wasn't upstream thing...
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
CCing Daniel De Graaf in case he feels like having an opinion....
Thanks,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain
2020-04-28 4:04 ` [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain Jason Andryuk
@ 2020-05-14 16:35 ` Ian Jackson
2020-05-17 13:55 ` Jason Andryuk
0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:35 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki,
Wei Liu, Ian Jackson
Jason Andryuk writes ("[PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
> relevant consoles.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Address TODO in dm_state_save_to_fdset: Only remove savefile for
> non-stubdom.
...
> + if (is_stubdom) {
> + /* Linux stubdomain connects specific FD to STUBDOM_CONSOLE_RESTORE
> + */
> + flexarray_append(dm_args, "-incoming");
> + flexarray_append(dm_args, "fd:3");
Would it be possible to use a different fixed fd number ? Low numbers
are particularly vulnerable to clashes with autoallocated numbers.
I suggest randomly allocating one in the range [64,192>. My random
number generator picked 119. So 118 and 119 ?
Also, why couldn't your wrapper script add this argument ? If you do
that there then there is one place that knows the fd number and a
slightly less tortuous linkage between libxl and the script...
It's not stated anywhere here that I can see but I think what is
happening here is that your wrapper script knows the qemu savefile
pathname and reads it directly. Maybbe a comment would be
worthwhile ?
The code looks like a good implementation of what it does, though.
Regards,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 10/21] tools: add missing libxenvchan cflags
2020-04-28 4:04 ` [PATCH v5 10/21] tools: add missing libxenvchan cflags Jason Andryuk
@ 2020-05-14 16:35 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:35 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 10/21] tools: add missing libxenvchan cflags"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> libxenvchan.h include xenevtchn.h and xengnttab.h, so applications built
> with it needs applicable -I in CFLAGS too.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 11/21] tools: add simple vchan-socket-proxy
2020-04-28 4:04 ` [PATCH v5 11/21] tools: add simple vchan-socket-proxy Jason Andryuk
@ 2020-05-14 16:37 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:37 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 11/21] tools: add simple vchan-socket-proxy"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Add a simple proxy for tunneling socket connection over vchan. This is
> based on existing vchan-node* applications, but extended with socket
> support. vchan-socket-proxy serves both as a client and as a server,
> depending on parameters. It can be used to transparently communicate
> with an application in another domian that normally expose UNIX socket
> interface. Specifically, it's written to communicate with qemu running
> within stubdom.
>
> Server mode listens for vchan connections and when one is opened,
> connects to a pointed UNIX socket. Client mode listens on UNIX
> socket and when someone connects, opens a vchan connection. Only
> a single connection at a time is supported.
>
> Additionally, socket can be provided as a number - in which case it's
> interpreted as already open FD (in case of UNIX listening socket -
> listen() needs to be already called). Or "-" meaning stdin/stdout - in
> which case it is reduced to vchan-node2 functionality.
>
> Example usage:
>
> 1. (in dom0) vchan-socket-proxy --mode=client <DOMID>
> /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
>
> 2. (in DOMID) vchan-socket-proxy --mode=server 0
> /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
>
> This will listen on /run/qemu.(DOMID) in dom0 and whenever connection is
> made, it will connect to DOMID, where server process will connect to
> /run/qemu.(DOMID) there. When client disconnects, vchan connection is
> terminated and server vchan-socket-proxy process also disconnects from
> qemu.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
I have not reviewed this. It sounds very useful. Thank you!
(I did skim over it.)
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 12/21] libxl: use vchan for QMP access with Linux stubdomain
2020-04-28 4:04 ` [PATCH v5 12/21] libxl: use vchan for QMP access with Linux stubdomain Jason Andryuk
@ 2020-05-14 16:39 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:39 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 12/21] libxl: use vchan for QMP access with Linux stubdomain"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Access to QMP of QEMU in Linux stubdomain is possible over vchan
> connection. Handle the actual vchan connection in a separate process
> (vchan-socket-proxy). This simplified integration with QMP (already
> quite complex), but also allows preliminary filtering of (potentially
> malicious) QMP input.
> Since only one client can be connected to vchan server at the same time
> and it is not enforced by the libxenvchan itself, additional client-side
> locking is needed. It is implicitly implemented by vchan-socket-proxy,
> as it handle only one connection at a time. Note that qemu supports only
> one simultaneous client on a control socket anyway (but in UNIX socket
> case, it enforce it server-side), so it doesn't add any extra
> limitation.
You might mention in your commit message libxl qmp client code already
has locking to avoid attempts to talk concurrently to the same qemu.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Thanks,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 13/21] Regenerate autotools files
2020-04-28 4:04 ` [PATCH v5 13/21] Regenerate autotools files Jason Andryuk
@ 2020-05-14 16:41 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:41 UTC (permalink / raw)
To: Jason Andryuk
Cc: xen-devel, Marek Marczykowski-Górecki, Wei Liu, Samuel Thibault
Jason Andryuk writes ("[PATCH v5 13/21] Regenerate autotools files"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Since we have those generated files committed to the repo (why?!),
> update them after changing configure.ac.
This should be folded into the patch that changes the input. Can you
please add a note to the commit message instructing the committer to
rerun autotools.
We have these files in-tree because not everyone has a recent
autotools.
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use
2020-04-28 4:04 ` [PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use Jason Andryuk
@ 2020-05-14 16:42 ` Ian Jackson
2020-05-14 17:36 ` Marek Marczykowski-Górecki
0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:42 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Until xenconsoled learns how to handle multiple consoles, this is needed
> for save/restore support (qemu state is transferred over secondary
> consoles).
> Additionally, Linux-based stubdomain waits for all the backends to
> initialize during boot. Lack of some console backends results in
> stubdomain startup timeout.
>
> This is a temporary patch until xenconsoled will be improved.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
> tools/libxl/libxl_dm.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index e420c3fc7b..5e5e7a27b3 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -2484,7 +2484,11 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
> }
> }
>
> - need_qemu = libxl__need_xenpv_qemu(gc, dm_config);
> + /*
> + * Until xenconsoled learns how to handle multiple consoles, require qemu
> + * in dom0 to serve consoles for a stubdomain - it require at least 3 of them.
> + */
> + need_qemu = 1 || libxl__need_xenpv_qemu(gc, &sdss->dm_config);
But I don't think this is true for a trad non-linux stubdm ?
So I think this ought to be conditional.
What am I missing ?
Regards,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 15/21] libxl: ignore emulated IDE disks beyond the first 4
2020-04-28 4:04 ` [PATCH v5 15/21] libxl: ignore emulated IDE disks beyond the first 4 Jason Andryuk
@ 2020-05-14 16:43 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:43 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 15/21] libxl: ignore emulated IDE disks beyond the first 4"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Qemu supports only 4 emulated IDE disks, when given more (or with higher
> indexes), it will fail to start. Since the disks can still be accessible
> using PV interface, just ignore emulated path and log a warning, instead
> of rejecting the configuration altogether.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 16/21] libxl: consider also qemu in stubdomain in libxl__dm_active check
2020-04-28 4:04 ` [PATCH v5 16/21] libxl: consider also qemu in stubdomain in libxl__dm_active check Jason Andryuk
@ 2020-05-14 16:43 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:43 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("[PATCH v5 16/21] libxl: consider also qemu in stubdomain in libxl__dm_active check"):
> From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Since qemu-xen can now run in stubdomain too, handle this case when
> checking it's state too.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 17/21] docs: Add device-model-domid to xenstore-paths
2020-04-28 4:04 ` [PATCH v5 17/21] docs: Add device-model-domid to xenstore-paths Jason Andryuk
@ 2020-05-14 16:44 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:44 UTC (permalink / raw)
To: Jason Andryuk
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
George Dunlap, Jan Beulich, xen-devel
Jason Andryuk writes ("[PATCH v5 17/21] docs: Add device-model-domid to xenstore-paths"):
> Document device-model-domid for when using a device model stubdomain.
>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 18/21] libxl: Check stubdomain kernel & ramdisk presence
2020-04-28 4:04 ` [PATCH v5 18/21] libxl: Check stubdomain kernel & ramdisk presence Jason Andryuk
@ 2020-05-14 16:45 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:45 UTC (permalink / raw)
To: Jason Andryuk; +Cc: Anthony Perard, xen-devel, Wei Liu
Jason Andryuk writes ("[PATCH v5 18/21] libxl: Check stubdomain kernel & ramdisk presence"):
> Just out of context is the following comment for libxl__domain_make:
> /* fixme: this function can leak the stubdom if it fails */
>
> When the stubdomain kernel or ramdisk is not present, the domid and
> stubdomain name will indeed be leaked. Avoid the leak by checking the
> file presence and erroring out when absent. It doesn't fix all cases,
> but it avoids a big one when using a linux device model stubdomain.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 19/21] libxl: Refactor kill_device_model to libxl__kill_xs_path
2020-04-28 4:04 ` [PATCH v5 19/21] libxl: Refactor kill_device_model to libxl__kill_xs_path Jason Andryuk
@ 2020-05-14 16:45 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:45 UTC (permalink / raw)
To: Jason Andryuk; +Cc: Anthony Perard, xen-devel, Wei Liu
Jason Andryuk writes ("[PATCH v5 19/21] libxl: Refactor kill_device_model to libxl__kill_xs_path"):
> Move kill_device_model to libxl__kill_xs_path so we have a helper to
> kill a process from a pid stored in xenstore. We'll be using it to kill
> vchan-qmp-proxy.
>
> libxl__kill_xs_path takes a "what" string for use in printing error
> messages. kill_device_model is retained in libxl_dm.c to provide the
> string.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 20/21] libxl: Kill vchan-socket-proxy when cleaning up qmp
2020-04-28 4:04 ` [PATCH v5 20/21] libxl: Kill vchan-socket-proxy when cleaning up qmp Jason Andryuk
@ 2020-05-14 16:47 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:47 UTC (permalink / raw)
To: Jason Andryuk; +Cc: Anthony Perard, xen-devel, Wei Liu
Jason Andryuk writes ("[PATCH v5 20/21] libxl: Kill vchan-socket-proxy when cleaning up qmp"):
> We need to kill the vchan-socket-proxy so we don't leak the daemonized
> processes. libxl__stubdomain_is_linux_running works against the
> guest_domid, but the xenstore path is beneath the stubdomain. This
> leads to the use of libxl_is_stubdom in addition to
> libxl__stubdomain_is_linux_running so that the stubdomain calls kill for
> the qmp-proxy
In theory maybe this patch should be folded into the one that
introduces the vchan proxy. But since this whole mode of operation is
new, having a point in the history where it leaks these is OK I
think.
Do others agree ?
For my part,
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
However, if possible maybe this patch could be moved to right after
the one which spawns the proxy ?
Thanks,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 21/21] tools: Clean up vchan-socket-proxy socket
2020-04-28 4:04 ` [PATCH v5 21/21] tools: Clean up vchan-socket-proxy socket Jason Andryuk
@ 2020-05-14 16:48 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:48 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Wei Liu
Jason Andryuk writes ("[PATCH v5 21/21] tools: Clean up vchan-socket-proxy socket"):
> To avoid socket files lingering in /run/xen, have vchan-socket-proxy
> clean up the sockets it creates. Use a signal handler as well as atexit
> to handle both means of termination.
This should be done in [lib]xl destroy, not here. That way if the
proxy crashes or something it will also get cleaned up.
Thanks,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
` (22 preceding siblings ...)
2020-05-14 16:07 ` Ian Jackson
@ 2020-05-14 16:55 ` Ian Jackson
2020-05-14 19:10 ` Jason Andryuk
23 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2020-05-14 16:55 UTC (permalink / raw)
To: Jason Andryuk
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Paul Durrant,
Andrew Cooper, George Dunlap, Marek Marczykowski-Górecki,
Simon Gaiser, Jan Beulich, Samuel Thibault, Anthony Perard,
xen-devel, Eric Shelton
Jason Andryuk writes ("[PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain"):
> In coordination with Marek, I'm making a submission of his patches for Linux
> stubdomain device-model support. I made a few of my own additions, but Marek
> did the heavy lifting. Thank you, Marek.
Hi. I finished reading these patches. Thank you very much. They
were nicely structured. I found them clear and easy to read. As
you'll have seen I have requested only a few changes.
I am very hopeful that this series will make 4.14. Codefreeze is
Friday the 22nd of May. Please let us know whether you think we'll
all be able to make that...
Regards,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use
2020-05-14 16:42 ` Ian Jackson
@ 2020-05-14 17:36 ` Marek Marczykowski-Górecki
0 siblings, 0 replies; 55+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-05-14 17:36 UTC (permalink / raw)
To: Ian Jackson; +Cc: Anthony Perard, xen-devel, Wei Liu, Jason Andryuk
[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]
On Thu, May 14, 2020 at 05:42:36PM +0100, Ian Jackson wrote:
> Jason Andryuk writes ("[PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use"):
> > From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> >
> > Until xenconsoled learns how to handle multiple consoles, this is needed
> > for save/restore support (qemu state is transferred over secondary
> > consoles).
> > Additionally, Linux-based stubdomain waits for all the backends to
> > initialize during boot. Lack of some console backends results in
> > stubdomain startup timeout.
> >
> > This is a temporary patch until xenconsoled will be improved.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> > ---
> > tools/libxl/libxl_dm.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index e420c3fc7b..5e5e7a27b3 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -2484,7 +2484,11 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
> > }
> > }
> >
> > - need_qemu = libxl__need_xenpv_qemu(gc, dm_config);
> > + /*
> > + * Until xenconsoled learns how to handle multiple consoles, require qemu
> > + * in dom0 to serve consoles for a stubdomain - it require at least 3 of them.
> > + */
> > + need_qemu = 1 || libxl__need_xenpv_qemu(gc, &sdss->dm_config);
>
> But I don't think this is true for a trad non-linux stubdm ?
> So I think this ought to be conditional.
For qemu-trad is true too. Stubdomain (mini-os + qemu-trad and linux +
qemu-xen) is always started with at least 3 consoles: log, save,
restore. Which currently requires qemu in dom0. So, yes, technically it
is a bug in the current libxl for qemu-trad. In practice, it works in
most cases because there is something else that triggers qemu in dom0
too: vfb/vkb added if vnc/sdl/spice is enabled.
--
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain
2020-05-14 16:55 ` Ian Jackson
@ 2020-05-14 19:10 ` Jason Andryuk
0 siblings, 0 replies; 55+ messages in thread
From: Jason Andryuk @ 2020-05-14 19:10 UTC (permalink / raw)
To: Ian Jackson
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Paul Durrant,
Andrew Cooper, George Dunlap, Marek Marczykowski-Górecki,
Simon Gaiser, Jan Beulich, Samuel Thibault, Anthony Perard,
xen-devel, Eric Shelton
On Thu, May 14, 2020 at 12:55 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Jason Andryuk writes ("[PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain"):
> > In coordination with Marek, I'm making a submission of his patches for Linux
> > stubdomain device-model support. I made a few of my own additions, but Marek
> > did the heavy lifting. Thank you, Marek.
>
> Hi. I finished reading these patches. Thank you very much. They
> were nicely structured. I found them clear and easy to read. As
> you'll have seen I have requested only a few changes.
I haven't taken a look yet, but thanks for the review.
Marek deserves credit for the structuring and work.
> I am very hopeful that this series will make 4.14. Codefreeze is
> Friday the 22nd of May. Please let us know whether you think we'll
> all be able to make that...
Yes, I'm aiming for 4.14. I plan to re-spin and re-post over the new
couple days.
Regards,
Jason
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options.
2020-05-14 16:19 ` Ian Jackson
@ 2020-05-17 13:55 ` Jason Andryuk
0 siblings, 0 replies; 55+ messages in thread
From: Jason Andryuk @ 2020-05-17 13:55 UTC (permalink / raw)
To: Ian Jackson
Cc: Wei Liu, Marek Marczykowski-Górecki, Simon Gaiser,
Anthony Perard, xen-devel, Eric Shelton
On Thu, May 14, 2020 at 12:19 PM Ian Jackson <ian.jackson@citrix.com> wrote:
> Jason Andryuk writes ("[PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options."):
> > @@ -1974,8 +2006,10 @@ static int libxl__build_device_model_args(libxl__gc *gc,
> > args, envs,
> > state);
> > case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> > - assert(dm_state_fd != NULL);
> > - assert(*dm_state_fd < 0);
> > + if (!libxl_defbool_val(guest_config->b_info.device_model_stubdomain)) {
> > + assert(dm_state_fd != NULL);
> > + assert(*dm_state_fd < 0);
> > + }
>
> This } seems to be misindented ?
This was a stray tab. Fixed along with the { } changes.
Thanks,
Jason
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain
2020-05-14 16:35 ` Ian Jackson
@ 2020-05-17 13:55 ` Jason Andryuk
2020-05-18 14:15 ` Ian Jackson
0 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-05-17 13:55 UTC (permalink / raw)
To: Ian Jackson
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
On Thu, May 14, 2020 at 12:35 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Jason Andryuk writes ("[PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> > From: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> >
> > Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
> > relevant consoles.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > Address TODO in dm_state_save_to_fdset: Only remove savefile for
> > non-stubdom.
> ...
> > + if (is_stubdom) {
> > + /* Linux stubdomain connects specific FD to STUBDOM_CONSOLE_RESTORE
> > + */
> > + flexarray_append(dm_args, "-incoming");
> > + flexarray_append(dm_args, "fd:3");
>
> Would it be possible to use a different fixed fd number ? Low numbers
> are particularly vulnerable to clashes with autoallocated numbers.
>
> I suggest randomly allocating one in the range [64,192>. My random
> number generator picked 119. So 118 and 119 ?
This makes sense and would be the easiest change.
> Also, why couldn't your wrapper script add this argument ? If you do
> that there then there is one place that knows the fd number and a
> slightly less tortuous linkage between libxl and the script...
I like this idea, but there is a complication. "-incoming" is only
added when performing a restore, so it cannot just be blindly added to
all qemu command lines in the stubdom. Two options I see are to
either communicate a restore some other way (so the stubdom scripts
can add the appropriate option), or pass something though dm_args, but
let the script convert it into something usable.
There is "-incoming defer" where we can later specify
"migrate_incoming fd:119". Another option is to `sed
s/defer/fd:119/`, but that is a little tricky since we need to look at
the preceding key to know if we should sed the second. We could pass
only "-incoming" and require the stubdom script to modify that option.
I haven't tested any of this.
> It's not stated anywhere here that I can see but I think what is
> happening here is that your wrapper script knows the qemu savefile
> pathname and reads it directly. Maybbe a comment would be
> worthwhile ?
The existing comment "Linux stubdomain connects specific FD to
STUBDOM_CONSOLE_RESTORE" is trying to state that.
STUBDOM_CONSOLE_RESTORE is defined as 2 for console 2 (/dev/hvc2), but
it is only a libxl_internal.h define.
Regards,
Jason
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys
2020-05-14 16:25 ` Ian Jackson
@ 2020-05-17 14:29 ` Jason Andryuk
0 siblings, 0 replies; 55+ messages in thread
From: Jason Andryuk @ 2020-05-17 14:29 UTC (permalink / raw)
To: Ian Jackson
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
On Thu, May 14, 2020 at 12:25 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Jason Andryuk writes ("[PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys"):
> > + xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path), roperm, ARRAY_SIZE(roperm));
>
> This line seems out of place. At least, it is not mentioned in the
> commit message. If it's needed, can you please split it out - and, of
> course, then, provide an explanation :-).
This was copied over from the mini-os version. It looks like it is
unused by qemu-xen, so it can be dropped.
Thanks,
Jason
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain
2020-05-17 13:55 ` Jason Andryuk
@ 2020-05-18 14:15 ` Ian Jackson
2020-05-18 14:50 ` Marek Marczykowski-Górecki
0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2020-05-18 14:15 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> On Thu, May 14, 2020 at 12:35 PM Ian Jackson <ian.jackson@citrix.com> wrote:
> > I suggest randomly allocating one in the range [64,192>. My random
> > number generator picked 119. So 118 and 119 ?
>
> This makes sense and would be the easiest change.
Cool.
> > Also, why couldn't your wrapper script add this argument ? If you do
> > that there then there is one place that knows the fd number and a
> > slightly less tortuous linkage between libxl and the script...
>
> I like this idea, but there is a complication. "-incoming" is only
> added when performing a restore, so it cannot just be blindly added to
> all qemu command lines in the stubdom. Two options I see are to
> either communicate a restore some other way (so the stubdom scripts
> can add the appropriate option), or pass something though dm_args, but
> let the script convert it into something usable.
>
> There is "-incoming defer" where we can later specify
> "migrate_incoming fd:119". Another option is to `sed
> s/defer/fd:119/`, but that is a little tricky since we need to look at
> the preceding key to know if we should sed the second. We could pass
> only "-incoming" and require the stubdom script to modify that option.
>
> I haven't tested any of this.
Erk. I see now why you did it the way you did !
> > It's not stated anywhere here that I can see but I think what is
> > happening here is that your wrapper script knows the qemu savefile
> > pathname and reads it directly. Maybbe a comment would be
> > worthwhile ?
>
> The existing comment "Linux stubdomain connects specific FD to
> STUBDOM_CONSOLE_RESTORE" is trying to state that.
> STUBDOM_CONSOLE_RESTORE is defined as 2 for console 2 (/dev/hvc2), but
> it is only a libxl_internal.h define.
Err, by "the qemu savefile pathname" I meant the pathname in dom0.
I assume your wrapper script opens that and feeds it to the console.
Is that right ?
Thanks,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain
2020-05-18 14:15 ` Ian Jackson
@ 2020-05-18 14:50 ` Marek Marczykowski-Górecki
2020-05-18 15:18 ` [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain [and 1 more messages] Ian Jackson
0 siblings, 1 reply; 55+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-05-18 14:50 UTC (permalink / raw)
To: Ian Jackson; +Cc: Anthony Perard, xen-devel, Wei Liu, Jason Andryuk
[-- Attachment #1: Type: text/plain, Size: 1550 bytes --]
On Mon, May 18, 2020 at 03:15:17PM +0100, Ian Jackson wrote:
> Jason Andryuk writes ("Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> > On Thu, May 14, 2020 at 12:35 PM Ian Jackson <ian.jackson@citrix.com> wrote:
> > > It's not stated anywhere here that I can see but I think what is
> > > happening here is that your wrapper script knows the qemu savefile
> > > pathname and reads it directly. Maybbe a comment would be
> > > worthwhile ?
> >
> > The existing comment "Linux stubdomain connects specific FD to
> > STUBDOM_CONSOLE_RESTORE" is trying to state that.
> > STUBDOM_CONSOLE_RESTORE is defined as 2 for console 2 (/dev/hvc2), but
> > it is only a libxl_internal.h define.
>
> Err, by "the qemu savefile pathname" I meant the pathname in dom0.
> I assume your wrapper script opens that and feeds it to the console.
> Is that right ?
Not really a wrapper script. On dom0 side it is console backend (qemu)
instructed to connect the console to a file, instead of pty. I have
implemented similar feature in my xenconsoled patch series sent a while
ago (sent along with v2 of this series), but that series needs some more
love.
On the stubdomain side, it is a script that launches qemu - opens a
/dev/hvc2, then pass the FD to qemu via -incoming option (which is
really constructed by libxl).
--
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain [and 1 more messages]
2020-05-18 14:50 ` Marek Marczykowski-Górecki
@ 2020-05-18 15:18 ` Ian Jackson
2020-05-18 15:48 ` Jason Andryuk
0 siblings, 1 reply; 55+ messages in thread
From: Ian Jackson @ 2020-05-18 15:18 UTC (permalink / raw)
To: Marek Marczykowski-Górecki, Jason Andryuk
Cc: Anthony Perard, xen-devel, Wei Liu
>
Marek Marczykowski-Górecki writes ("Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> On Mon, May 18, 2020 at 03:15:17PM +0100, Ian Jackson wrote:
> > Err, by "the qemu savefile pathname" I meant the pathname in dom0.
> > I assume your wrapper script opens that and feeds it to the console.
> > Is that right ?
>
> Not really a wrapper script. On dom0 side it is console backend (qemu)
> instructed to connect the console to a file, instead of pty. I have
> implemented similar feature in my xenconsoled patch series sent a while
> ago (sent along with v2 of this series), but that series needs some more
> love.
>
> On the stubdomain side, it is a script that launches qemu - opens a
> /dev/hvc2, then pass the FD to qemu via -incoming option (which is
> really constructed by libxl).
Hi. Thanks for trying to help me understand. I was still confused
though. I tried to explain another way and that helped me see what's
going on.
I think I understand now.
For reference, my confusion was this:
Jason Andryuk writes ("[PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> index bdc23554eb..45d0dd56f5 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1744,10 +1744,17 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
> }
>
> if (state->saved_state) {
> - /* This file descriptor is meant to be used by QEMU */
> - *dm_state_fd = open(state->saved_state, O_RDONLY);
> - flexarray_append(dm_args, "-incoming");
> - flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
> + if (is_stubdom) {
> + /* Linux stubdomain connects specific FD to STUBDOM_CONSOLE_RESTORE
> + */
> + flexarray_append(dm_args, "-incoming");
> + flexarray_append(dm_args, "fd:3");
> + } else {
> + /* This file descriptor is meant to be used by QEMU */
> + *dm_state_fd = open(state->saved_state, O_RDONLY);
> + flexarray_append(dm_args, "-incoming");
> + flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
> + }
In this hunk, the call
*dm_state_fd = open(state->saved_state, O_RDONLY);
becomes conditional. It is no longer executed in the stubdomain
case.
So then, who opens state->saved_state ? And how do they get told
where it is ? If it's somewhere else in libxl, why doesn't it show up
in this patch ?
Posing the question liked that allowed me to see that the answer is
console[i].output = GCSPRINTF("file:%s",
libxl__device_model_savefile(gc, guest_domid));
in spawn_stub_launch_dm. And it doesn't appear in the patch because
it's already used for minios stub trad qemu and the same code is
now going to be executed for linux stub moderm qemu.
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain [and 1 more messages]
2020-05-18 15:18 ` [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain [and 1 more messages] Ian Jackson
@ 2020-05-18 15:48 ` Jason Andryuk
2020-05-18 16:37 ` Ian Jackson
0 siblings, 1 reply; 55+ messages in thread
From: Jason Andryuk @ 2020-05-18 15:48 UTC (permalink / raw)
To: Ian Jackson
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
On Mon, May 18, 2020 at 11:18 AM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> >
> Marek Marczykowski-Górecki writes ("Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> > On Mon, May 18, 2020 at 03:15:17PM +0100, Ian Jackson wrote:
> > > Err, by "the qemu savefile pathname" I meant the pathname in dom0.
> > > I assume your wrapper script opens that and feeds it to the console.
> > > Is that right ?
> >
> > Not really a wrapper script. On dom0 side it is console backend (qemu)
> > instructed to connect the console to a file, instead of pty. I have
> > implemented similar feature in my xenconsoled patch series sent a while
> > ago (sent along with v2 of this series), but that series needs some more
> > love.
> >
> > On the stubdomain side, it is a script that launches qemu - opens a
> > /dev/hvc2, then pass the FD to qemu via -incoming option (which is
> > really constructed by libxl).
>
> Hi. Thanks for trying to help me understand. I was still confused
> though. I tried to explain another way and that helped me see what's
> going on.
>
> I think I understand now.
>
> For reference, my confusion was this:
>
> Jason Andryuk writes ("[PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain"):
> > index bdc23554eb..45d0dd56f5 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -1744,10 +1744,17 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
> > }
> >
> > if (state->saved_state) {
> > - /* This file descriptor is meant to be used by QEMU */
> > - *dm_state_fd = open(state->saved_state, O_RDONLY);
> > - flexarray_append(dm_args, "-incoming");
> > - flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
> > + if (is_stubdom) {
> > + /* Linux stubdomain connects specific FD to STUBDOM_CONSOLE_RESTORE
> > + */
> > + flexarray_append(dm_args, "-incoming");
> > + flexarray_append(dm_args, "fd:3");
> > + } else {
> > + /* This file descriptor is meant to be used by QEMU */
> > + *dm_state_fd = open(state->saved_state, O_RDONLY);
> > + flexarray_append(dm_args, "-incoming");
> > + flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
> > + }
>
> In this hunk, the call
> *dm_state_fd = open(state->saved_state, O_RDONLY);
> becomes conditional. It is no longer executed in the stubdomain
> case.
>
> So then, who opens state->saved_state ? And how do they get told
> where it is ? If it's somewhere else in libxl, why doesn't it show up
> in this patch ?
>
> Posing the question liked that allowed me to see that the answer is
>
> console[i].output = GCSPRINTF("file:%s",
> libxl__device_model_savefile(gc, guest_domid));
>
> in spawn_stub_launch_dm. And it doesn't appear in the patch because
> it's already used for minios stub trad qemu and the same code is
> now going to be executed for linux stub moderm qemu.
Do you want the commit message to add a blurb about this? So the
message becomes:
"""
Rely on a wrapper script in stubdomain to attach relevant consoles to
qemu. The save console (1) must be attached to fdset/1. When
performing a restore, $STUBDOM_RESTORE_INCOMING_ARG must be replaced on
the qemu command line by "fd:$FD", where $FD is an open file descriptor
number to the restore console (2).
Existing libxl code (for dom0) already connects the stubdom's save &
restore console outputs to the save & restore files.
"""
-Jason
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain [and 1 more messages]
2020-05-18 15:48 ` Jason Andryuk
@ 2020-05-18 16:37 ` Ian Jackson
0 siblings, 0 replies; 55+ messages in thread
From: Ian Jackson @ 2020-05-18 16:37 UTC (permalink / raw)
To: Jason Andryuk
Cc: Anthony Perard, xen-devel, Marek Marczykowski-Górecki, Wei Liu
Jason Andryuk writes ("Re: [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain [and 1 more messages]"):
> On Mon, May 18, 2020 at 11:18 AM Ian Jackson <ian.jackson@citrix.com> wrote:
> > [explanation of confusion]
>
> Do you want the commit message to add a blurb about this? So the
> message becomes:
> """
> Rely on a wrapper script in stubdomain to attach relevant consoles to
> qemu. The save console (1) must be attached to fdset/1. When
> performing a restore, $STUBDOM_RESTORE_INCOMING_ARG must be replaced on
> the qemu command line by "fd:$FD", where $FD is an open file descriptor
> number to the restore console (2).
>
> Existing libxl code (for dom0) already connects the stubdom's save &
> restore console outputs to the save & restore files.
> """
I think that would be good, thanks, yes but I won't insist on it.
I think I already gave my ack for v6 of this. If you add the commit
message text above you should obviously keep that ack.
Thanks,
Ian.
^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2020-05-18 16:37 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28 4:04 [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 01/21] Document ioemu MiniOS stubdomain protocol Jason Andryuk
2020-05-14 16:08 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 02/21] Document ioemu Linux " Jason Andryuk
2020-05-14 16:08 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 03/21] libxl: fix qemu-trad cmdline for no sdl/vnc case Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 04/21] libxl: Allow running qemu-xen in stubdomain Jason Andryuk
2020-05-14 16:10 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options Jason Andryuk
2020-05-14 16:19 ` Ian Jackson
2020-05-17 13:55 ` Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 06/21] libxl: write qemu arguments into separate xenstore keys Jason Andryuk
2020-05-14 16:25 ` Ian Jackson
2020-05-17 14:29 ` Jason Andryuk
2020-04-28 4:04 ` [PATCH v5 07/21] xl: add stubdomain related options to xl config parser Jason Andryuk
2020-05-14 16:26 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 08/21] tools/libvchan: notify server when client is connected Jason Andryuk
2020-05-14 16:27 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain Jason Andryuk
2020-05-14 16:35 ` Ian Jackson
2020-05-17 13:55 ` Jason Andryuk
2020-05-18 14:15 ` Ian Jackson
2020-05-18 14:50 ` Marek Marczykowski-Górecki
2020-05-18 15:18 ` [PATCH v5 09/21] libxl: add save/restore support for qemu-xen in stubdomain [and 1 more messages] Ian Jackson
2020-05-18 15:48 ` Jason Andryuk
2020-05-18 16:37 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 10/21] tools: add missing libxenvchan cflags Jason Andryuk
2020-05-14 16:35 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 11/21] tools: add simple vchan-socket-proxy Jason Andryuk
2020-05-14 16:37 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 12/21] libxl: use vchan for QMP access with Linux stubdomain Jason Andryuk
2020-05-14 16:39 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 13/21] Regenerate autotools files Jason Andryuk
2020-05-14 16:41 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 14/21] libxl: require qemu in dom0 even if stubdomain is in use Jason Andryuk
2020-05-14 16:42 ` Ian Jackson
2020-05-14 17:36 ` Marek Marczykowski-Górecki
2020-04-28 4:04 ` [PATCH v5 15/21] libxl: ignore emulated IDE disks beyond the first 4 Jason Andryuk
2020-05-14 16:43 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 16/21] libxl: consider also qemu in stubdomain in libxl__dm_active check Jason Andryuk
2020-05-14 16:43 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 17/21] docs: Add device-model-domid to xenstore-paths Jason Andryuk
2020-05-14 16:44 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 18/21] libxl: Check stubdomain kernel & ramdisk presence Jason Andryuk
2020-05-14 16:45 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 19/21] libxl: Refactor kill_device_model to libxl__kill_xs_path Jason Andryuk
2020-05-14 16:45 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 20/21] libxl: Kill vchan-socket-proxy when cleaning up qmp Jason Andryuk
2020-05-14 16:47 ` Ian Jackson
2020-04-28 4:04 ` [PATCH v5 21/21] tools: Clean up vchan-socket-proxy socket Jason Andryuk
2020-05-14 16:48 ` Ian Jackson
2020-05-11 20:19 ` [PATCH v5 00/21] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
2020-05-14 16:07 ` Ian Jackson
2020-05-14 16:55 ` Ian Jackson
2020-05-14 19:10 ` Jason Andryuk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.