All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain.
@ 2019-01-28 21:30 Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
                   ` (16 more replies)
  0 siblings, 17 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel
  Cc: Simon Gaiser, Wei Liu, Ian Jackson,
	Marek Marczykowski-Górecki, Eric Shelton

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. This means libxenlight
will be linked with libxenvchan.

Ideally talking to stubdomain would be added only to new libxl__qmp_ev* API,
which is written from start with the assumption of untrusted qemu. But
unfortunately some parts of libxl public API that calls into QMP, are not
async-aware and can't use libxl__qmp_ev* API. Example of such API is
libxl_domain_unpause(). Because of this, separate patch add support for QMP
over vchan also to the old API.

There is also a need for external locking access to vchan connection (one
server can handle only one client and libvchan does not verify this). Since I
haven't found any asynchronous locking primitives in libxl, for now I've added
flock() on a lock file, but this also needs to be converted to async version.

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.
Beware: building on Debian is dangerous, as it require installing "dracut",
which will remove initramfs-tools. You may end up with broken initrd on
your host.

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

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

Cc: Simon Gaiser <simon@invisiblethingslab.com>
Cc: Eric Shelton <eshelton@pobox.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

Eric Shelton (1):
  libxl: Handle Linux stubdomain specific QEMU options.

Marek Marczykowski-Górecki (16):
  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
  libxl: create vkb device only for guests with graphics output
  xl: add stubdomain related options to xl config parser
  tools/libvchan: notify server when client is connected
  libxl: typo fix in comment
  libxl: move xswait declaration up in libxl_internal.h
  libxl: use vchan for QMP access with Linux stubdomain, libxl__ev_qmp_* version
  libxl: use vchan for QMP access with Linux stubdomain, non-async version
  libxl: add save/restore support for qemu-xen in stubdomain
  tools: add missing libxenvchan cflags
  libxl: add locking for libvchan QMP connection
  libxl: require qemu in dom0 even if stubdomain is in use

 docs/man/xl.cfg.5.pod.in     |  23 +-
 docs/misc/stubdom.txt        | 103 +++++++++-
 tools/Rules.mk               |   6 +-
 tools/libvchan/init.c        |   3 +-
 tools/libxl/Makefile         |   2 +-
 tools/libxl/libxl_create.c   |  51 +++-
 tools/libxl/libxl_dm.c       | 241 ++++++++++++++------
 tools/libxl/libxl_internal.h | 134 ++++++-----
 tools/libxl/libxl_mem.c      |   6 +-
 tools/libxl/libxl_qmp.c      | 433 ++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_types.idl  |   3 +-
 tools/xl/xl_parse.c          |   7 +-
 12 files changed, 856 insertions(+), 156 deletions(-)

base-commit: 4595e7d86aea956a55f8e5a607f3b8c8c3519f77
-- 
git-series 0.9.1

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-12 10:52   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 02/17] Document ioemu Linux " Marek Marczykowski-Górecki
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, Tim Deegan, Julien Grall,
	Jan Beulich

Add documentation based on reverse-engineered toolstack-ioemu stubdomain
protocol.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 docs/misc/stubdom.txt | 53 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+)

diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
index de7b6c7..4c524f2 100644
--- a/docs/misc/stubdom.txt
+++ b/docs/misc/stubdom.txt
@@ -23,6 +23,59 @@ and http://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
+2. stubdoma signal readiness by writing "running" to /local/domain/<stubdom-id>/device-model/<target-id>/state xenstore path
+3. 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
                                    =======
 
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 02/17] Document ioemu Linux stubdomain protocol
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 15:39   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 03/17] libxl: fix qemu-trad cmdline for no sdl/vnc case Marek Marczykowski-Górecki
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, Tim Deegan, Julien Grall,
	Jan Beulich

Add documentation for upcoming Linux stubdomain for qemu-upstream.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 docs/misc/stubdom.txt | 50 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+)

diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
index 4c524f2..9c94c6b 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
                                    =======
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 03/17] libxl: fix qemu-trad cmdline for no sdl/vnc case
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 02/17] Document ioemu Linux " Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

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@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.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 b245956..3601b14 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -715,14 +715,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");
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (2 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 03/17] libxl: fix qemu-trad cmdline for no sdl/vnc case Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:01   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 05/17] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

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>

---
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 a4e74a5..bb62542 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -160,15 +160,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 459f9bf..b8c698a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2195,6 +2195,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)
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 05/17] libxl: Handle Linux stubdomain specific QEMU options.
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (3 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:02   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 06/17] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel
  Cc: Simon Gaiser, Wei Liu, Ian Jackson,
	Marek Marczykowski-Górecki, 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"
 * pass downscript for network interfaces

Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
[drop Qubes-specific parts]
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.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)
---
 tools/libxl/libxl_create.c   |  40 +++++++-
 tools/libxl/libxl_dm.c       | 176 +++++++++++++++++++++++++-----------
 tools/libxl/libxl_internal.h |   1 +-
 tools/libxl/libxl_mem.c      |   6 +-
 tools/libxl/libxl_types.idl  |   3 +-
 5 files changed, 171 insertions(+), 55 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bb62542..13fc304 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -160,6 +160,31 @@ 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());
+                    b_info->stubdomain_ramdisk = NULL;
+                    break;
+                case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+                    b_info->stubdomain_kernel =
+                        libxl__abs_path(NOGC,
+                                "stubdom-linux-kernel",
+                                libxl__xenfirmwaredir_path());
+                    b_info->stubdomain_ramdisk =
+                        libxl__abs_path(NOGC,
+                                "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) {
@@ -195,6 +220,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);
@@ -1538,7 +1574,9 @@ static void domcreate_devmodel_started(libxl__egc *egc,
     if (dcs->sdss.dm.guest_domid) {
         if (d_config->b_info.device_model_version
             == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
-            libxl__qmp_initializations(gc, domid, d_config);
+            if (!libxl_defbool_val(d_config->b_info.device_model_stubdomain)) {
+                libxl__qmp_initializations(gc, domid, d_config);
+            }
         }
     }
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 3601b14..1fa4fa8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1169,6 +1169,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);
@@ -1179,30 +1180,33 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                       "-xen-domid",
                       GCSPRINTF("%d", guest_domid), NULL);
 
-    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));
-    } else {
-        flexarray_append(dm_args,
-                         GCSPRINTF("socket,id=libxl-cmd,"
-                                   "path=%s,server,nowait",
-                                   libxl__qemu_qmp_path(gc, guest_domid)));
-    }
+    /* 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));
+        } 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, "-no-shutdown");
+        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;
@@ -1246,7 +1250,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");
@@ -1285,7 +1289,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.
          */
@@ -1297,7 +1301,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);
@@ -1339,18 +1343,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);
+                    }
                 }
             }
         }
@@ -1359,7 +1379,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)
@@ -1784,7 +1804,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);
@@ -1795,6 +1817,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,
@@ -1946,7 +1978,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) {
@@ -1956,8 +1988,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,
@@ -2062,6 +2096,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);
@@ -2112,10 +2156,14 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     libxl__domain_build_state_init(stubdom_state);
     dmss_init(&sdss->dm);
 
-    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 = 0;
@@ -2136,8 +2184,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     libxl_domain_build_info_init_type(&dm_config->b_info, LIBXL_DOMAIN_TYPE_PV);
 
     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;
@@ -2178,10 +2226,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,
@@ -2201,6 +2247,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",
@@ -2210,6 +2258,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",
@@ -2291,6 +2348,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));
 
@@ -2324,8 +2386,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 b8c698a..a26d36e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -118,6 +118,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 INVALID_DOMID ~0
diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
index 448a2af..368377e 100644
--- a/tools/libxl/libxl_mem.c
+++ b/tools/libxl/libxl_mem.c
@@ -465,8 +465,10 @@ int libxl_domain_need_memory(libxl_ctx *ctx,
     case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_HVM:
         *need_memkb += b_info->shadow_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 += b_info->shadow_memkb + LIBXL_PV_EXTRA_MEMORY;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index b685ac4..35092f2 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -498,6 +498,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),
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 06/17] libxl: write qemu arguments into separate xenstore keys
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (4 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 05/17] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:02   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 07/17] libxl: create vkb device only for guests with graphics output Marek Marczykowski-Górecki
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

This allows using arguments with spaces, like -append, without
nominating any special "separator" character.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.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 1fa4fa8..6cfc256 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2047,6 +2047,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)
@@ -2249,7 +2283,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)),
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 07/17] libxl: create vkb device only for guests with graphics output
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (5 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 06/17] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:02   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 08/17] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

The forced vkb device is meant for better performance of qemu access
(at least according to ebbd2561b4cefb299f0f68a88b2788504223de18 "libxl:
Add a vkbd frontend/backend pair for HVM guests"), which isn't used if
there is no configured channel to actually access that keyboard.

One can still add vkb device manually if needed.

This is continuation of b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do
not start dom0 qemu for stubdomain when not needed".

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libxl/libxl_create.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 13fc304..736b93c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1439,9 +1439,13 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         libxl__device_console_add(gc, domid, &console, state, &device);
         libxl__device_console_dispose(&console);
 
-        libxl_device_vkb_init(&vkb);
-        libxl__device_add(gc, domid, &libxl__vkb_devtype, &vkb);
-        libxl_device_vkb_dispose(&vkb);
+        if (libxl_defbool_val(d_config->b_info.u.hvm.vnc.enable)
+            || libxl_defbool_val(d_config->b_info.u.hvm.spice.enable)
+            || libxl_defbool_val(d_config->b_info.u.hvm.sdl.enable)) {
+            libxl_device_vkb_init(&vkb);
+            libxl__device_add(gc, domid, &libxl__vkb_devtype, &vkb);
+            libxl_device_vkb_dispose(&vkb);
+        }
 
         dcs->sdss.dm.guest_domid = domid;
         if (libxl_defbool_val(d_config->b_info.device_model_stubdomain))
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 08/17] xl: add stubdomain related options to xl config parser
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (6 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 07/17] libxl: create vkb device only for guests with graphics output Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:02   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 09/17] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
---
 docs/man/xl.cfg.5.pod.in | 23 +++++++++++++++++++----
 tools/xl/xl_parse.c      |  7 +++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 3b92f39..f475196 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2694,10 +2694,25 @@ 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<device_model_stubdomain_override=BOOLEAN>
 
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 352cd21..77e4cf0 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2502,6 +2502,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);            \
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 09/17] tools/libvchan: notify server when client is connected
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (7 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 08/17] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:02   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 10/17] libxl: typo fix in comment Marek Marczykowski-Górecki
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

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>
---
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 180833d..50a64c1 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);
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 10/17] libxl: typo fix in comment
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (8 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 09/17] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:02   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 11/17] libxl: move xswait declaration up in libxl_internal.h Marek Marczykowski-Górecki
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libxl/libxl_qmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 42c8ab8..a235095 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1452,7 +1452,7 @@ static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
  *
  * - Allowed internal state transition:
  * disconnected                     -> connecting
- * connection                       -> capability_negotiation
+ * connecting                       -> capability_negotiation
  * capability_negotiation/connected -> waiting_reply
  * waiting_reply                    -> connected
  * any                              -> broken
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 11/17] libxl: move xswait declaration up in libxl_internal.h
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (9 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 10/17] libxl: typo fix in comment Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:02   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 12/17] libxl: use vchan for QMP access with Linux stubdomain, libxl__ev_qmp_* version Marek Marczykowski-Górecki
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

It will be needed for qmp_ev_* over vchan.

No functional change.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

---
Changes in v3:
 - new patch
---
 tools/libxl/libxl_internal.h | 108 ++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 54 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a26d36e..0095835 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -358,6 +358,60 @@ struct libxl__ev_child {
     LIBXL_LIST_ENTRY(struct libxl__ev_child) entry;
 };
 
+/*----- xswait: wait for a xenstore node to be suitable -----*/
+
+typedef struct libxl__xswait_state libxl__xswait_state;
+
+/*
+ * rc describes the circumstances of this callback:
+ *
+ * rc==0
+ *
+ *     The xenstore path (may have) changed.  It has been read for
+ *     you.  The result is in data (allocated from the ao gc).
+ *     data may be NULL, which means that the xenstore read gave
+ *     ENOENT.
+ *
+ *     If you are satisfied, you MUST call libxl__xswait_stop.
+ *     Otherwise, xswait will continue waiting and watching and
+ *     will call you back later.
+ *
+ * rc==ERROR_TIMEDOUT, rc==ERROR_ABORTED
+ *
+ *     The specified timeout was reached.
+ *     This has NOT been logged (except to the debug log).
+ *     xswait will not continue (but calling libxl__xswait_stop is OK).
+ *
+ * rc!=0, !=ERROR_TIMEDOUT, !=ERROR_ABORTED
+ *
+ *     Some other error occurred.
+ *     This HAS been logged.
+ *     xswait will not continue (but calling libxl__xswait_stop is OK).
+ *
+ * xswait.path may start with with '@', in which case no read is done
+ * and the callback will always get data==0.
+ */
+typedef void libxl__xswait_callback(libxl__egc *egc,
+      libxl__xswait_state *xswa, int rc, const char *data);
+
+struct libxl__xswait_state {
+    /* caller must fill these in, and they must all remain valid */
+    libxl__ao *ao;
+    const char *what; /* for error msgs: noun phrase, what we're waiting for */
+    const char *path;
+    int timeout_ms; /* as for poll(2) */
+    libxl__xswait_callback *callback;
+    /* remaining fields are private to xswait */
+    libxl__ev_time time_ev;
+    libxl__ev_xswatch watch_ev;
+};
+
+void libxl__xswait_init(libxl__xswait_state*);
+void libxl__xswait_stop(libxl__gc*, libxl__xswait_state*); /*idempotent*/
+bool libxl__xswait_inuse(const libxl__xswait_state *ss);
+
+int libxl__xswait_start(libxl__gc*, libxl__xswait_state*);
+
 /*
  * QMP asynchronous calls
  *
@@ -1401,60 +1455,6 @@ _hidden const char *libxl__device_nic_devname(libxl__gc *gc,
 
 _hidden int libxl__get_domid(libxl__gc *gc, uint32_t *domid);
 
-/*----- xswait: wait for a xenstore node to be suitable -----*/
-
-typedef struct libxl__xswait_state libxl__xswait_state;
-
-/*
- * rc describes the circumstances of this callback:
- *
- * rc==0
- *
- *     The xenstore path (may have) changed.  It has been read for
- *     you.  The result is in data (allocated from the ao gc).
- *     data may be NULL, which means that the xenstore read gave
- *     ENOENT.
- *
- *     If you are satisfied, you MUST call libxl__xswait_stop.
- *     Otherwise, xswait will continue waiting and watching and
- *     will call you back later.
- *
- * rc==ERROR_TIMEDOUT, rc==ERROR_ABORTED
- *
- *     The specified timeout was reached.
- *     This has NOT been logged (except to the debug log).
- *     xswait will not continue (but calling libxl__xswait_stop is OK).
- *
- * rc!=0, !=ERROR_TIMEDOUT, !=ERROR_ABORTED
- *
- *     Some other error occurred.
- *     This HAS been logged.
- *     xswait will not continue (but calling libxl__xswait_stop is OK).
- *
- * xswait.path may start with with '@', in which case no read is done
- * and the callback will always get data==0.
- */
-typedef void libxl__xswait_callback(libxl__egc *egc,
-      libxl__xswait_state *xswa, int rc, const char *data);
-
-struct libxl__xswait_state {
-    /* caller must fill these in, and they must all remain valid */
-    libxl__ao *ao;
-    const char *what; /* for error msgs: noun phrase, what we're waiting for */
-    const char *path;
-    int timeout_ms; /* as for poll(2) */
-    libxl__xswait_callback *callback;
-    /* remaining fields are private to xswait */
-    libxl__ev_time time_ev;
-    libxl__ev_xswatch watch_ev;
-};
-
-void libxl__xswait_init(libxl__xswait_state*);
-void libxl__xswait_stop(libxl__gc*, libxl__xswait_state*); /*idempotent*/
-bool libxl__xswait_inuse(const libxl__xswait_state *ss);
-
-int libxl__xswait_start(libxl__gc*, libxl__xswait_state*);
-
 /*
  * libxl__ev_devstate - waits a given time for a device to
  * reach a given state.  Follows the libxl_ev_* conventions.
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 12/17] libxl: use vchan for QMP access with Linux stubdomain, libxl__ev_qmp_* version
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (10 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 11/17] libxl: move xswait declaration up in libxl_internal.h Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 13/17] libxl: use vchan for QMP access with Linux stubdomain, non-async version Marek Marczykowski-Górecki
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

Access to QMP of QEMU in Linux stubdomain is possible over vchan
connection. Add appropriate handling in libxl__ev_qmp_* API, keeping all the
asynchronous properties.
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. 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.

Regarding pipe to self:
Vchan issue notification about incoming data (or space for it)
only once - even if there is more data to read, FD returned by
libxenvchan_fd_for_select() will not be readable. Similar to buffer
space - there is one notification if more data can be written, but FD
isn't considered "writable" after libxenvchan_wait() call, even if in
fact there is a buffer space.
There are two situations where it is problematic:
 - some QMP message results in a user callback and processing further
   messages must stop (even if more data is already available in vchan
   buffer)
 - data is scheduled to write after a buffer space notification; this
   may result from either delayed libxl__ev_qmp_send() call, or internal
   state change

To avoid the above problems, use pipe to self to schedule vchan data
processing, even if vchan would not issue notification itself.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
TODO:
 - handle locking for vchan access - a lockfile + flock comes to mind,
   but there is no async provisioning for it in libxl

Changes in v3:
 - new patch, in place of "libxl: access QMP socket via console for
   qemu-in-stubdomain"
---
 tools/Rules.mk               |   4 +-
 tools/libxl/Makefile         |   2 +-
 tools/libxl/libxl_dm.c       |   2 +-
 tools/libxl/libxl_internal.h |   7 +-
 tools/libxl/libxl_qmp.c      | 293 +++++++++++++++++++++++++++++++++++-
 5 files changed, 301 insertions(+), 7 deletions(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 68f2ed7..12b3129 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -187,8 +187,8 @@ SHLIB_libblktapctl  =
 PKG_CONFIG_REMOVE += xenblktapctl
 endif
 
-CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl) $(CFLAGS_xeninclude)
-SHDEPS_libxenlight = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl)
+CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl) $(CFLAGS_xeninclude) $(CFLAGS_libxenvchan)
+SHDEPS_libxenlight = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) $(SHLIB_libblktapctl) $(SHLIB_libxenvchan)
 LDLIBS_libxenlight = $(SHDEPS_libxenlight) $(XEN_XENLIGHT)/libxenlight$(libextension)
 SHLIB_libxenlight  = $(SHDEPS_libxenlight) -Wl,-rpath-link=$(XEN_XENLIGHT)
 
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 6da342e..55b0b63 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -24,6 +24,7 @@ LIBXL_LIBS = $(LDLIBS_libxentoollog) $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl)
 ifeq ($(CONFIG_LIBNL),y)
 LIBXL_LIBS += $(LIBNL3_LIBS)
 endif
+LIBXL_LIBS += $(LDLIBS_libxenvchan)
 
 CFLAGS_LIBXL += $(CFLAGS_libxentoollog)
 CFLAGS_LIBXL += $(CFLAGS_libxentoolcore)
@@ -35,6 +36,7 @@ CFLAGS_LIBXL += $(CFLAGS_libblktapctl)
 ifeq ($(CONFIG_LIBNL),y)
 CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
 endif
+CFLAGS_LIBXL += $(CFLAGS_libxenvchan)
 CFLAGS_LIBXL += -Wshadow
 
 LIBXL_LIBS-$(CONFIG_ARM) += -lfdt
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 6cfc256..b9a53f3 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1180,7 +1180,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                       "-xen-domid",
                       GCSPRINTF("%d", guest_domid), NULL);
 
-    /* There is currently no way to access the QMP socket in the stubdom */
+    /* QMP access to qemu running in stubdomain is done over vchan, not local socket */
     if (!is_stubdom) {
         flexarray_append(dm_args, "-chardev");
         if (state->dm_monitor_fd >= 0) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0095835..9a903a5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -57,6 +57,7 @@
 #include <xenctrl.h>
 #include <xenguest.h>
 #include <xc_dom.h>
+#include <libxenvchan.h>
 
 #include <xen-tools/libs.h>
 
@@ -509,6 +510,12 @@ struct libxl__ev_qmp {
     libxl__carefd *cfd;
     libxl__ev_fd efd;
     libxl__qmp_state state;
+    /* pipe to wake itself if there is more data in vchan */
+    libxl__carefd *pipe_cfd_read;
+    libxl__carefd *pipe_cfd_write;
+    libxl__ev_fd pipe_efd;
+    struct libxenvchan *vchan;
+    libxl__xswait_state xswait;
     int id;
     int next_id;        /* next id to use */
     /* receive buffer */
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index a235095..45b9f74 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1466,6 +1466,14 @@ static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
 
 /* prototypes */
 
+static int qmp_ev_connect_vchan(libxl__gc *gc, libxl__ev_qmp *ev);
+static void qmp_vchan_watch_callback(libxl__egc *egc,
+      libxl__xswait_state *xswa, int rc, const char *data);
+static void qmp_ev_vchan_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
+                               int fd, short events, short revents);
+static void qmp_ev_vchan_pipe_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
+                                          int fd, short events, short revents);
+static void qmp_ev_vchan_schedule_wakeup(libxl__ev_qmp *ev);
 static void qmp_ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
                                int fd, short events, short revents);
 static int qmp_ev_callback_writable(libxl__gc *gc,
@@ -1491,6 +1499,17 @@ static void qmp_ev_ensure_reading_writing(libxl__gc *gc, libxl__ev_qmp *ev)
     else if ((ev->state == qmp_state_waiting_reply) && ev->msg)
         events |= POLLOUT;
 
+    if (ev->vchan || ev->xswait.what) {
+        if (ev->vchan &&
+                libxenvchan_buffer_space(ev->vchan) &&
+                (events & POLLOUT)) {
+            /* If vchan notification about available buffer space was received
+             * already, it won't be dispatched again, trigger it manually. */
+            qmp_ev_vchan_schedule_wakeup(ev);
+        }
+        return;
+    }
+
     libxl__ev_fd_modify(gc, &ev->efd, events);
 }
 
@@ -1564,6 +1583,8 @@ static int qmp_error_class_to_libxl_error_code(libxl__gc *gc,
 
 /* Setup connection */
 
+/*   - QMP over unix socket */
+
 static int qmp_ev_connect(libxl__gc *gc, libxl__ev_qmp *ev)
     /* disconnected -> connecting but with `msg` free
      * on error: broken */
@@ -1575,6 +1596,10 @@ static int qmp_ev_connect(libxl__gc *gc, libxl__ev_qmp *ev)
 
     assert(ev->state == qmp_state_disconnected);
 
+    /* use vchan connection for stubdomain */
+    if (libxl_get_stubdom_id(CTX, ev->domid))
+        return qmp_ev_connect_vchan(gc, ev);
+
     qmp_socket_path = libxl__qemu_qmp_path(gc, ev->domid);
 
     LOGD(DEBUG, ev->domid, "Connecting to %s", qmp_socket_path);
@@ -1618,6 +1643,106 @@ out:
     return rc;
 }
 
+/*   - QMP over vchan */
+
+static int qmp_ev_connect_vchan(libxl__gc *gc, libxl__ev_qmp *ev)
+    /* disconnected -> connecting but with `msg` free
+     * on error: broken */
+{
+    int dm_domid;
+    int rc;
+
+    assert(ev->state == qmp_state_disconnected);
+
+    dm_domid = libxl_get_stubdom_id(CTX, ev->domid);
+    assert(dm_domid != 0);
+
+    ev->xswait.ao = ev->ao;
+    ev->xswait.what = GCSPRINTF("qmp vchan for %u", ev->domid);
+    ev->xswait.path = DEVICE_MODEL_XS_PATH(gc, dm_domid, ev->domid, "/qmp-vchan");
+    ev->xswait.timeout_ms = LIBXL_STUBDOM_START_TIMEOUT * 1000;
+    ev->xswait.callback = qmp_vchan_watch_callback;
+
+    LOGD(DEBUG, ev->domid, "Connecting to vchan at %s", ev->xswait.path);
+
+    rc = libxl__xswait_start(gc, &ev->xswait);
+    if (rc) goto out;
+
+    qmp_ev_set_state(gc, ev, qmp_state_connecting);
+
+    return 0;
+
+out:
+    return rc;
+}
+
+static void qmp_vchan_watch_callback(libxl__egc *egc,
+      libxl__xswait_state *xswa, int rc, const char *data)
+{
+    libxl__ev_qmp *ev = CONTAINER_OF(xswa, *ev, xswait);
+    STATE_AO_GC(ev->ao);
+    int dm_domid = libxl_get_stubdom_id(CTX, ev->domid);
+    int pipe_fd[2];
+
+    if (!rc) {
+        ev->vchan = libxenvchan_client_init(CTX->lg, dm_domid, ev->xswait.path);
+        if (ev->vchan) {
+            /* ok */
+            libxl__xswait_stop(gc, &ev->xswait);
+
+            ev->vchan->blocking = 0;
+
+            rc = libxl__ev_fd_register(gc, &ev->efd, qmp_ev_vchan_fd_callback,
+                    libxenvchan_fd_for_select(ev->vchan), POLLIN);
+            if (rc)
+                goto error;
+
+            libxl__carefd_begin();
+            rc = pipe(pipe_fd);
+            if (!rc) {
+                ev->pipe_cfd_read = libxl__carefd_record(CTX, pipe_fd[0]);
+                ev->pipe_cfd_write = libxl__carefd_record(CTX, pipe_fd[1]);
+            }
+            libxl__carefd_unlock();
+            if (rc) {
+                LOGED(ERROR, ev->domid, "pipe() failed");
+                rc = ERROR_FAIL;
+                goto error;
+            }
+            if (!ev->pipe_cfd_read || !ev->pipe_cfd_write) {
+                LOGED(ERROR, ev->domid, "pipe FD registration failed");
+                rc = ERROR_FAIL;
+                goto error;
+            }
+            rc = libxl__ev_fd_register(gc, &ev->pipe_efd, qmp_ev_vchan_pipe_fd_callback,
+                    libxl__carefd_fd(ev->pipe_cfd_read), POLLIN);
+            if (rc)
+                goto error;
+        } else if (errno == ENOENT) {
+            /* not ready yet, wait */
+            return;
+        } else {
+            LOGED(ERROR, ev->domid, "Connection to qmp vchan for %u failed", ev->domid);
+            rc = ERROR_FAIL;
+        }
+    }
+
+    if (!rc)
+        return;
+
+error:
+
+    if (rc==ERROR_TIMEDOUT || rc==ERROR_ABORTED)
+        LOGD(ERROR, ev->domid, "Connection to qmp vchan for %u timed out", ev->domid);
+
+    /* On error, deallocate all private ressources */
+    libxl__ev_qmp_dispose(gc, ev);
+
+    /* And tell libxl__ev_qmp user about the error */
+    ev->callback(egc, ev, NULL, rc); /* must be last */
+}
+
+
 /* QMP FD callbacks */
 
 static void qmp_ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
@@ -1690,6 +1815,105 @@ error:
     ev->callback(egc, ev, NULL, rc); /* must be last */
 }
 
+/* Make sure vchan events will be handled even if no new notification is sent.
+ */
+static void qmp_ev_vchan_schedule_wakeup(libxl__ev_qmp *ev)
+{
+    assert(ev->pipe_cfd_write);
+    write(libxl__carefd_fd(ev->pipe_cfd_write), ".", 1);
+}
+
+static int qmp_ev_vchan_handle_read_write(libxl__egc *egc, libxl__ev_qmp *ev)
+{
+    int rc;
+    STATE_AO_GC(ev->ao);
+
+    rc = qmp_ev_callback_readable(egc, ev, -1);
+    if (rc)
+        /* returns both rc values -ERROR_* and 1 */
+        return rc;
+
+    if (ev->tx_buf || ((ev->state == qmp_state_waiting_reply) && ev->msg)) {
+        rc = qmp_ev_callback_writable(gc, ev, -1);
+        if (rc)
+            return rc;
+    }
+    return 0;
+}
+
+static void qmp_ev_vchan_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
+                               int fd, short events, short revents)
+    /* On entry, ev_fd is (of course) Active.  The ev_qmp may be in any
+     * state where this is permitted.  qmp_ev_vchan_fd_callback will do the work
+     * necessary to make progress, depending on the current state, and make
+     * the appropriate state transitions and callbacks.  */
+{
+    libxl__ev_qmp *ev = CONTAINER_OF(ev_fd, *ev, efd);
+    STATE_AO_GC(ev->ao);
+    int rc;
+
+    assert(ev->vchan);
+
+    if (revents & ~(POLLIN)) {
+        LOGD(ERROR, ev->domid,
+             "unexpected poll event 0x%x on QMP vchan FD (expected POLLIN)",
+            revents);
+        rc = ERROR_FAIL;
+        goto error;
+    }
+
+    if (revents & POLLIN) {
+        libxenvchan_wait(ev->vchan);
+        rc = qmp_ev_vchan_handle_read_write(egc, ev);
+        if (rc < 0)
+            goto error;
+    }
+
+    return;
+
+error:
+    assert(rc);
+
+    LOGD(ERROR, ev->domid,
+         "Error happened with the QMP connection to QEMU");
+
+    /* On error, deallocate all private ressources */
+    libxl__ev_qmp_dispose(gc, ev);
+
+    /* And tell libxl__ev_qmp user about the error */
+    ev->callback(egc, ev, NULL, rc); /* must be last */
+}
+
+static void qmp_ev_vchan_pipe_fd_callback(libxl__egc *egc, libxl__ev_fd *ev_fd,
+                                          int fd, short events, short revents)
+{
+    char buf[1];
+    int rc = 0;
+    libxl__ev_qmp *ev = CONTAINER_OF(ev_fd, *ev, pipe_efd);
+    STATE_AO_GC(ev->ao);
+
+    if (revents & POLLIN) {
+        read(fd, buf, 1);
+        rc = qmp_ev_vchan_handle_read_write(egc, ev);
+        if (rc < 0)
+            goto error;
+    }
+
+    return;
+
+error:
+    assert(rc);
+
+    LOGD(ERROR, ev->domid,
+         "Error happened with the QMP connection to QEMU");
+
+    /* On error, deallocate all private ressources */
+    libxl__ev_qmp_dispose(gc, ev);
+
+    /* And tell libxl__ev_qmp user about the error */
+    ev->callback(egc, ev, NULL, rc); /* must be last */
+}
+
 static int qmp_ev_callback_writable(libxl__gc *gc,
                                     libxl__ev_qmp *ev, int fd)
     /* on entry: !disconnected
@@ -1725,6 +1949,13 @@ static int qmp_ev_callback_writable(libxl__gc *gc,
         ev->payload_fd >= 0 &&
         ev->tx_buf_off == 0) {
 
+        /* sending FDs not supported over vchan */
+        if (ev->vchan) {
+            /* XXX should it be assert? */
+            LOGED(ERROR, ev->domid, "Sending FD over vchan connection not supported");
+            return ERROR_NI;
+        }
+
         rc = libxl__sendmsg_fds(gc, fd, ev->tx_buf[ev->tx_buf_off],
                                 1, &ev->payload_fd, "QMP socket");
         /* Check for EWOULDBLOCK, and return to try again later */
@@ -1737,7 +1968,16 @@ static int qmp_ev_callback_writable(libxl__gc *gc,
 
     while (ev->tx_buf_off < ev->tx_buf_len) {
         ssize_t max_write = ev->tx_buf_len - ev->tx_buf_off;
-        r = write(fd, ev->tx_buf + ev->tx_buf_off, max_write);
+        if (ev->vchan) {
+            r = libxenvchan_write(ev->vchan, ev->tx_buf + ev->tx_buf_off, max_write);
+            /* libxenvchan_write returns 0 if no space left, translate to EWOULDBLOCK */
+            if (r == 0) {
+                r = -1;
+                errno = EWOULDBLOCK;
+            }
+        } else {
+            r = write(fd, ev->tx_buf + ev->tx_buf_off, max_write);
+        }
         if (r < 0) {
             if (errno == EINTR)
                 continue;
@@ -1790,6 +2030,24 @@ static int qmp_ev_callback_readable(libxl__egc *egc,
             else if (rc)
                 return rc;
 
+            /*
+             * When adding support for multiple simultaneousl requests (or events),
+             * possibly multiple responses will be already waiting in vchan
+             * buffer. 'ev' can't be touched here after calling user callback,
+             * so remaining messages may be processed in the next callback
+             * (qmp_ev_fd_callback/qmp_ev_vchan_fd_callback). But if at this
+             * point all the data is already in vchan buffer and qemu will not
+             * write anything more, no vchan notification will be issued
+             * (ev->efd will not report POLLIN). Because of this, the callback
+             * will need to be triggered the other way:
+             *
+             * if (ev->vchan && libxenvchan_data_ready(ev->vchan))
+             *     qmp_ev_vchan_schedule_wakeup(ev);
+             *
+             * With only one qmp request pending, this isn't needed, as if user
+             * callback is called, there are no more (interesting) messages in
+             * the vchan buffer (there was only one).
+             */
             /* Must be last and return when the user callback is called */
             rc = qmp_ev_handle_message(egc, ev, o);
             if (rc)
@@ -1811,8 +2069,18 @@ static int qmp_ev_callback_readable(libxl__egc *egc,
             ev->rx_buf = libxl__realloc(gc, ev->rx_buf, ev->rx_buf_size);
         }
 
-        r = read(fd, ev->rx_buf + ev->rx_buf_used,
-                 ev->rx_buf_size - ev->rx_buf_used);
+        if (ev->vchan) {
+            r = libxenvchan_read(ev->vchan, ev->rx_buf + ev->rx_buf_used,
+                    ev->rx_buf_size - ev->rx_buf_used);
+            /* translate r == 0 to EWOULDBLOCK */
+            if (r == 0) {
+                r = -1;
+                errno = EWOULDBLOCK;
+            }
+        } else {
+            r = read(fd, ev->rx_buf + ev->rx_buf_used,
+                     ev->rx_buf_size - ev->rx_buf_used);
+        }
         if (r < 0) {
             if (errno == EINTR)
                 continue;
@@ -2098,7 +2366,14 @@ void libxl__ev_qmp_init(libxl__ev_qmp *ev)
     ev->next_id = 0x786c7100;
 
     ev->cfd = NULL;
+    ev->pipe_cfd_read = NULL;
+    ev->pipe_cfd_write = NULL;
+    ev->vchan = NULL;
+    libxl__xswait_init(&ev->xswait);
+    ev->xswait.what = NULL;
+    ev->xswait.path = NULL;
     libxl__ev_fd_init(&ev->efd);
+    libxl__ev_fd_init(&ev->pipe_efd);
     ev->state = qmp_state_disconnected;
     ev->id = 0;
 
@@ -2162,7 +2437,17 @@ void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev)
     LOGD(DEBUG, ev->domid, " ev %p", ev);
 
     libxl__ev_fd_deregister(gc, &ev->efd);
-    libxl__carefd_close(ev->cfd);
+    if (ev->vchan)
+        libxenvchan_close(ev->vchan);
+    else
+        libxl__carefd_close(ev->cfd);
+    if (ev->pipe_cfd_read) {
+        libxl__ev_fd_deregister(gc, &ev->pipe_efd);
+        libxl__carefd_close(ev->pipe_cfd_read);
+    }
+    if (ev->pipe_cfd_write) {
+        libxl__carefd_close(ev->pipe_cfd_write);
+    }
 
     libxl__ev_qmp_init(ev);
 }
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 13/17] libxl: use vchan for QMP access with Linux stubdomain, non-async version
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (11 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 12/17] libxl: use vchan for QMP access with Linux stubdomain, libxl__ev_qmp_* version Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 14/17] libxl: add save/restore support for qemu-xen in stubdomain Marek Marczykowski-Górecki
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

Access to QMP of QEMU in Linux stubdomain is possible over vchan
connection. Add appropriate handling to synchronous API.
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. 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.

Ideally, I woudn't need (or want) this part, and would communicate with
stubdomain only with async API. But some libxl public functions that are
not async-compatible do need to call qmp commands (for example
libxl_domain_unpause). Alternative to this patch, would be return error,
breaking all such functions, and incrementally convert them to async
API.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---

Two TODOs here:
 - handle locking, similarly to previous patch
 - select() qmp_next() can now be triggered by malicious stubdomain
   without actually sending any data, which would evade timeout
   enforcing (as select() each time is called with full timeout); the
   easiest thing to do, would be to re-use timeout value from previous
   select() call, but that works only on Linux; any better idea?
   Note that even without using vchan, malicious qemu could cause
   qmp_next() to wait almost indefinitely - by writing one byte at a
   time and never finishing the message

Changes in v3:
 - new patch, in place of "libxl: access QMP socket via console for
   qemu-in-stubdomain"
---
 tools/libxl/libxl_qmp.c | 61 ++++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 45b9f74..19ce3ce 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -113,6 +113,7 @@ typedef struct callback_id_pair {
 
 struct libxl__qmp_handler {
     int qmp_fd;
+    struct libxenvchan *vchan;
     bool connected;
     time_t timeout;
     /* wait_for_id will be used by the synchronous send function */
@@ -496,7 +497,10 @@ static void qmp_close(libxl__qmp_handler *qmp)
     callback_id_pair *pp = NULL;
     callback_id_pair *tmp = NULL;
 
-    close(qmp->qmp_fd);
+    if (qmp->vchan)
+        libxenvchan_close(qmp->vchan);
+    else
+        close(qmp->qmp_fd);
     LIBXL_STAILQ_FOREACH(pp, &qmp->callback_list, next) {
         free(tmp);
         tmp = pp;
@@ -516,7 +520,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
 
     do {
         fd_set rfds;
-        int ret = 0;
+        int ret = 1; /* used when select() is skipped */
         struct timeval timeout = {
             .tv_sec = qmp->timeout,
             .tv_usec = 0,
@@ -525,7 +529,8 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
         FD_ZERO(&rfds);
         FD_SET(qmp->qmp_fd, &rfds);
 
-        ret = select(qmp->qmp_fd + 1, &rfds, NULL, NULL, &timeout);
+        if (!(qmp->vchan && libxenvchan_data_ready(qmp->vchan)))
+            ret = select(qmp->qmp_fd + 1, &rfds, NULL, NULL, &timeout);
         if (ret == 0) {
             LOGD(ERROR, qmp->domid, "timeout");
             return -1;
@@ -536,7 +541,14 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
             return -1;
         }
 
-        rd = read(qmp->qmp_fd, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE);
+        if (qmp->vchan) {
+            libxenvchan_wait(qmp->vchan);
+            if (!libxenvchan_data_ready(qmp->vchan))
+                continue;
+            rd = libxenvchan_read(qmp->vchan, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE);
+        } else {
+            rd = read(qmp->qmp_fd, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE);
+        }
         if (rd == 0) {
             LOGD(ERROR, qmp->domid, "Unexpected end of socket");
             return -1;
@@ -684,9 +696,15 @@ static int qmp_send(libxl__qmp_handler *qmp,
         goto out;
     }
 
-    if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
-                            "QMP command", "QMP socket"))
-        goto out;
+    if (qmp->vchan) {
+        /* vchan->blocking == 1, so no need to wrap it in a loop */
+        if (libxenvchan_write(qmp->vchan, buf, strlen(buf)) == -1)
+            goto out;
+    } else {
+        if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
+                                "QMP command", "QMP socket"))
+            goto out;
+    }
 
     rc = qmp->last_id_used;
 out:
@@ -798,19 +816,34 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, uint32_t domid)
 {
     int ret = 0;
     libxl__qmp_handler *qmp = NULL;
-    char *qmp_socket;
+    int dm_domid;
+    char *qmp_path;
 
     qmp = qmp_init_handler(gc, domid);
     if (!qmp) return NULL;
 
-    qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
-    if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
-        LOGED(ERROR, domid, "Connection error");
-        qmp_free_handler(qmp);
-        return NULL;
+    dm_domid = libxl_get_stubdom_id(CTX, domid);
+    if (dm_domid) {
+        qmp_path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/qmp-vchan");
+        /* TODO: add locking */
+        qmp->vchan = libxenvchan_client_init(CTX->lg, dm_domid, qmp_path);
+        if (!qmp->vchan) {
+            LOGED(ERROR, domid, "QMP vchan connection failed: %s", strerror(errno));
+            qmp_free_handler(qmp);
+            return NULL;
+        }
+        qmp->vchan->blocking = 1;
+        qmp->qmp_fd = libxenvchan_fd_for_select(qmp->vchan);
+    } else {
+        qmp_path = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
+        if ((ret = qmp_open(qmp, qmp_path, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
+            LOGED(ERROR, domid, "Connection error");
+            qmp_free_handler(qmp);
+            return NULL;
+        }
     }
 
-    LOGD(DEBUG, domid, "connected to %s", qmp_socket);
+    LOGD(DEBUG, domid, "connected to %s", qmp_path);
 
     /* Wait for the response to qmp_capabilities */
     while (!qmp->connected) {
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 14/17] libxl: add save/restore support for qemu-xen in stubdomain
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (12 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 13/17] libxl: use vchan for QMP access with Linux stubdomain, non-async version Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 15/17] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

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>
---
Changes in v3:
 - adjust for qmp_ev*
 - assume specific fdset id in qemu set in stubdomain
---
 tools/libxl/libxl_dm.c  | 23 +++++++++++------------
 tools/libxl/libxl_qmp.c | 25 ++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b9a53f3..ce65321 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1715,10 +1715,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]);
@@ -2192,14 +2199,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 = 0;
 
     libxl_domain_create_info_init(&dm_config->c_info);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 19ce3ce..7643f15 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1328,6 +1328,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);
 
@@ -1360,10 +1361,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,
@@ -1394,7 +1402,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);
@@ -1409,6 +1416,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;
 
@@ -1426,6 +1448,7 @@ static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
 
 error:
     assert(rc);
+    /* TODO: only in non-stubdom case */
     libxl__remove_file(gc, dsps->dm_savefile);
     dsps->callback_device_model_done(egc, dsps, rc);
 }
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 15/17] tools: add missing libxenvchan cflags
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (13 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 14/17] libxl: add save/restore support for qemu-xen in stubdomain Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-02-21 16:05   ` Wei Liu
  2019-01-28 21:30 ` [PATCH v3 16/17] libxl: add locking for libvchan QMP connection Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 17/17] libxl: require qemu in dom0 even if stubdomain is in use Marek Marczykowski-Górecki
  16 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

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>
---
 tools/Rules.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 12b3129..a7c6c21 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -160,7 +160,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)
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 16/17] libxl: add locking for libvchan QMP connection
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (14 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 15/17] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  2019-01-28 21:30 ` [PATCH v3 17/17] libxl: require qemu in dom0 even if stubdomain is in use Marek Marczykowski-Górecki
  16 siblings, 0 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

It is not safe for multiple clients to (even try to) connect to the same
vchan server at the same time. Contrary to QMP over local socket,
connection over vchan needs external locking. For now use flock() for
this. This is not ideal for async QMP API, as flock() will block the
whole thread while other thread/process talks to the same QEMU instance.
This may be a problem especially in cause of malicious QEMU, that could
stall the communication. But since libxl doesn't have asynchronous
locking primitives, keep flock() until something better can be used
instead.

Note that qemu will handle only one client at a time anyway, so this
does not introduce artificial limit here.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libxl/libxl_internal.h |  1 +-
 tools/libxl/libxl_qmp.c      | 58 +++++++++++++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9a903a5..36b38fd 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -515,6 +515,7 @@ struct libxl__ev_qmp {
     libxl__carefd *pipe_cfd_write;
     libxl__ev_fd pipe_efd;
     struct libxenvchan *vchan;
+    libxl__carefd *vchan_lock;
     libxl__xswait_state xswait;
     int id;
     int next_id;        /* next id to use */
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 7643f15..260dc0a 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -114,6 +114,7 @@ typedef struct callback_id_pair {
 struct libxl__qmp_handler {
     int qmp_fd;
     struct libxenvchan *vchan;
+    libxl__carefd *vchan_lock;
     bool connected;
     time_t timeout;
     /* wait_for_id will be used by the synchronous send function */
@@ -497,9 +498,10 @@ static void qmp_close(libxl__qmp_handler *qmp)
     callback_id_pair *pp = NULL;
     callback_id_pair *tmp = NULL;
 
-    if (qmp->vchan)
+    if (qmp->vchan) {
         libxenvchan_close(qmp->vchan);
-    else
+        libxl__carefd_close(qmp->vchan_lock);
+    } else
         close(qmp->qmp_fd);
     LIBXL_STAILQ_FOREACH(pp, &qmp->callback_list, next) {
         free(tmp);
@@ -805,6 +807,40 @@ static void qmp_parameters_add_integer(libxl__gc *gc,
     qmp_parameters_common_add(gc, param, name, obj);
 }
 
+static libxl__carefd *qmp_vchan_lock(libxl__gc *gc, int domid)
+{
+    libxl__carefd *cfd;
+    char *lock_path;
+    int fd, r;
+
+    lock_path = GCSPRINTF("%s/qmp-libxl-%d.lock", libxl__run_dir_path(), domid);
+    libxl__carefd_begin();
+    fd = open(lock_path, O_RDWR | O_CREAT, 0644);
+    cfd = libxl__carefd_opened(CTX, fd);
+    if (!cfd) {
+        LOGED(ERROR, domid, "QMP lock file open error");
+        goto error;
+    }
+
+    /* Try to lock the file, retrying on EINTR */
+    for (;;) {
+        r = flock(fd, LOCK_EX);
+        if (!r)
+            break;
+        if (errno != EINTR) {
+            /* All other errno: EBADF, EINVAL, ENOLCK, EWOULDBLOCK */
+            LOGED(ERROR, domid,
+                  "unexpected error while trying to lock %s, fd=%d, errno=%d",
+                  lock_path, fd, errno);
+            goto error;
+        }
+    }
+    return cfd;
+error:
+    libxl__carefd_close(cfd);
+    return NULL;
+}
+
 #define QMP_PARAMETERS_SPRINTF(args, name, format, ...) \
     qmp_parameters_add_string(gc, args, name, GCSPRINTF(format, __VA_ARGS__))
 
@@ -825,10 +861,15 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, uint32_t domid)
     dm_domid = libxl_get_stubdom_id(CTX, domid);
     if (dm_domid) {
         qmp_path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/qmp-vchan");
-        /* TODO: add locking */
+        qmp->vchan_lock = qmp_vchan_lock(gc, domid);
+        if (!qmp->vchan_lock) {
+            qmp_free_handler(qmp);
+            return NULL;
+        }
         qmp->vchan = libxenvchan_client_init(CTX->lg, dm_domid, qmp_path);
         if (!qmp->vchan) {
             LOGED(ERROR, domid, "QMP vchan connection failed: %s", strerror(errno));
+            libxl__carefd_close(qmp->vchan_lock);
             qmp_free_handler(qmp);
             return NULL;
         }
@@ -1741,6 +1782,12 @@ static void qmp_vchan_watch_callback(libxl__egc *egc,
     int pipe_fd[2];
 
     if (!rc) {
+        /* FIXME: convert to async locking */
+        ev->vchan_lock = qmp_vchan_lock(gc, ev->domid);
+        if (!ev->vchan_lock) {
+            rc= ERROR_FAIL;
+            goto error;
+        }
         ev->vchan = libxenvchan_client_init(CTX->lg, dm_domid, ev->xswait.path);
         if (ev->vchan) {
             /* ok */
@@ -1775,6 +1822,8 @@ static void qmp_vchan_watch_callback(libxl__egc *egc,
             if (rc)
                 goto error;
         } else if (errno == ENOENT) {
+            libxl__carefd_close(ev->vchan_lock);
+            ev->vchan_lock = NULL;
             /* not ready yet, wait */
             return;
         } else {
@@ -2425,6 +2474,7 @@ void libxl__ev_qmp_init(libxl__ev_qmp *ev)
     ev->pipe_cfd_read = NULL;
     ev->pipe_cfd_write = NULL;
     ev->vchan = NULL;
+    ev->vchan_lock = NULL;
     libxl__xswait_init(&ev->xswait);
     ev->xswait.what = NULL;
     ev->xswait.path = NULL;
@@ -2497,6 +2547,8 @@ void libxl__ev_qmp_dispose(libxl__gc *gc, libxl__ev_qmp *ev)
         libxenvchan_close(ev->vchan);
     else
         libxl__carefd_close(ev->cfd);
+    if (ev->vchan_lock)
+        libxl__carefd_close(ev->vchan_lock);
     if (ev->pipe_cfd_read) {
         libxl__ev_fd_deregister(gc, &ev->pipe_efd);
         libxl__carefd_close(ev->pipe_cfd_read);
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 17/17] libxl: require qemu in dom0 even if stubdomain is in use
  2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (15 preceding siblings ...)
  2019-01-28 21:30 ` [PATCH v3 16/17] libxl: add locking for libvchan QMP connection Marek Marczykowski-Górecki
@ 2019-01-28 21:30 ` Marek Marczykowski-Górecki
  16 siblings, 0 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-01-28 21:30 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki

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>
---
 tools/libxl/libxl_dm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index ce65321..0fdc2f8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2438,7 +2438,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, since the above code always adds at least 2 consoles.
+     */
+    need_qemu = 1;
 
     for (i = 0; i < num_console; i++) {
         libxl__device device;
-- 
git-series 0.9.1

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol
  2019-01-28 21:30 ` [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
@ 2019-02-12 10:52   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-12 10:52 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich, xen-devel

On Mon, Jan 28, 2019 at 10:30:18PM +0100, Marek Marczykowski-Górecki wrote:
> Add documentation based on reverse-engineered toolstack-ioemu stubdomain
> protocol.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

This matches my (vague) understanding of how it works. Thanks for
writing it down!

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 02/17] Document ioemu Linux stubdomain protocol
  2019-01-28 21:30 ` [PATCH v3 02/17] Document ioemu Linux " Marek Marczykowski-Górecki
@ 2019-02-21 15:39   ` Wei Liu
  2019-02-21 17:08     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 34+ messages in thread
From: Wei Liu @ 2019-02-21 15:39 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich, xen-devel

On Mon, Jan 28, 2019 at 10:30:19PM +0100, Marek Marczykowski-Górecki wrote:
> Add documentation for upcoming Linux stubdomain for qemu-upstream.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  docs/misc/stubdom.txt | 50 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+)
> 
> diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
> index 4c524f2..9c94c6b 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

Since you're defining a new protocol here, you have the liberty to
eliminate this uncertainty, unless for some reason you want it to be
compatible with the old stubdom?


Wei.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain
  2019-01-28 21:30 ` [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
@ 2019-02-21 16:01   ` Wei Liu
  2019-02-21 17:06     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:01 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:21PM +0100, Marek Marczykowski-Górecki wrote:
> 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>
> 
> ---
> 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 a4e74a5..bb62542 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -160,15 +160,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 459f9bf..b8c698a 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2195,6 +2195,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;

I don't think the logic is accurate. You're precluding running
qemu-xen in a unikernel as stubdom.

I think putting an extra key in xenstore with the underlying platform is
more desirable.

> +}
> +
> +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;

Subsequently you will need a new field in b_info.

What do you think?

Wei.

> +}
> +
>  #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)
> -- 
> git-series 0.9.1

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 05/17] libxl: Handle Linux stubdomain specific QEMU options.
  2019-01-28 21:30 ` [PATCH v3 05/17] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
@ 2019-02-21 16:02   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Simon Gaiser, xen-devel, Ian Jackson, Wei Liu, Eric Shelton

On Mon, Jan 28, 2019 at 10:30:22PM +0100, Marek Marczykowski-Górecki wrote:
> 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"
>  * pass downscript for network interfaces
> 
> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
> [drop Qubes-specific parts]
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 06/17] libxl: write qemu arguments into separate xenstore keys
  2019-01-28 21:30 ` [PATCH v3 06/17] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
@ 2019-02-21 16:02   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:23PM +0100, Marek Marczykowski-Górecki wrote:
> This allows using arguments with spaces, like -append, without
> nominating any special "separator" character.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 07/17] libxl: create vkb device only for guests with graphics output
  2019-01-28 21:30 ` [PATCH v3 07/17] libxl: create vkb device only for guests with graphics output Marek Marczykowski-Górecki
@ 2019-02-21 16:02   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:24PM +0100, Marek Marczykowski-Górecki wrote:
> The forced vkb device is meant for better performance of qemu access
> (at least according to ebbd2561b4cefb299f0f68a88b2788504223de18 "libxl:
> Add a vkbd frontend/backend pair for HVM guests"), which isn't used if
> there is no configured channel to actually access that keyboard.
> 
> One can still add vkb device manually if needed.
> 
> This is continuation of b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do
> not start dom0 qemu for stubdomain when not needed".
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 10/17] libxl: typo fix in comment
  2019-01-28 21:30 ` [PATCH v3 10/17] libxl: typo fix in comment Marek Marczykowski-Górecki
@ 2019-02-21 16:02   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:27PM +0100, Marek Marczykowski-Górecki wrote:
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 11/17] libxl: move xswait declaration up in libxl_internal.h
  2019-01-28 21:30 ` [PATCH v3 11/17] libxl: move xswait declaration up in libxl_internal.h Marek Marczykowski-Górecki
@ 2019-02-21 16:02   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:28PM +0100, Marek Marczykowski-Górecki wrote:
> It will be needed for qmp_ev_* over vchan.
> 
> No functional change.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 08/17] xl: add stubdomain related options to xl config parser
  2019-01-28 21:30 ` [PATCH v3 08/17] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
@ 2019-02-21 16:02   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:25PM +0100, Marek Marczykowski-Górecki wrote:
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> ---
>  docs/man/xl.cfg.5.pod.in | 23 +++++++++++++++++++----
>  tools/xl/xl_parse.c      |  7 +++++++
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index 3b92f39..f475196 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -2694,10 +2694,25 @@ 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.


Where is the documentation for stubdom_memory?

>  
>  =item B<device_model_stubdomain_override=BOOLEAN>
>  
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 352cd21..77e4cf0 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2502,6 +2502,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);            \
> -- 
> git-series 0.9.1

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 09/17] tools/libvchan: notify server when client is connected
  2019-01-28 21:30 ` [PATCH v3 09/17] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
@ 2019-02-21 16:02   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:26PM +0100, Marek Marczykowski-Górecki wrote:
> 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>
> ---
> 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 180833d..50a64c1 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);
> +

Indentation is wrong. Fix it and:

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 15/17] tools: add missing libxenvchan cflags
  2019-01-28 21:30 ` [PATCH v3 15/17] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
@ 2019-02-21 16:05   ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 16:05 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Mon, Jan 28, 2019 at 10:30:32PM +0100, Marek Marczykowski-Górecki wrote:
> 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>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain
  2019-02-21 16:01   ` Wei Liu
@ 2019-02-21 17:06     ` Marek Marczykowski-Górecki
  2019-02-21 17:58       ` Wei Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-02-21 17:06 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, Ian Jackson


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

On Thu, Feb 21, 2019 at 04:01:59PM +0000, Wei Liu wrote:
> On Mon, Jan 28, 2019 at 10:30:21PM +0100, Marek Marczykowski-Górecki wrote:
> > 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>
> > 
> > ---
> > 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_internal.h b/tools/libxl/libxl_internal.h
> > index 459f9bf..b8c698a 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -2195,6 +2195,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;
> 
> I don't think the logic is accurate. You're precluding running
> qemu-xen in a unikernel as stubdom.
> 
> I think putting an extra key in xenstore with the underlying platform is
> more desirable.
> 
> > +}
> > +
> > +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;
> 
> Subsequently you will need a new field in b_info.
> 
> What do you think?

This is _exactly_ what was in v2 of this patch and Ian suggested to
change it:
https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01317.html

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

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

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 02/17] Document ioemu Linux stubdomain protocol
  2019-02-21 15:39   ` Wei Liu
@ 2019-02-21 17:08     ` Marek Marczykowski-Górecki
  2019-02-21 17:31       ` Wei Liu
  0 siblings, 1 reply; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-02-21 17:08 UTC (permalink / raw)
  To: Wei Liu
  Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
	Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall,
	Jan Beulich, xen-devel


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

On Thu, Feb 21, 2019 at 03:39:25PM +0000, Wei Liu wrote:
> On Mon, Jan 28, 2019 at 10:30:19PM +0100, Marek Marczykowski-Górecki wrote:
> > Add documentation for upcoming Linux stubdomain for qemu-upstream.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> >  docs/misc/stubdom.txt | 50 ++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 50 insertions(+)
> > 
> > diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
> > index 4c524f2..9c94c6b 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
> 
> Since you're defining a new protocol here, you have the liberty to
> eliminate this uncertainty, unless for some reason you want it to be
> compatible with the old stubdom?

I'm not sure who access this xenstore key, since I haven't found how is
it used in minios based stubdomain. Is it used by qemu?

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

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

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 02/17] Document ioemu Linux stubdomain protocol
  2019-02-21 17:08     ` Marek Marczykowski-Górecki
@ 2019-02-21 17:31       ` Wei Liu
  2019-02-21 18:17         ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 34+ messages in thread
From: Wei Liu @ 2019-02-21 17:31 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
	Julien Grall, Jan Beulich, xen-devel

On Thu, Feb 21, 2019 at 06:08:22PM +0100, Marek Marczykowski-Górecki wrote:
> On Thu, Feb 21, 2019 at 03:39:25PM +0000, Wei Liu wrote:
> > On Mon, Jan 28, 2019 at 10:30:19PM +0100, Marek Marczykowski-Górecki wrote:
> > > Add documentation for upcoming Linux stubdomain for qemu-upstream.
> > > 
> > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > > ---
> > >  docs/misc/stubdom.txt | 50 ++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 50 insertions(+)
> > > 
> > > diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
> > > index 4c524f2..9c94c6b 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
> > 
> > Since you're defining a new protocol here, you have the liberty to
> > eliminate this uncertainty, unless for some reason you want it to be
> > compatible with the old stubdom?
> 
> I'm not sure who access this xenstore key, since I haven't found how is
> it used in minios based stubdomain. Is it used by qemu?

It is read by hvmloader afaict.

Wei.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain
  2019-02-21 17:06     ` Marek Marczykowski-Górecki
@ 2019-02-21 17:58       ` Wei Liu
  0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2019-02-21 17:58 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Wei Liu, Ian Jackson

On Thu, Feb 21, 2019 at 06:06:31PM +0100, Marek Marczykowski-Górecki wrote:
> On Thu, Feb 21, 2019 at 04:01:59PM +0000, Wei Liu wrote:
> > On Mon, Jan 28, 2019 at 10:30:21PM +0100, Marek Marczykowski-Górecki wrote:
> > > 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>
> > > 
> > > ---
> > > 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_internal.h b/tools/libxl/libxl_internal.h
> > > index 459f9bf..b8c698a 100644
> > > --- a/tools/libxl/libxl_internal.h
> > > +++ b/tools/libxl/libxl_internal.h
> > > @@ -2195,6 +2195,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;
> > 
> > I don't think the logic is accurate. You're precluding running
> > qemu-xen in a unikernel as stubdom.
> > 
> > I think putting an extra key in xenstore with the underlying platform is
> > more desirable.
> > 
> > > +}
> > > +
> > > +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;
> > 
> > Subsequently you will need a new field in b_info.
> > 
> > What do you think?
> 
> This is _exactly_ what was in v2 of this patch and Ian suggested to
> change it:
> https://lists.xenproject.org/archives/html/xen-devel/2018-10/msg01317.html

Alright. Ian thought unikernel qemu-xen stubdomain was not coming along
any time soon. I don't disagree. You can keep the patch as-is.

I think in the future if it does come along, we can still easily
distinguish it from the rest.

Wei.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 02/17] Document ioemu Linux stubdomain protocol
  2019-02-21 17:31       ` Wei Liu
@ 2019-02-21 18:17         ` Marek Marczykowski-Górecki
  0 siblings, 0 replies; 34+ messages in thread
From: Marek Marczykowski-Górecki @ 2019-02-21 18:17 UTC (permalink / raw)
  To: Wei Liu
  Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, George Dunlap,
	Andrew Cooper, Tim Deegan, Julien Grall, Jan Beulich, xen-devel,
	Ian Jackson


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

On Thu, Feb 21, 2019 at 05:31:54PM +0000, Wei Liu wrote:
> On Thu, Feb 21, 2019 at 06:08:22PM +0100, Marek Marczykowski-Górecki wrote:
> > On Thu, Feb 21, 2019 at 03:39:25PM +0000, Wei Liu wrote:
> > > On Mon, Jan 28, 2019 at 10:30:19PM +0100, Marek Marczykowski-Górecki wrote:
> > > > Add documentation for upcoming Linux stubdomain for qemu-upstream.
> > > > 
> > > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > > > ---
> > > >  docs/misc/stubdom.txt | 50 ++++++++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 50 insertions(+)
> > > > 
> > > > diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt
> > > > index 4c524f2..9c94c6b 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
> > > 
> > > Since you're defining a new protocol here, you have the liberty to
> > > eliminate this uncertainty, unless for some reason you want it to be
> > > compatible with the old stubdom?
> > 
> > I'm not sure who access this xenstore key, since I haven't found how is
> > it used in minios based stubdomain. Is it used by qemu?
> 
> It is read by hvmloader afaict.

Ah, in that case I think it should be removed from this (and minios)
spec, because it is irrelevant to stubdomain.

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

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

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

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2019-02-21 18:17 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-28 21:30 [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
2019-01-28 21:30 ` [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
2019-02-12 10:52   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 02/17] Document ioemu Linux " Marek Marczykowski-Górecki
2019-02-21 15:39   ` Wei Liu
2019-02-21 17:08     ` Marek Marczykowski-Górecki
2019-02-21 17:31       ` Wei Liu
2019-02-21 18:17         ` Marek Marczykowski-Górecki
2019-01-28 21:30 ` [PATCH v3 03/17] libxl: fix qemu-trad cmdline for no sdl/vnc case Marek Marczykowski-Górecki
2019-01-28 21:30 ` [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
2019-02-21 16:01   ` Wei Liu
2019-02-21 17:06     ` Marek Marczykowski-Górecki
2019-02-21 17:58       ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 05/17] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
2019-02-21 16:02   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 06/17] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
2019-02-21 16:02   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 07/17] libxl: create vkb device only for guests with graphics output Marek Marczykowski-Górecki
2019-02-21 16:02   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 08/17] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
2019-02-21 16:02   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 09/17] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
2019-02-21 16:02   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 10/17] libxl: typo fix in comment Marek Marczykowski-Górecki
2019-02-21 16:02   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 11/17] libxl: move xswait declaration up in libxl_internal.h Marek Marczykowski-Górecki
2019-02-21 16:02   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 12/17] libxl: use vchan for QMP access with Linux stubdomain, libxl__ev_qmp_* version Marek Marczykowski-Górecki
2019-01-28 21:30 ` [PATCH v3 13/17] libxl: use vchan for QMP access with Linux stubdomain, non-async version Marek Marczykowski-Górecki
2019-01-28 21:30 ` [PATCH v3 14/17] libxl: add save/restore support for qemu-xen in stubdomain Marek Marczykowski-Górecki
2019-01-28 21:30 ` [PATCH v3 15/17] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
2019-02-21 16:05   ` Wei Liu
2019-01-28 21:30 ` [PATCH v3 16/17] libxl: add locking for libvchan QMP connection Marek Marczykowski-Górecki
2019-01-28 21:30 ` [PATCH v3 17/17] libxl: require qemu in dom0 even if stubdomain is in use Marek Marczykowski-Górecki

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.