Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain.
@ 2020-01-15  2:39 Marek Marczykowski-Górecki
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 01/16] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
                   ` (16 more replies)
  0 siblings, 17 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 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. The actual connection
is made in a separate process. As discussed on Xen Summit 2019, this allows to
apply some basic checks and/or filtering (not part of this series), to limit
libxl exposure for potentially malicious stubdomain.

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
Changes in v4:
 - extract vchan connection into a separate process
 - rebase on master
 - various fixes

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 (15):
  Document ioemu MiniOS stubdomain protocol
  Document ioemu Linux stubdomain protocol
  libxl: fix qemu-trad cmdline for no sdl/vnc case
  libxl: Allow running qemu-xen in stubdomain
  libxl: write qemu arguments into separate xenstore keys
  xl: add stubdomain related options to xl config parser
  tools/libvchan: notify server when client is connected
  libxl: add save/restore support for qemu-xen in stubdomain
  tools: add missing libxenvchan cflags
  tools: add simple vchan-socket-proxy
  libxl: use vchan for QMP access with Linux stubdomain
  Regenerate autotools files
  libxl: require qemu in dom0 even if stubdomain is in use
  libxl: ignore emulated IDE disks beyond the first 4
  libxl: consider also qemu in stubdomain in libxl__dm_active check

 .gitignore                          |   1 +-
 configure                           |  14 +-
 docs/configure                      |  14 +-
 docs/man/xl.cfg.5.pod.in            |  23 +-
 docs/misc/stubdom.txt               | 103 ++++++-
 stubdom/configure                   |  14 +-
 tools/Rules.mk                      |   2 +-
 tools/config.h.in                   |   3 +-
 tools/configure                     |  46 +--
 tools/configure.ac                  |   9 +-
 tools/libvchan/Makefile             |   7 +-
 tools/libvchan/init.c               |   3 +-
 tools/libvchan/init.c.rej           |  60 ++++-
 tools/libvchan/vchan-socket-proxy.c | 469 +++++++++++++++++++++++++++++-
 tools/libxl/libxl_create.c          |  37 +-
 tools/libxl/libxl_dm.c              | 437 ++++++++++++++++++++++-----
 tools/libxl/libxl_internal.h        |  19 +-
 tools/libxl/libxl_mem.c             |   6 +-
 tools/libxl/libxl_qmp.c             |  25 +-
 tools/libxl/libxl_types.idl         |   3 +-
 tools/xl/xl_parse.c                 |   7 +-
 21 files changed, 1151 insertions(+), 151 deletions(-)
 create mode 100644 tools/libvchan/init.c.rej
 create mode 100644 tools/libvchan/vchan-socket-proxy.c

base-commit: fae249d23413b2bf7d98a97d8f649cf7d102c1ae
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 01/16] Document ioemu MiniOS stubdomain protocol
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 18:30   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 02/16] Document ioemu Linux " Marek Marczykowski-Górecki
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, 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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 02/16] Document ioemu Linux stubdomain protocol
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 01/16] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 18:54   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 03/16] libxl: fix qemu-trad cmdline for no sdl/vnc case Marek Marczykowski-Górecki
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, 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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 03/16] libxl: fix qemu-trad cmdline for no sdl/vnc case
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 01/16] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 02/16] Document ioemu Linux " Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 04/16] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Wei Liu, Jason Andryuk, Ian Jackson,
	Marek Marczykowski-Górecki, Anthony PERARD

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 e92e412..558cb41 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -718,14 +718,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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 04/16] libxl: Allow running qemu-xen in stubdomain
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (2 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 03/16] libxl: fix qemu-trad cmdline for no sdl/vnc case Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 18:56   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

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 32d45dc..142b960 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -169,15 +169,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 ba8c9b4..cc3cf26 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2299,6 +2299,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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options.
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (3 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 04/16] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 19:24   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki,
	Simon Gaiser, Anthony PERARD, 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   |  36 +++++++-
 tools/libxl/libxl_dm.c       | 190 ++++++++++++++++++++++++------------
 tools/libxl/libxl_internal.h |   1 +-
 tools/libxl/libxl_mem.c      |   6 +-
 tools/libxl/libxl_types.idl  |   3 +-
 5 files changed, 175 insertions(+), 61 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 142b960..a6d40b7 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -169,6 +169,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) {
@@ -204,6 +229,17 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
     if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
         b_info->target_memkb = b_info->max_memkb;
 
+    if (b_info->stubdomain_memkb == LIBXL_MEMKB_DEFAULT) {
+        if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+            if (libxl__stubdomain_is_linux(b_info))
+                b_info->stubdomain_memkb = LIBXL_LINUX_STUBDOM_MEM * 1024;
+            else
+                b_info->stubdomain_memkb = 28 * 1024; // MiniOS
+        } else {
+            b_info->stubdomain_memkb = 0; // no stubdomain
+        }
+    }
+
     libxl_defbool_setdefault(&b_info->claim_mode, false);
 
     libxl_defbool_setdefault(&b_info->localtime, false);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 558cb41..926d963 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1172,6 +1172,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);
@@ -1182,38 +1183,41 @@ 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));
+    /* There is currently no way to access the QMP socket in the stubdom */
+    if (!is_stubdom) {
+        flexarray_append(dm_args, "-chardev");
+        if (state->dm_monitor_fd >= 0) {
+            flexarray_append(dm_args,
+                GCSPRINTF("socket,id=libxl-cmd,fd=%d,server,nowait",
+                          state->dm_monitor_fd));
 
-        /*
-         * Start QEMU with its "CPU" paused, it will not start any emulation
-         * until the QMP command "cont" is used. This also prevent QEMU from
-         * writing "running" to the "state" xenstore node so we only use this
-         * flag when we have the QMP based startup notification.
-         * */
-        flexarray_append(dm_args, "-S");
-    } else {
-        flexarray_append(dm_args,
-                         GCSPRINTF("socket,id=libxl-cmd,"
-                                   "path=%s,server,nowait",
-                                   libxl__qemu_qmp_path(gc, guest_domid)));
-    }
+            /*
+             * Start QEMU with its "CPU" paused, it will not start any emulation
+             * until the QMP command "cont" is used. This also prevent QEMU from
+             * writing "running" to the "state" xenstore node so we only use this
+             * flag when we have the QMP based startup notification.
+             * */
+            flexarray_append(dm_args, "-S");
+        } else {
+            flexarray_append(dm_args,
+                             GCSPRINTF("socket,id=libxl-cmd,"
+                                       "path=%s,server,nowait",
+                                       libxl__qemu_qmp_path(gc, guest_domid)));
+        }
 
-    flexarray_append(dm_args, "-no-shutdown");
-    flexarray_append(dm_args, "-mon");
-    flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
+        flexarray_append(dm_args, "-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;
@@ -1257,7 +1261,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");
@@ -1296,7 +1300,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.
          */
@@ -1308,7 +1312,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);
@@ -1350,18 +1354,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);
+                    }
                 }
             }
         }
@@ -1370,7 +1390,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)
@@ -1797,7 +1817,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);
@@ -1808,6 +1830,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,
@@ -1948,7 +1980,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) {
@@ -1958,8 +1990,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,
@@ -2064,6 +2098,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);
@@ -2122,10 +2166,14 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     dmss_init(&sdss->pvqemu);
     libxl__xswait_init(&sdss->xswait);
 
-    if (guest_config->b_info.device_model_version !=
-        LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
-        ret = ERROR_INVAL;
-        goto out;
+    assert(libxl_defbool_val(guest_config->b_info.device_model_stubdomain));
+
+    if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
+        if (d_state->saved_state) {
+            LOG(ERROR, "Save/Restore not supported yet with Linux Stubdom.");
+            ret = -1;
+            goto out;
+        }
     }
 
     sdss->pvqemu.guest_domid = 0;
@@ -2147,8 +2195,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
 
     dm_config->b_info.shadow_memkb = 0;
     dm_config->b_info.max_vcpus = 1;
-    dm_config->b_info.max_memkb = 28 * 1024 +
-        guest_config->b_info.video_memkb;
+    dm_config->b_info.max_memkb = guest_config->b_info.stubdomain_memkb;
+    dm_config->b_info.max_memkb += guest_config->b_info.video_memkb;
     dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
 
     dm_config->b_info.max_grant_frames = guest_config->b_info.max_grant_frames;
@@ -2187,10 +2235,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,
@@ -2210,6 +2256,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",
@@ -2219,6 +2267,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",
@@ -2300,6 +2357,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));
 
@@ -2333,8 +2395,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 cc3cf26..2b4a1cc 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -119,6 +119,7 @@
 #define STUBDOM_CONSOLE_RESTORE 2
 #define STUBDOM_CONSOLE_SERIAL 3
 #define STUBDOM_SPECIAL_CONSOLES 3
+#define LIBXL_LINUX_STUBDOM_MEM 128
 #define TAP_DEVICE_SUFFIX "-emu"
 #define DOMID_XS_PATH "domid"
 #define INVALID_DOMID ~0
diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
index 7c01fac..65957a4 100644
--- a/tools/libxl/libxl_mem.c
+++ b/tools/libxl/libxl_mem.c
@@ -459,8 +459,10 @@ int libxl__domain_need_memory_calculate(libxl__gc *gc,
     case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_HVM:
         *need_memkb += LIBXL_HVM_EXTRA_MEMORY;
-        if (libxl_defbool_val(b_info->device_model_stubdomain))
-            *need_memkb += 32 * 1024;
+        if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+            *need_memkb += b_info->stubdomain_memkb;
+            *need_memkb += b_info->video_memkb;
+        }
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         *need_memkb += LIBXL_PV_EXTRA_MEMORY;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 7921950..011676f 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -516,6 +516,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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (4 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 19:36   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

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 926d963..bf49262 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2049,6 +2049,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)
@@ -2258,7 +2292,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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (5 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 19:41   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Ian Jackson, Marek Marczykowski-Górecki, Wei Liu, Jason Andryuk

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 245d3f9..6ae0bd0 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2720,10 +2720,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 b881184..fc5dd65 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2525,6 +2525,13 @@ skip_usbdev:
     xlu_cfg_replace_string(config, "device_model_user",
                            &b_info->device_model_user, 0);
 
+    xlu_cfg_replace_string (config, "stubdomain_kernel",
+                            &b_info->stubdomain_kernel, 0);
+    xlu_cfg_replace_string (config, "stubdomain_ramdisk",
+                            &b_info->stubdomain_ramdisk, 0);
+    if (!xlu_cfg_get_long (config, "stubdomain_memory", &l, 0))
+        b_info->stubdomain_memkb = l * 1024;
+
 #define parse_extra_args(type)                                            \
     e = xlu_cfg_get_list_as_string_list(config, "device_model_args"#type, \
                                     &b_info->extra##type, 0);            \
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (6 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 19:44   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 09/16] libxl: add save/restore support for qemu-xen in stubdomain Marek Marczykowski-Górecki
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 09/16] libxl: add save/restore support for qemu-xen in stubdomain
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (7 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 10/16] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

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 bf49262..528ca3e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1728,10 +1728,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]);
@@ -2202,14 +2209,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 efaba91..0f38546 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -962,6 +962,7 @@ static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
                        const libxl__json_object *response, int rc);
 static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
                               const libxl__json_object *response, int rc);
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int fdset);
 static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
                            const libxl__json_object *response, int rc);
 
@@ -994,10 +995,17 @@ static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
     EGC_GC;
     libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
     const char *const filename = dsps->dm_savefile;
+    uint32_t dm_domid = libxl_get_stubdom_id(CTX, dsps->domid);
 
     if (rc)
         goto error;
 
+    if (dm_domid) {
+        /* see Linux stubdom interface in docs/stubdom.txt */
+        dm_state_save_to_fdset(egc, ev, 1);
+        return;
+    }
+
     ev->payload_fd = open(filename, O_WRONLY | O_CREAT, 0600);
     if (ev->payload_fd < 0) {
         LOGED(ERROR, ev->domid,
@@ -1028,7 +1036,6 @@ static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
     EGC_GC;
     int fdset;
     const libxl__json_object *o;
-    libxl__json_object *args = NULL;
     libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
 
     close(ev->payload_fd);
@@ -1043,6 +1050,21 @@ static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
         goto error;
     }
     fdset = libxl__json_object_get_integer(o);
+    dm_state_save_to_fdset(egc, ev, fdset);
+    return;
+
+error:
+    assert(rc);
+    libxl__remove_file(gc, dsps->dm_savefile);
+    dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int fdset)
+{
+    EGC_GC;
+    int rc;
+    libxl__json_object *args = NULL;
+    libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
 
     ev->callback = dm_state_saved;
 
@@ -1060,6 +1082,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	[flat|nested] 53+ messages in thread

* [Xen-devel] [PATCH v4 10/16] tools: add missing libxenvchan cflags
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (8 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 09/16] libxl: add save/restore support for qemu-xen in stubdomain Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-20 19:58   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy Marek Marczykowski-Górecki
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

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 31cf419..9c550c1 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -159,7 +159,7 @@ SHDEPS_libxenstat  = $(SHLIB_libxenctrl) $(SHLIB_libxenstore)
 LDLIBS_libxenstat  = $(SHDEPS_libxenstat) $(XEN_LIBXENSTAT)/libxenstat$(libextension)
 SHLIB_libxenstat   = $(SHDEPS_libxenstat) -Wl,-rpath-link=$(XEN_LIBXENSTAT)
 
-CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
+CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
 SHDEPS_libxenvchan = $(SHLIB_libxentoollog) $(SHLIB_libxenstore) $(SHLIB_libxenevtchn) $(SHLIB_libxengnttab)
 LDLIBS_libxenvchan = $(SHDEPS_libxenvchan) $(XEN_LIBVCHAN)/libxenvchan$(libextension)
 SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (9 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 10/16] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-15 11:02   ` Jan Beulich
                     ` (2 more replies)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain Marek Marczykowski-Górecki
                   ` (5 subsequent siblings)
  16 siblings, 3 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson,
	Marek Marczykowski-Górecki, Jan Beulich

Add a simple proxy for tunneling socket connection over vchan. This is
based on existing vchan-node* applications, but extended with socket
support. vchan-socket-proxy serves both as a client and as a server,
depending on parameters. It can be used to transparently communicate
with an application in another domian that normally expose UNIX socket
interface. Specifically, it's written to communicate with qemu running
within stubdom.

Server mode listens for vchan connections and when one is opened,
connects to a pointed UNIX socket.  Client mode listens on UNIX
socket and when someone connects, opens a vchan connection.  Only
a single connection at a time is supported.

Additionally, socket can be provided as a number - in which case it's
interpreted as already open FD (in case of UNIX listening socket -
listen() needs to be already called). Or "-" meaning stdin/stdout - in
which case it is reduced to vchan-node2 functionality.

Example usage:

1. (in dom0) vchan-socket-proxy --mode=client <DOMID>
    /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)

2. (in DOMID) vchan-socket-proxy --mode=server 0
   /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)

This will listen on /run/qemu.(DOMID) in dom0 and whenever connection is
made, it will connect to DOMID, where server process will connect to
/run/qemu.(DOMID) there. When client disconnects, vchan connection is
terminated and server vchan-socket-proxy process also disconnects from
qemu.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 .gitignore                          |   1 +-
 tools/libvchan/Makefile             |   7 +-
 tools/libvchan/init.c.rej           |  60 ++++-
 tools/libvchan/vchan-socket-proxy.c | 469 +++++++++++++++++++++++++++++-
 4 files changed, 536 insertions(+), 1 deletion(-)
 create mode 100644 tools/libvchan/init.c.rej
 create mode 100644 tools/libvchan/vchan-socket-proxy.c

diff --git a/.gitignore b/.gitignore
index 017856c..1c9dd93 100644
--- a/.gitignore
+++ b/.gitignore
@@ -372,6 +372,7 @@ tools/misc/xenwatchdogd
 tools/misc/xen-hvmcrash
 tools/misc/xen-lowmemd
 tools/libvchan/vchan-node[12]
+tools/libvchan/vchan-socket-proxy
 tools/ocaml/*/.ocamldep.make
 tools/ocaml/*/*.cm[ixao]
 tools/ocaml/*/*.cmxa
diff --git a/tools/libvchan/Makefile b/tools/libvchan/Makefile
index 7892750..1c845ca 100644
--- a/tools/libvchan/Makefile
+++ b/tools/libvchan/Makefile
@@ -13,6 +13,7 @@ LIBVCHAN_PIC_OBJS = $(patsubst %.o,%.opic,$(LIBVCHAN_OBJS))
 LIBVCHAN_LIBS = $(LDLIBS_libxenstore) $(LDLIBS_libxengnttab) $(LDLIBS_libxenevtchn)
 $(LIBVCHAN_OBJS) $(LIBVCHAN_PIC_OBJS): CFLAGS += $(CFLAGS_libxenstore) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
 $(NODE_OBJS) $(NODE2_OBJS): CFLAGS += $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
+vchan-socket-proxy.o: CFLAGS += $(CFLAGS_libxenstore) $(CFLAGS_libxenctrl) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
 
 MAJOR = 4.14
 MINOR = 0
@@ -39,7 +40,7 @@ $(PKG_CONFIG_LOCAL): PKG_CONFIG_LIBDIR = $(CURDIR)
 $(PKG_CONFIG_LOCAL): PKG_CONFIG_CFLAGS_LOCAL = $(CFLAGS_xeninclude)
 
 .PHONY: all
-all: libxenvchan.so vchan-node1 vchan-node2 libxenvchan.a $(PKG_CONFIG_INST) $(PKG_CONFIG_LOCAL)
+all: libxenvchan.so vchan-node1 vchan-node2 vchan-socket-proxy libxenvchan.a $(PKG_CONFIG_INST) $(PKG_CONFIG_LOCAL)
 
 libxenvchan.so: libxenvchan.so.$(MAJOR)
 	ln -sf $< $@
@@ -59,6 +60,9 @@ vchan-node1: $(NODE_OBJS) libxenvchan.so
 vchan-node2: $(NODE2_OBJS) libxenvchan.so
 	$(CC) $(LDFLAGS) -o $@ $(NODE2_OBJS) $(LDLIBS_libxenvchan) $(APPEND_LDFLAGS)
 
+vchan-socket-proxy: vchan-socket-proxy.o libxenvchan.so
+	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+
 .PHONY: install
 install: all
 	$(INSTALL_DIR) $(DESTDIR)$(libdir)
@@ -66,6 +70,7 @@ install: all
 	$(INSTALL_PROG) libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)
 	ln -sf libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)/libxenvchan.so.$(MAJOR)
 	ln -sf libxenvchan.so.$(MAJOR) $(DESTDIR)$(libdir)/libxenvchan.so
+	$(INSTALL_PROG) vchan-socket-proxy $(DESTDIR)$(bindir)
 	$(INSTALL_DATA) libxenvchan.h $(DESTDIR)$(includedir)
 	$(INSTALL_DATA) libxenvchan.a $(DESTDIR)$(libdir)
 	$(INSTALL_DATA) xenvchan.pc $(DESTDIR)$(PKG_INSTALLDIR)
diff --git a/tools/libvchan/init.c.rej b/tools/libvchan/init.c.rej
new file mode 100644
index 0000000..8b3ed73
--- /dev/null
+++ b/tools/libvchan/init.c.rej
@@ -0,0 +1,60 @@
+--- tools/libvchan/init.c
++++ tools/libvchan/init.c
+@@ -266,31 +266,33 @@ static int init_xs_srv(struct libxenvchan *ctrl, int domain, const char* xs_base
+ 	perms[1].id = domain;
+ 	perms[1].perms = XS_PERM_READ;
+ 
+-retry_transaction:
+-	xs_trans = xs_transaction_start(xs);
+-	if (!xs_trans)
+-		goto fail;
+-
+-	snprintf(ref, sizeof ref, "%d", ring_ref);
+-	snprintf(buf, sizeof buf, "%s/ring-ref", xs_base);
+-	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
+-		goto fail;
+-	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
+-		goto fail;
+-
+-	snprintf(ref, sizeof ref, "%d", ctrl->event_port);
+-	snprintf(buf, sizeof buf, "%s/event-channel", xs_base);
+-	if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
+-		goto fail;
+-	if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
+-		goto fail;
+-
+-	if (!xs_transaction_end(xs, xs_trans, 0)) {
+-		if (errno == EAGAIN)
+-			goto retry_transaction;
+-	} else {
+-		ret = 0;
++	for (;;) {
++		xs_trans = xs_transaction_start(xs);
++		if (!xs_trans)
++			goto fail;
++
++		snprintf(ref, sizeof ref, "%d", ring_ref);
++		snprintf(buf, sizeof buf, "%s/ring-ref", xs_base);
++		if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
++			goto fail;
++		if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
++			goto fail;
++
++		snprintf(ref, sizeof ref, "%d", ctrl->event_port);
++		snprintf(buf, sizeof buf, "%s/event-channel", xs_base);
++		if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
++			goto fail;
++		if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
++			goto fail;
++
++		if (xs_transaction_end(xs, xs_trans, 0))
++			break;
++		else if (errno != EAGAIN)
++			goto fail;
++		/* EAGAIN, retry */
+ 	}
++	ret = 0;
++
+  fail:
+ 	free(domid_str);
+ 	xs_close(xs);
diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
new file mode 100644
index 0000000..6b4ae09
--- /dev/null
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -0,0 +1,469 @@
+/**
+ * @file
+ * @section AUTHORS
+ *
+ * Copyright (C) 2010  Rafal Wojtczuk  <rafal@invisiblethingslab.com>
+ *
+ *  Authors:
+ *       Rafal Wojtczuk  <rafal@invisiblethingslab.com>
+ *       Daniel De Graaf <dgdegra@tycho.nsa.gov>
+ *       Marek Marczykowski-Górecki  <marmarek@invisiblethingslab.com>
+ *
+ * @section LICENSE
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @section DESCRIPTION
+ *
+ * This is a vchan to unix socket proxy. Vchan server is set, and on client
+ * connection, local socket connection is established. Communication is bidirectional.
+ * One client is served at a time, clients needs to coordinate this themselves.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <getopt.h>
+
+#include <xenstore.h>
+#include <xenctrl.h>
+#include <libxenvchan.h>
+
+static void usage(char** argv)
+{
+    fprintf(stderr, "usage:\n"
+        "\t%s [options] domainid nodepath [socket-path|file-no|-]\n"
+        "\n"
+        "options:\n"
+        "\t-m, --mode=client|server - vchan connection mode\n"
+        "\t-m, --state-path=path - xenstore path where write \"running\" to at startup\n"
+        "\t-v, --verbose - verbose logging\n"
+        "\n"
+        "client: client of a vchan connection, fourth parameter can be:\n"
+        "\tsocket-path: listen on a UNIX socket at this path and connect to vchan\n"
+        "\t  whenever new connection is accepted;\n"
+        "\t  handle multiple _subsequent_ connections, until terminated\n"
+        "\tfile-no: except open FD of a socket in listen mode; otherwise similar to socket-path\n"
+        "\t-: open vchan connection immediately and pass the data from stdin/stdout;\n"
+        "\t  terminate when vchan connection is closed\n"
+        "server: server of a vchan connection, fourth parameter can be:\n"
+        "\tsocket-path: connect to this UNIX socket when new vchan connection is accepted\n"
+        "\t  handle multiple _subsequent_ connections, until terminated\n"
+        "\tfile-no: pass data to/from this FD; terminate when vchan connection is closed\n"
+        "\t-: pass data to/from stdin/stdout; terminatate when vchan connection is closed\n",
+        argv[0]);
+    exit(1);
+}
+
+#define BUFSIZE 8192
+char inbuf[BUFSIZE];
+char outbuf[BUFSIZE];
+int insiz = 0;
+int outsiz = 0;
+int verbose = 0;
+
+static void vchan_wr(struct libxenvchan *ctrl) {
+    int ret;
+
+    if (!insiz)
+        return;
+    ret = libxenvchan_write(ctrl, inbuf, insiz);
+    if (ret < 0) {
+        fprintf(stderr, "vchan write failed\n");
+        exit(1);
+    }
+    if (verbose)
+        fprintf(stderr, "written %d bytes to vchan\n", ret);
+    if (ret > 0) {
+        insiz -= ret;
+        memmove(inbuf, inbuf + ret, insiz);
+    }
+}
+
+static void socket_wr(int output_fd) {
+    int ret;
+
+    if (!outsiz)
+        return;
+    ret = write(output_fd, outbuf, outsiz);
+    if (ret < 0 && errno != EAGAIN)
+        exit(1);
+    if (ret > 0) {
+        outsiz -= ret;
+        memmove(outbuf, outbuf + ret, outsiz);
+    }
+}
+
+static int set_nonblocking(int fd, int nonblocking) {
+    int flags = fcntl(fd, F_GETFL);
+    if (flags == -1)
+        return -1;
+
+    if (nonblocking)
+        flags |= O_NONBLOCK;
+    else
+        flags &= ~O_NONBLOCK;
+
+    if (fcntl(fd, F_SETFL, flags) == -1)
+        return -1;
+
+    return 0;
+}
+
+static int connect_socket(const char *path_or_fd) {
+    int fd;
+    char *endptr;
+    struct sockaddr_un addr;
+
+    fd = strtoll(path_or_fd, &endptr, 0);
+    if (*endptr == '\0') {
+        set_nonblocking(fd, 1);
+        return fd;
+    }
+
+    fd = socket(AF_UNIX, SOCK_STREAM, 0);
+    if (fd == -1)
+        return -1;
+
+    addr.sun_family = AF_UNIX;
+    strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
+    if (connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        close(fd);
+        return -1;
+    }
+
+    set_nonblocking(fd, 1);
+
+    return fd;
+}
+
+static int listen_socket(const char *path_or_fd) {
+    int fd;
+    char *endptr;
+    struct sockaddr_un addr;
+
+    fd = strtoll(path_or_fd, &endptr, 0);
+    if (*endptr == '\0') {
+        return fd;
+    }
+
+    /* if not a number, assume a socket path */
+    fd = socket(AF_UNIX, SOCK_STREAM, 0);
+    if (fd == -1)
+        return -1;
+
+    addr.sun_family = AF_UNIX;
+    strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
+    if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
+        close(fd);
+        return -1;
+    }
+    if (listen(fd, 5) != 0) {
+        close(fd);
+        return -1;
+    }
+
+    return fd;
+}
+
+static struct libxenvchan *connect_vchan(int domid, const char *path) {
+    struct libxenvchan *ctrl = NULL;
+    struct xs_handle *xs = NULL;
+    xc_interface *xc = NULL;
+    xc_dominfo_t dominfo;
+    char **watch_ret;
+    unsigned int watch_num;
+    int ret;
+
+    xs = xs_open(XS_OPEN_READONLY);
+    if (!xs) {
+        perror("xs_open");
+        goto out;
+    }
+    xc = xc_interface_open(NULL, NULL, XC_OPENFLAG_NON_REENTRANT);
+    if (!xc) {
+        perror("xc_interface_open");
+        goto out;
+    }
+    /* wait for vchan server to create *path* */
+    xs_watch(xs, path, "path");
+    xs_watch(xs, "@releaseDomain", "release");
+    while ((watch_ret = xs_read_watch(xs, &watch_num))) {
+        /* don't care about exact which fired the watch */
+        free(watch_ret);
+        ctrl = libxenvchan_client_init(NULL, domid, path);
+        if (ctrl)
+            break;
+
+        ret = xc_domain_getinfo(xc, domid, 1, &dominfo);
+        /* break the loop if domain is definitely not there anymore, but
+         * continue if it is or the call failed (like EPERM) */
+        if (ret == -1 && errno == ESRCH)
+            break;
+        if (ret == 1 && (dominfo.domid != (uint32_t)domid || dominfo.dying))
+            break;
+    }
+
+out:
+    if (xc)
+        xc_interface_close(xc);
+    if (xs)
+        xs_close(xs);
+    return ctrl;
+}
+
+
+static void discard_buffers(struct libxenvchan *ctrl) {
+    /* discard local buffers */
+    insiz = 0;
+    outsiz = 0;
+
+    /* discard remaining incoming data */
+    while (libxenvchan_data_ready(ctrl)) {
+        if (libxenvchan_read(ctrl, inbuf, BUFSIZE) == -1) {
+            perror("vchan read");
+            exit(1);
+        }
+    }
+}
+
+int data_loop(struct libxenvchan *ctrl, int input_fd, int output_fd)
+{
+    int ret;
+    int libxenvchan_fd;
+    int max_fd;
+
+    libxenvchan_fd = libxenvchan_fd_for_select(ctrl);
+    for (;;) {
+        fd_set rfds;
+        fd_set wfds;
+        FD_ZERO(&rfds);
+        FD_ZERO(&wfds);
+
+        max_fd = -1;
+        if (input_fd != -1 && insiz != BUFSIZE) {
+            FD_SET(input_fd, &rfds);
+            if (input_fd > max_fd)
+                max_fd = input_fd;
+        }
+        if (output_fd != -1 && outsiz) {
+            FD_SET(output_fd, &wfds);
+            if (output_fd > max_fd)
+                max_fd = output_fd;
+        }
+        FD_SET(libxenvchan_fd, &rfds);
+        if (libxenvchan_fd > max_fd)
+            max_fd = libxenvchan_fd;
+        ret = select(max_fd + 1, &rfds, &wfds, NULL, NULL);
+        if (ret < 0) {
+            perror("select");
+            exit(1);
+        }
+        if (FD_ISSET(libxenvchan_fd, &rfds)) {
+            libxenvchan_wait(ctrl);
+            if (!libxenvchan_is_open(ctrl)) {
+                if (verbose)
+                    fprintf(stderr, "vchan client disconnected\n");
+                while (outsiz)
+                    socket_wr(output_fd);
+                close(output_fd);
+                close(input_fd);
+                discard_buffers(ctrl);
+                break;
+            }
+            vchan_wr(ctrl);
+        }
+
+        /* socket_fd guaranteed to be != -1 */
+
+        if (FD_ISSET(input_fd, &rfds)) {
+            ret = read(input_fd, inbuf + insiz, BUFSIZE - insiz);
+            if (ret < 0 && errno != EAGAIN)
+                exit(1);
+            if (verbose)
+                fprintf(stderr, "from-unix: %.*s\n", ret, inbuf + insiz);
+            if (ret == 0) {
+                /* EOF on socket, write everything in the buffer and close the
+                 * socket */
+                while (insiz) {
+                    vchan_wr(ctrl);
+                    libxenvchan_wait(ctrl);
+                }
+                close(input_fd);
+                input_fd = -1;
+                /* TODO: maybe signal the vchan client somehow? */
+                break;
+            }
+            if (ret)
+                insiz += ret;
+            vchan_wr(ctrl);
+        }
+        if (FD_ISSET(output_fd, &wfds))
+            socket_wr(output_fd);
+        while (libxenvchan_data_ready(ctrl) && outsiz < BUFSIZE) {
+            ret = libxenvchan_read(ctrl, outbuf + outsiz, BUFSIZE - outsiz);
+            if (ret < 0)
+                exit(1);
+            if (verbose)
+                fprintf(stderr, "from-vchan: %.*s\n", ret, outbuf + outsiz);
+            outsiz += ret;
+            socket_wr(output_fd);
+        }
+    }
+    return 0;
+}
+
+/**
+    Simple libxenvchan application, both client and server.
+    Both sides may write and read, both from the libxenvchan and from
+    stdin/stdout (just like netcat).
+*/
+
+static struct option options[] = {
+    { "mode",       required_argument, NULL, 'm' },
+    { "verbose",          no_argument, NULL, 'v' },
+    { "state-path", required_argument, NULL, 's' },
+    { }
+};
+
+int main(int argc, char **argv)
+{
+    int is_server = 0;
+    int socket_fd;
+    int input_fd, output_fd;
+    struct libxenvchan *ctrl = NULL;
+    const char *socket_path;
+    int domid;
+    const char *vchan_path;
+    const char *state_path = NULL;
+    int opt;
+
+    while ((opt = getopt_long(argc, argv, "m:vs:", options, NULL)) != -1) {
+        switch (opt) {
+            case 'm':
+                if (strcmp(optarg, "server") == 0)
+                    is_server = 1;
+                else if (strcmp(optarg, "client") == 0)
+                    is_server = 0;
+                else {
+                    fprintf(stderr, "invalid argument for --mode: %s\n", optarg);
+                    usage(argv);
+                    return 1;
+                }
+                break;
+            case 'v':
+                verbose = 1;
+                break;
+            case 's':
+                state_path = optarg;
+                break;
+            case '?':
+                usage(argv);
+        }
+    }
+
+    if (argc-optind != 3)
+        usage(argv);
+
+    domid = atoi(argv[optind]);
+    vchan_path = argv[optind+1];
+    socket_path = argv[optind+2];
+
+    if (is_server) {
+        ctrl = libxenvchan_server_init(NULL, domid, vchan_path, 0, 0);
+        if (!ctrl) {
+            perror("libxenvchan_server_init");
+            exit(1);
+        }
+    } else {
+        if (strcmp(socket_path, "-") == 0) {
+            input_fd = 0;
+            output_fd = 1;
+        } else {
+            socket_fd = listen_socket(socket_path);
+            if (socket_fd == -1) {
+                perror("listen socket");
+                return 1;
+            }
+        }
+    }
+
+    if (state_path) {
+        struct xs_handle *xs;
+
+        xs = xs_open(0);
+        if (!xs) {
+            perror("xs_open");
+            return 1;
+        }
+        if (!xs_write(xs, XBT_NULL, state_path, "running", strlen("running"))) {
+            perror("xs_write");
+            return 1;
+        }
+        xs_close(xs);
+    }
+
+    for (;;) {
+        if (is_server) {
+            /* wait for vchan connection */
+            while (libxenvchan_is_open(ctrl) != 1)
+                libxenvchan_wait(ctrl);
+            /* vchan client connected, setup local FD if needed */
+            if (strcmp(socket_path, "-") == 0) {
+                input_fd = 0;
+                output_fd = 1;
+            } else {
+                input_fd = output_fd = connect_socket(socket_path);
+            }
+            if (input_fd == -1) {
+                perror("connect socket");
+                return 1;
+            }
+            if (data_loop(ctrl, input_fd, output_fd) != 0)
+                break;
+            /* keep it running only when get UNIX socket path */
+            if (socket_path[0] != '/')
+                break;
+        } else {
+            /* wait for local socket connection */
+            if (strcmp(socket_path, "-") != 0)
+                input_fd = output_fd = accept(socket_fd, NULL, NULL);
+            if (input_fd == -1) {
+                perror("accept");
+                return 1;
+            }
+            set_nonblocking(input_fd, 1);
+            set_nonblocking(output_fd, 1);
+            ctrl = connect_vchan(domid, vchan_path);
+            if (!ctrl) {
+                perror("vchan client init");
+                return 1;
+            }
+            if (data_loop(ctrl, input_fd, output_fd) != 0)
+                break;
+            /* don't reconnect if output was stdout */
+            if (strcmp(socket_path, "-") == 0)
+                break;
+
+            libxenvchan_close(ctrl);
+            ctrl = NULL;
+        }
+    }
+    return 0;
+}
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (10 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-21 20:17   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 13/16] Regenerate autotools files Marek Marczykowski-Górecki
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

Access to QMP of QEMU in Linux stubdomain is possible over vchan
connection. Handle the actual vchan connection in a separate process
(vchan-socket-proxy). This simplified integration with QMP (already
quite complex), but also allows preliminary filtering of (potentially
malicious) QMP input.
Since only one client can be connected to vchan server at the same time
and it is not enforced by the libxenvchan itself, additional client-side
locking is needed. It is implicitly implemented by vchan-socket-proxy,
as it handle only one connection at a time. Note that qemu supports only
one simultaneous client on a control socket anyway (but in UNIX socket
case, it enforce it server-side), so it doesn't add any extra
limitation.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes in v4:
 - new patch, in place of both "libxl: use vchan for QMP access ..."
---
 tools/configure.ac           |   9 ++-
 tools/libxl/libxl_dm.c       | 159 ++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_internal.h |   1 +-
 3 files changed, 161 insertions(+), 8 deletions(-)

diff --git a/tools/configure.ac b/tools/configure.ac
index 8d86c42..20bbdbf 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -192,6 +192,15 @@ AC_SUBST(qemu_xen)
 AC_SUBST(qemu_xen_path)
 AC_SUBST(qemu_xen_systemd)
 
+AC_ARG_WITH([stubdom-qmp-proxy],
+    AC_HELP_STRING([--stubdom-qmp-proxy@<:@=PATH@:>@],
+        [Use supplied binary PATH as a QMP proxy into stubdomain]),[
+    stubdom_qmp_proxy="$withval"
+],[
+    stubdom_qmp_proxy="$bindir/vchan-socket-proxy"
+])
+AC_DEFINE_UNQUOTED([STUBDOM_QMP_PROXY_PATH], ["$stubdom_qmp_proxy"], [QMP proxy path])
+
 AC_ARG_WITH([system-seabios],
     AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@],
        [Use system supplied seabios PATH instead of building and installing
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 528ca3e..23ac7e4 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1183,7 +1183,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, stubdomain setup it itself */
     if (!is_stubdom) {
         flexarray_append(dm_args, "-chardev");
         if (state->dm_monitor_fd >= 0) {
@@ -2178,6 +2178,23 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc,
 static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
                               int rc, const char *p);
 
+static void spawn_qmp_proxy(libxl__egc *egc,
+                            libxl__stub_dm_spawn_state *sdss);
+
+static void qmp_proxy_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+                              const char *xsdata);
+
+static void qmp_proxy_startup_failed(libxl__egc *egc,
+                                     libxl__spawn_state *spawn,
+                                     int rc);
+
+static void qmp_proxy_detached(libxl__egc *egc,
+                               libxl__spawn_state *spawn);
+
+static void qmp_proxy_spawn_outcome(libxl__egc *egc,
+                                    libxl__stub_dm_spawn_state *sdss,
+                                    int rc);
+
 char *libxl__stub_dm_name(libxl__gc *gc, const char *guest_name)
 {
     return GCSPRINTF("%s-dm", guest_name);
@@ -2460,24 +2477,150 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
             goto out;
     }
 
+    sdss->qmp_proxy_spawn.ao = ao;
+    if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
+        spawn_qmp_proxy(egc, sdss);
+    } else {
+        qmp_proxy_spawn_outcome(egc, sdss, 0);
+    }
+
+    return;
+
+out:
+    assert(ret);
+    qmp_proxy_spawn_outcome(egc, sdss, ret);
+}
+
+static void spawn_qmp_proxy(libxl__egc *egc,
+                            libxl__stub_dm_spawn_state *sdss)
+{
+    STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
+    const uint32_t guest_domid = sdss->dm.guest_domid;
+    const uint32_t dm_domid = sdss->pvqemu.guest_domid;
+    const char *dom_path = libxl__xs_get_dompath(gc, dm_domid);
+    char **args;
+    int nr = 0;
+    int rc, logfile_w, null;
+
+    if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) {
+        LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid);
+    sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path);
+    sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path);
+
+    sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+    sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid;
+    sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm;
+    sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed;
+    sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached;
+
+    const int arraysize = 6;
+    GCNEW_ARRAY(args, arraysize);
+    args[nr++] = STUBDOM_QMP_PROXY_PATH;
+    args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath);
+    args[nr++] = GCSPRINTF("%u", dm_domid);
+    args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid);
+    args[nr++] = (char*)libxl__qemu_qmp_path(gc, guest_domid);
+    args[nr++] = NULL;
+    assert(nr == arraysize);
+
+    logfile_w = libxl__create_qemu_logfile(gc, GCSPRINTF("qmp-proxy-%s",
+                                                         sdss->dm_config.c_info.name));
+    if (logfile_w < 0) {
+        rc = logfile_w;
+        goto out;
+    }
+    null = open("/dev/null", O_RDWR);
+    if (null < 0) {
+        LOGED(ERROR, guest_domid, "unable to open /dev/null");
+        rc = ERROR_FAIL;
+        goto out_close;
+    }
+
+    rc = libxl__spawn_spawn(egc, &sdss->qmp_proxy_spawn);
+    if (rc < 0)
+        goto out_close;
+    if (!rc) { /* inner child */
+        setsid();
+        libxl__exec(gc, null, null, logfile_w, STUBDOM_QMP_PROXY_PATH, args, NULL);
+        /* unreachable */
+    }
+
+    rc = 0;
+
+out_close:
+    if (logfile_w >= 0)
+        close(logfile_w);
+    if (null >= 0)
+        close(null);
+out:
+    if (rc)
+        qmp_proxy_spawn_outcome(egc, sdss, rc);
+}
+
+static void qmp_proxy_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+                              const char *xsdata)
+{
+    STATE_AO_GC(spawn->ao);
+
+    if (!xsdata)
+        return;
+
+    if (strcmp(xsdata, "running"))
+        return;
+
+    libxl__spawn_initiate_detach(gc, spawn);
+}
+
+static void qmp_proxy_startup_failed(libxl__egc *egc,
+                                     libxl__spawn_state *spawn,
+                                     int rc)
+{
+    libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(spawn, *sdss, qmp_proxy_spawn);
+    qmp_proxy_spawn_outcome(egc, sdss, rc);
+}
+
+static void qmp_proxy_detached(libxl__egc *egc,
+                               libxl__spawn_state *spawn)
+{
+    libxl__stub_dm_spawn_state *sdss = CONTAINER_OF(spawn, *sdss, qmp_proxy_spawn);
+    qmp_proxy_spawn_outcome(egc, sdss, 0);
+}
+
+static void qmp_proxy_spawn_outcome(libxl__egc *egc,
+                                    libxl__stub_dm_spawn_state *sdss,
+                                    int rc)
+{
+    STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
+    int need_pvqemu = libxl__need_xenpv_qemu(gc, &sdss->dm_config);
+
+    if (rc) goto out;
+
+    if (need_pvqemu < 0) {
+        rc = need_pvqemu;
+        goto out;
+    }
+
     sdss->pvqemu.spawn.ao = ao;
-    sdss->pvqemu.guest_domid = dm_domid;
     sdss->pvqemu.guest_config = &sdss->dm_config;
     sdss->pvqemu.build_state = &sdss->dm_state;
     sdss->pvqemu.callback = spawn_stubdom_pvqemu_cb;
-
-    if (!need_qemu) {
+    if (need_pvqemu) {
+        libxl__spawn_local_dm(egc, &sdss->pvqemu);
+    } else {
         /* If dom0 qemu not needed, do not launch it */
         spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, 0);
-    } else {
-        libxl__spawn_local_dm(egc, &sdss->pvqemu);
     }
 
     return;
 
 out:
-    assert(ret);
-    spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, ret);
+    assert(rc);
+    spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, rc);
 }
 
 static void spawn_stubdom_pvqemu_cb(libxl__egc *egc,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2b4a1cc..895bb65 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4129,6 +4129,7 @@ typedef struct {
     libxl__destroy_domid_state dis;
     libxl__multidev multidev;
     libxl__xswait_state xswait;
+    libxl__spawn_state qmp_proxy_spawn;
 } libxl__stub_dm_spawn_state;
 
 _hidden void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state*);
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 13/16] Regenerate autotools files
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (11 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-15 21:57   ` Rich Persaud
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 14/16] libxl: require qemu in dom0 even if stubdomain is in use Marek Marczykowski-Górecki
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Samuel Thibault, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

Since we have those generated files committed to the repo (why?!),
update them after changing configure.ac.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 configure         | 14 +-------------
 docs/configure    | 14 +-------------
 stubdom/configure | 14 +-------------
 tools/config.h.in |  3 +++
 tools/configure   | 46 ++++++++++++++++++++++++++++------------------
 5 files changed, 34 insertions(+), 57 deletions(-)

diff --git a/configure b/configure
index 83f84b0..e5d14b7 100755
--- a/configure
+++ b/configure
@@ -644,7 +644,6 @@ infodir
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -723,7 +722,6 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -976,15 +974,6 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1122,7 +1111,7 @@ fi
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir runstatedir
+		libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1275,7 +1264,6 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
diff --git a/docs/configure b/docs/configure
index fdfc110..598813d 100755
--- a/docs/configure
+++ b/docs/configure
@@ -634,7 +634,6 @@ infodir
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -711,7 +710,6 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -964,15 +962,6 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1110,7 +1099,7 @@ fi
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir runstatedir
+		libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1263,7 +1252,6 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
diff --git a/stubdom/configure b/stubdom/configure
index 8f0bdcf..e2443e9 100755
--- a/stubdom/configure
+++ b/stubdom/configure
@@ -661,7 +661,6 @@ infodir
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -751,7 +750,6 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1004,15 +1002,6 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1150,7 +1139,7 @@ fi
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir runstatedir
+		libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1303,7 +1292,6 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
diff --git a/tools/config.h.in b/tools/config.h.in
index 5a5944e..5abf609 100644
--- a/tools/config.h.in
+++ b/tools/config.h.in
@@ -123,6 +123,9 @@
 /* Define to 1 if you have the ANSI C header files. */
 #undef STDC_HEADERS
 
+/* QMP proxy path */
+#undef STUBDOM_QMP_PROXY_PATH
+
 /* Enable large inode numbers on Mac OS X 10.5.  */
 #ifndef _DARWIN_USE_64_BIT_INODE
 # define _DARWIN_USE_64_BIT_INODE 1
diff --git a/tools/configure b/tools/configure
index 977a883..ef4b5d8 100755
--- a/tools/configure
+++ b/tools/configure
@@ -770,7 +770,6 @@ infodir
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -811,6 +810,7 @@ with_linux_backend_modules
 enable_qemu_traditional
 enable_rombios
 with_system_qemu
+with_stubdom_qmp_proxy
 with_system_seabios
 with_system_ovmf
 enable_ipxe
@@ -896,7 +896,6 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1149,15 +1148,6 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1295,7 +1285,7 @@ fi
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir runstatedir
+		libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1448,7 +1438,6 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
@@ -1531,6 +1520,9 @@ Optional Packages:
                           Use system supplied qemu PATH or qemu (taken from
                           $PATH) as qemu-xen device model instead of building
                           and installing our own version
+  --stubdom-qmp-proxy[=PATH]
+                          Use supplied binary PATH as a QMP proxy into
+                          stubdomain
   --with-system-seabios[=PATH]
                           Use system supplied seabios PATH instead of building
                           and installing our own version
@@ -3378,7 +3370,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3424,7 +3416,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3448,7 +3440,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3493,7 +3485,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -3517,7 +3509,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -4519,6 +4511,24 @@ _ACEOF
 
 
 
+# Check whether --with-stubdom-qmp-proxy was given.
+if test "${with_stubdom_qmp_proxy+set}" = set; then :
+  withval=$with_stubdom_qmp_proxy;
+    stubdom_qmp_proxy="$withval"
+
+else
+
+    stubdom_qmp_proxy="$bindir/vchan-socket-proxy"
+
+fi
+
+
+cat >>confdefs.h <<_ACEOF
+#define STUBDOM_QMP_PROXY_PATH "$stubdom_qmp_proxy"
+_ACEOF
+
+
+
 # Check whether --with-system-seabios was given.
 if test "${with_system_seabios+set}" = set; then :
   withval=$with_system_seabios;
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 14/16] libxl: require qemu in dom0 even if stubdomain is in use
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (12 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 13/16] Regenerate autotools files Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 15/16] libxl: ignore emulated IDE disks beyond the first 4 Marek Marczykowski-Górecki
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

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 | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 23ac7e4..43af31b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2464,7 +2464,11 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
         }
     }
 
-    need_qemu = libxl__need_xenpv_qemu(gc, dm_config);
+    /*
+     * Until xenconsoled learns how to handle multiple consoles, require qemu
+     * in dom0 to serve consoles for a stubdomain - it require at least 3 of them.
+     */
+    need_qemu = 1 || libxl__need_xenpv_qemu(gc, &sdss->dm_config);
 
     for (i = 0; i < num_console; i++) {
         libxl__device device;
@@ -2596,7 +2600,11 @@ static void qmp_proxy_spawn_outcome(libxl__egc *egc,
                                     int rc)
 {
     STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
-    int need_pvqemu = libxl__need_xenpv_qemu(gc, &sdss->dm_config);
+    /*
+     * Until xenconsoled learns how to handle multiple consoles, require qemu
+     * in dom0 to serve consoles for a stubdomain - it require at least 3 of them.
+     */
+    int need_pvqemu = 1 || libxl__need_xenpv_qemu(gc, &sdss->dm_config);
 
     if (rc) goto out;
 
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 15/16] libxl: ignore emulated IDE disks beyond the first 4
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (13 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 14/16] libxl: require qemu in dom0 even if stubdomain is in use Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-21 20:24   ` Jason Andryuk
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 16/16] libxl: consider also qemu in stubdomain in libxl__dm_active check Marek Marczykowski-Górecki
  2020-01-22 16:50 ` [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

Qemu supports only 4 emulated IDE disks, when given more (or with higher
indexes), it will fail to start. Since the disks can still be accessible
using PV interface, just ignore emulated path and log a warning, instead
of rejecting the configuration altogether.

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

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 43af31b..89eca1e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1871,6 +1871,13 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
             }
 
             if (disks[i].is_cdrom) {
+                if (disk > 4) {
+                    LOGD(WARN, guest_domid, "Emulated CDROM can be only one of the first 4 disks.\n"
+                         "Disk %s will be available via PV drivers but not as an "
+                         "emulated disk.",
+                         disks[i].vdev);
+                    continue;
+                }
                 drive = libxl__sprintf(gc,
                          "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
                          disk, dev_number);
@@ -1948,6 +1955,10 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                                                        &disks[i],
                                                        colo_mode);
                 } else {
+                    LOGD(WARN, guest_domid, "Only 4 emulated IDE disks are supported.\n"
+                         "Disk %s will be available via PV drivers but not as an "
+                         "emulated disk.",
+                         disks[i].vdev);
                     continue; /* Do not emulate this disk */
                 }
 
-- 
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] 53+ messages in thread

* [Xen-devel] [PATCH v4 16/16] libxl: consider also qemu in stubdomain in libxl__dm_active check
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (14 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 15/16] libxl: ignore emulated IDE disks beyond the first 4 Marek Marczykowski-Górecki
@ 2020-01-15  2:39 ` Marek Marczykowski-Górecki
  2020-01-21 20:25   ` Jason Andryuk
  2020-01-22 16:50 ` [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk
  16 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-15  2:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Ian Jackson, Marek Marczykowski-Górecki, Wei Liu

Since qemu-xen can now run in stubdomain too, handle this case when
checking it's state too.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libxl/libxl_dm.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 89eca1e..7698887 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -3729,12 +3729,18 @@ out:
 
 int libxl__dm_active(libxl__gc *gc, uint32_t domid)
 {
-    char *pid, *path;
+    char *pid, *dm_domid, *path;
 
     path = GCSPRINTF("/local/domain/%d/image/device-model-pid", domid);
     pid = libxl__xs_read(gc, XBT_NULL, path);
 
-    return pid != NULL;
+    if (pid)
+        return true;
+
+    path = GCSPRINTF("/local/domain/%d/image/device-model-domid", domid);
+    dm_domid = libxl__xs_read(gc, XBT_NULL, path);
+
+    return dm_domid != NULL;
 }
 
 int libxl__dm_check_start(libxl__gc *gc, libxl_domain_config *d_config,
-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy Marek Marczykowski-Górecki
@ 2020-01-15 11:02   ` Jan Beulich
  2020-01-16 17:11     ` Marek Marczykowski-Górecki
  2020-01-17 18:44   ` Rich Persaud
  2020-01-21 19:43   ` Jason Andryuk
  2 siblings, 1 reply; 53+ messages in thread
From: Jan Beulich @ 2020-01-15 11:02 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel

On 15.01.2020 03:39, Marek Marczykowski-Górecki wrote:
> Add a simple proxy for tunneling socket connection over vchan. This is
> based on existing vchan-node* applications, but extended with socket
> support. vchan-socket-proxy serves both as a client and as a server,
> depending on parameters. It can be used to transparently communicate
> with an application in another domian that normally expose UNIX socket
> interface. Specifically, it's written to communicate with qemu running
> within stubdom.
> 
> Server mode listens for vchan connections and when one is opened,
> connects to a pointed UNIX socket.  Client mode listens on UNIX
> socket and when someone connects, opens a vchan connection.  Only
> a single connection at a time is supported.
> 
> Additionally, socket can be provided as a number - in which case it's
> interpreted as already open FD (in case of UNIX listening socket -
> listen() needs to be already called). Or "-" meaning stdin/stdout - in
> which case it is reduced to vchan-node2 functionality.
> 
> Example usage:
> 
> 1. (in dom0) vchan-socket-proxy --mode=client <DOMID>
>     /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
> 
> 2. (in DOMID) vchan-socket-proxy --mode=server 0
>    /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
> 
> This will listen on /run/qemu.(DOMID) in dom0 and whenever connection is
> made, it will connect to DOMID, where server process will connect to
> /run/qemu.(DOMID) there. When client disconnects, vchan connection is
> terminated and server vchan-socket-proxy process also disconnects from
> qemu.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  .gitignore                          |   1 +-

I guess this is why various non-tool-stack maintainers have
been Cc-ed. It would have been nice if you had stripped the
unnecessary Cc-s. I don't think ./MAINTAINERS can properly
express a suitable rule of Cc REST if the adjustment is not
simply accompanying the addition of some new output file.

>  tools/libvchan/Makefile             |   7 +-
>  tools/libvchan/init.c.rej           |  60 ++++-

Now since I've been Cc-ed, I'd like to ask what this is about.

Jan

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

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

* Re: [Xen-devel] [PATCH v4 13/16] Regenerate autotools files
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 13/16] Regenerate autotools files Marek Marczykowski-Górecki
@ 2020-01-15 21:57   ` Rich Persaud
  2020-01-21 20:56     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Rich Persaud @ 2020-01-15 21:57 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Ian Jackson, Wei Liu, Samuel Thibault

> On Jan 14, 2020, at 21:42, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
> Since we have those generated files committed to the repo (why?!),
> update them after changing configure.ac.

Is there any reason not to remove the generated configure files?  A developer using generated files on system B would be incorporating configuration assumptions from system A where the configure script was generated.  If we are going to ship configure scripts, do we need to document a "system A" reference distro/environment where all configure scripts from Xen will be generated?


Other notes:

1.  Debian autoreconf works in the Xen root directory, but the default OpenEmbedded autoreconf uses Gnu libtoolize and fails because some Xen build subdirectories don't have configure.ac/.in.   

2.  If OpenEmbedded autoreconf is run only in the tools directory (where it works and generates a new tools configure), then root configure (generated from older configure.ac) will silently ignore the newer tools configure and write config.h _without_ tools-specific config, such as the vchan QMP proxy.

3. If autoreconf runs successfully in the root directory, then tools-specific configure is correctly generated and everything works as expected.

This silent failure could be avoided by deleting the generated configure scripts.  There may be other failure modes for using System A generated scripts on downstream build system B.

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

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

* Re: [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-15 11:02   ` Jan Beulich
@ 2020-01-16 17:11     ` Marek Marczykowski-Górecki
  2020-01-17  8:13       ` Jan Beulich
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-16 17:11 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel

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

On Wed, Jan 15, 2020 at 12:02:42PM +0100, Jan Beulich wrote:
> On 15.01.2020 03:39, Marek Marczykowski-Górecki wrote:
> >  .gitignore                          |   1 +-
> 
> I guess this is why various non-tool-stack maintainers have
> been Cc-ed. It would have been nice if you had stripped the
> unnecessary Cc-s. I don't think ./MAINTAINERS can properly
> express a suitable rule of Cc REST if the adjustment is not
> simply accompanying the addition of some new output file.

Maybe a solution would be to make use of more .gitignore files in
specific subdirs? I see there are some, but for example tools/misc is
mentioned in _both_ toplevel .gitignore and tools/misc/.gitignore.

> >  tools/libvchan/Makefile             |   7 +-
> >  tools/libvchan/init.c.rej           |  60 ++++-
> 
> Now since I've been Cc-ed, I'd like to ask what this is about.

This is obviously a mistake. Will remove in the next revision.

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-16 17:11     ` Marek Marczykowski-Górecki
@ 2020-01-17  8:13       ` Jan Beulich
  0 siblings, 0 replies; 53+ messages in thread
From: Jan Beulich @ 2020-01-17  8:13 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel

On 16.01.2020 18:11, Marek Marczykowski-Górecki  wrote:
> On Wed, Jan 15, 2020 at 12:02:42PM +0100, Jan Beulich wrote:
>> On 15.01.2020 03:39, Marek Marczykowski-Górecki wrote:
>>>  .gitignore                          |   1 +-
>>
>> I guess this is why various non-tool-stack maintainers have
>> been Cc-ed. It would have been nice if you had stripped the
>> unnecessary Cc-s. I don't think ./MAINTAINERS can properly
>> express a suitable rule of Cc REST if the adjustment is not
>> simply accompanying the addition of some new output file.
> 
> Maybe a solution would be to make use of more .gitignore files in
> specific subdirs? I see there are some, but for example tools/misc is
> mentioned in _both_ toplevel .gitignore and tools/misc/.gitignore.

Well, yes, we've been discussing this elsewhere. We should settle
on one model (central vs per-main-subtree vs per-dir), I think. I
have no particular preference except for disliking any mixed models.

Jan

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

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

* Re: [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy Marek Marczykowski-Górecki
  2020-01-15 11:02   ` Jan Beulich
@ 2020-01-17 18:44   ` Rich Persaud
  2020-01-17 18:56     ` Marek Marczykowski-Górecki
  2020-01-21 19:43   ` Jason Andryuk
  2 siblings, 1 reply; 53+ messages in thread
From: Rich Persaud @ 2020-01-17 18:44 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
	xen-devel

On Jan 14, 2020, at 21:42, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
> 
> diff --git a/tools/libvchan/Makefile b/tools/libvchan/Makefile
> index 7892750..1c845ca 100644
> --- a/tools/libvchan/Makefile
> +++ b/tools/libvchan/Makefile
> @@ -13,6 +13,7 @@ LIBVCHAN_PIC_OBJS = $(patsubst %.o,%.opic,$(LIBVCHAN_OBJS))
> LIBVCHAN_LIBS = $(LDLIBS_libxenstore) $(LDLIBS_libxengnttab) $(LDLIBS_libxenevtchn)
> $(LIBVCHAN_OBJS) $(LIBVCHAN_PIC_OBJS): CFLAGS += $(CFLAGS_libxenstore) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
> $(NODE_OBJS) $(NODE2_OBJS): CFLAGS += $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
> +vchan-socket-proxy.o: CFLAGS += $(CFLAGS_libxenstore) $(CFLAGS_libxenctrl) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn)
> 
> MAJOR = 4.14
> MINOR = 0
> @@ -39,7 +40,7 @@ $(PKG_CONFIG_LOCAL): PKG_CONFIG_LIBDIR = $(CURDIR)
> $(PKG_CONFIG_LOCAL): PKG_CONFIG_CFLAGS_LOCAL = $(CFLAGS_xeninclude)
> 
> .PHONY: all
> -all: libxenvchan.so vchan-node1 vchan-node2 libxenvchan.a $(PKG_CONFIG_INST) $(PKG_CONFIG_LOCAL)
> +all: libxenvchan.so vchan-node1 vchan-node2 vchan-socket-proxy libxenvchan.a $(PKG_CONFIG_INST) $(PKG_CONFIG_LOCAL)
> 
> libxenvchan.so: libxenvchan.so.$(MAJOR)
>    ln -sf $< $@
> @@ -59,6 +60,9 @@ vchan-node1: $(NODE_OBJS) libxenvchan.so
> vchan-node2: $(NODE2_OBJS) libxenvchan.so
>    $(CC) $(LDFLAGS) -o $@ $(NODE2_OBJS) $(LDLIBS_libxenvchan) $(APPEND_LDFLAGS)
> 
> +vchan-socket-proxy: vchan-socket-proxy.o libxenvchan.so
> +    $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
> +
> .PHONY: install
> install: all
>    $(INSTALL_DIR) $(DESTDIR)$(libdir)
> @@ -66,6 +70,7 @@ install: all
>    $(INSTALL_PROG) libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)
>    ln -sf libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)/libxenvchan.so.$(MAJOR)
>    ln -sf libxenvchan.so.$(MAJOR) $(DESTDIR)$(libdir)/libxenvchan.so
> +    $(INSTALL_PROG) vchan-socket-proxy $(DESTDIR)$(bindir)

Does this need directory creation, to avoid vchan binary being named "bin"?
+       $(INSTALL_DIR) $(DESTDIR)$(bindir)


> diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
> new file mode 100644
> index 0000000..6b4ae09
> --- /dev/null
> +++ b/tools/libvchan/vchan-socket-proxy.c
> @@ -0,0 +1,469 @@
> +/**
> + * @file
> + * @section AUTHORS
> + *
> + * Copyright (C) 2010  Rafal Wojtczuk  <rafal@invisiblethingslab.com>
> + *
> + *  Authors:
> + *       Rafal Wojtczuk  <rafal@invisiblethingslab.com>
> + *       Daniel De Graaf <dgdegra@tycho.nsa.gov>
> + *       Marek Marczykowski-Górecki  <marmarek@invisiblethingslab.com>
> + *
> + * @section LICENSE
> + *
> + *  This program is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this program; If not, see <http://www.gnu.org/licenses/>.
> + *
> + * @section DESCRIPTION
> + *
> + * This is a vchan to unix socket proxy. Vchan server is set, and on client
> + * connection, local socket connection is established. Communication is bidirectional.
> + * One client is served at a time, clients needs to coordinate this themselves.
> + */
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <getopt.h>
> +
> +#include <xenstore.h>
> +#include <xenctrl.h>
> +#include <libxenvchan.h>
> +
> +static void usage(char** argv)
> +{
> +    fprintf(stderr, "usage:\n"
> +        "\t%s [options] domainid nodepath [socket-path|file-no|-]\n"
> +        "\n"
> +        "options:\n"
> +        "\t-m, --mode=client|server - vchan connection mode\n"
> +        "\t-m, --state-path=path - xenstore path where write \"running\" to at startup\n"
> +        "\t-v, --verbose - verbose logging\n"
> +        "\n"
> +        "client: client of a vchan connection, fourth parameter can be:\n"
> +        "\tsocket-path: listen on a UNIX socket at this path and connect to vchan\n"
> +        "\t  whenever new connection is accepted;\n"
> +        "\t  handle multiple _subsequent_ connections, until terminated\n"
> +        "\tfile-no: except open FD of a socket in listen mode; otherwise similar to socket-path\n"
> +        "\t-: open vchan connection immediately and pass the data from stdin/stdout;\n"
> +        "\t  terminate when vchan connection is closed\n"
> +        "server: server of a vchan connection, fourth parameter can be:\n"
> +        "\tsocket-path: connect to this UNIX socket when new vchan connection is accepted\n"
> +        "\t  handle multiple _subsequent_ connections, until terminated\n"
> +        "\tfile-no: pass data to/from this FD; terminate when vchan connection is closed\n"
> +        "\t-: pass data to/from stdin/stdout; terminatate when vchan connection is closed\n",
> +        argv[0]);
> +    exit(1);
> +}
> +
> +#define BUFSIZE 8192
> +char inbuf[BUFSIZE];
> +char outbuf[BUFSIZE];
> +int insiz = 0;
> +int outsiz = 0;
> +int verbose = 0;
> +
> +static void vchan_wr(struct libxenvchan *ctrl) {
> +    int ret;
> +
> +    if (!insiz)
> +        return;
> +    ret = libxenvchan_write(ctrl, inbuf, insiz);
> +    if (ret < 0) {
> +        fprintf(stderr, "vchan write failed\n");
> +        exit(1);
> +    }
> +    if (verbose)
> +        fprintf(stderr, "written %d bytes to vchan\n", ret);
> +    if (ret > 0) {
> +        insiz -= ret;
> +        memmove(inbuf, inbuf + ret, insiz);
> +    }
> +}
> +
> +static void socket_wr(int output_fd) {
> +    int ret;
> +
> +    if (!outsiz)
> +        return;
> +    ret = write(output_fd, outbuf, outsiz);
> +    if (ret < 0 && errno != EAGAIN)
> +        exit(1);
> +    if (ret > 0) {
> +        outsiz -= ret;
> +        memmove(outbuf, outbuf + ret, outsiz);
> +    }
> +}
> +
> +static int set_nonblocking(int fd, int nonblocking) {
> +    int flags = fcntl(fd, F_GETFL);
> +    if (flags == -1)
> +        return -1;
> +
> +    if (nonblocking)
> +        flags |= O_NONBLOCK;
> +    else
> +        flags &= ~O_NONBLOCK;
> +
> +    if (fcntl(fd, F_SETFL, flags) == -1)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +static int connect_socket(const char *path_or_fd) {
> +    int fd;
> +    char *endptr;
> +    struct sockaddr_un addr;
> +
> +    fd = strtoll(path_or_fd, &endptr, 0);
> +    if (*endptr == '\0') {
> +        set_nonblocking(fd, 1);
> +        return fd;
> +    }
> +
> +    fd = socket(AF_UNIX, SOCK_STREAM, 0);
> +    if (fd == -1)
> +        return -1;
> +
> +    addr.sun_family = AF_UNIX;
> +    strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
> +    if (connect(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
> +        close(fd);
> +        return -1;
> +    }
> +
> +    set_nonblocking(fd, 1);
> +
> +    return fd;
> +}
> +
> +static int listen_socket(const char *path_or_fd) {
> +    int fd;
> +    char *endptr;
> +    struct sockaddr_un addr;
> +
> +    fd = strtoll(path_or_fd, &endptr, 0);
> +    if (*endptr == '\0') {
> +        return fd;
> +    }
> +
> +    /* if not a number, assume a socket path */
> +    fd = socket(AF_UNIX, SOCK_STREAM, 0);
> +    if (fd == -1)
> +        return -1;
> +
> +    addr.sun_family = AF_UNIX;
> +    strncpy(addr.sun_path, path_or_fd, sizeof(addr.sun_path));
> +    if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
> +        close(fd);
> +        return -1;
> +    }
> +    if (listen(fd, 5) != 0) {
> +        close(fd);
> +        return -1;
> +    }
> +
> +    return fd;
> +}
> +
> +static struct libxenvchan *connect_vchan(int domid, const char *path) {
> +    struct libxenvchan *ctrl = NULL;
> +    struct xs_handle *xs = NULL;
> +    xc_interface *xc = NULL;
> +    xc_dominfo_t dominfo;
> +    char **watch_ret;
> +    unsigned int watch_num;
> +    int ret;
> +
> +    xs = xs_open(XS_OPEN_READONLY);
> +    if (!xs) {
> +        perror("xs_open");
> +        goto out;
> +    }
> +    xc = xc_interface_open(NULL, NULL, XC_OPENFLAG_NON_REENTRANT);
> +    if (!xc) {
> +        perror("xc_interface_open");
> +        goto out;
> +    }
> +    /* wait for vchan server to create *path* */
> +    xs_watch(xs, path, "path");
> +    xs_watch(xs, "@releaseDomain", "release");
> +    while ((watch_ret = xs_read_watch(xs, &watch_num))) {
> +        /* don't care about exact which fired the watch */
> +        free(watch_ret);
> +        ctrl = libxenvchan_client_init(NULL, domid, path);
> +        if (ctrl)
> +            break;
> +
> +        ret = xc_domain_getinfo(xc, domid, 1, &dominfo);
> +        /* break the loop if domain is definitely not there anymore, but
> +         * continue if it is or the call failed (like EPERM) */
> +        if (ret == -1 && errno == ESRCH)
> +            break;
> +        if (ret == 1 && (dominfo.domid != (uint32_t)domid || dominfo.dying))
> +            break;
> +    }
> +
> +out:
> +    if (xc)
> +        xc_interface_close(xc);
> +    if (xs)
> +        xs_close(xs);
> +    return ctrl;
> +}
> +
> +
> +static void discard_buffers(struct libxenvchan *ctrl) {
> +    /* discard local buffers */
> +    insiz = 0;
> +    outsiz = 0;
> +
> +    /* discard remaining incoming data */
> +    while (libxenvchan_data_ready(ctrl)) {
> +        if (libxenvchan_read(ctrl, inbuf, BUFSIZE) == -1) {
> +            perror("vchan read");
> +            exit(1);
> +        }
> +    }
> +}
> +
> +int data_loop(struct libxenvchan *ctrl, int input_fd, int output_fd)
> +{
> +    int ret;
> +    int libxenvchan_fd;
> +    int max_fd;
> +
> +    libxenvchan_fd = libxenvchan_fd_for_select(ctrl);
> +    for (;;) {
> +        fd_set rfds;
> +        fd_set wfds;
> +        FD_ZERO(&rfds);
> +        FD_ZERO(&wfds);
> +
> +        max_fd = -1;
> +        if (input_fd != -1 && insiz != BUFSIZE) {
> +            FD_SET(input_fd, &rfds);
> +            if (input_fd > max_fd)
> +                max_fd = input_fd;
> +        }
> +        if (output_fd != -1 && outsiz) {
> +            FD_SET(output_fd, &wfds);
> +            if (output_fd > max_fd)
> +                max_fd = output_fd;
> +        }
> +        FD_SET(libxenvchan_fd, &rfds);
> +        if (libxenvchan_fd > max_fd)
> +            max_fd = libxenvchan_fd;
> +        ret = select(max_fd + 1, &rfds, &wfds, NULL, NULL);
> +        if (ret < 0) {
> +            perror("select");
> +            exit(1);
> +        }
> +        if (FD_ISSET(libxenvchan_fd, &rfds)) {
> +            libxenvchan_wait(ctrl);
> +            if (!libxenvchan_is_open(ctrl)) {
> +                if (verbose)
> +                    fprintf(stderr, "vchan client disconnected\n");
> +                while (outsiz)
> +                    socket_wr(output_fd);
> +                close(output_fd);
> +                close(input_fd);
> +                discard_buffers(ctrl);
> +                break;
> +            }
> +            vchan_wr(ctrl);
> +        }
> +
> +        /* socket_fd guaranteed to be != -1 */
> +
> +        if (FD_ISSET(input_fd, &rfds)) {
> +            ret = read(input_fd, inbuf + insiz, BUFSIZE - insiz);
> +            if (ret < 0 && errno != EAGAIN)
> +                exit(1);
> +            if (verbose)
> +                fprintf(stderr, "from-unix: %.*s\n", ret, inbuf + insiz);
> +            if (ret == 0) {
> +                /* EOF on socket, write everything in the buffer and close the
> +                 * socket */
> +                while (insiz) {
> +                    vchan_wr(ctrl);
> +                    libxenvchan_wait(ctrl);
> +                }
> +                close(input_fd);
> +                input_fd = -1;
> +                /* TODO: maybe signal the vchan client somehow? */
> +                break;
> +            }
> +            if (ret)
> +                insiz += ret;
> +            vchan_wr(ctrl);
> +        }
> +        if (FD_ISSET(output_fd, &wfds))
> +            socket_wr(output_fd);
> +        while (libxenvchan_data_ready(ctrl) && outsiz < BUFSIZE) {
> +            ret = libxenvchan_read(ctrl, outbuf + outsiz, BUFSIZE - outsiz);
> +            if (ret < 0)
> +                exit(1);
> +            if (verbose)
> +                fprintf(stderr, "from-vchan: %.*s\n", ret, outbuf + outsiz);
> +            outsiz += ret;
> +            socket_wr(output_fd);
> +        }
> +    }
> +    return 0;
> +}
> +
> +/**
> +    Simple libxenvchan application, both client and server.
> +    Both sides may write and read, both from the libxenvchan and from
> +    stdin/stdout (just like netcat).
> +*/
> +
> +static struct option options[] = {
> +    { "mode",       required_argument, NULL, 'm' },
> +    { "verbose",          no_argument, NULL, 'v' },
> +    { "state-path", required_argument, NULL, 's' },
> +    { }
> +};
> +
> +int main(int argc, char **argv)
> +{
> +    int is_server = 0;
> +    int socket_fd;

When compiled for OpenEmbedded / OpenXT, gcc complained about socket_fd being uninitialized before possible use.

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

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

* Re: [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-17 18:44   ` Rich Persaud
@ 2020-01-17 18:56     ` Marek Marczykowski-Górecki
  0 siblings, 0 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-17 18:56 UTC (permalink / raw)
  To: Rich Persaud
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
	xen-devel

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

On Fri, Jan 17, 2020 at 01:44:20PM -0500, Rich Persaud wrote:
> On Jan 14, 2020, at 21:42, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
> > @@ -66,6 +70,7 @@ install: all
> >    $(INSTALL_PROG) libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)
> >    ln -sf libxenvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(libdir)/libxenvchan.so.$(MAJOR)
> >    ln -sf libxenvchan.so.$(MAJOR) $(DESTDIR)$(libdir)/libxenvchan.so
> > +    $(INSTALL_PROG) vchan-socket-proxy $(DESTDIR)$(bindir)
> 
> Does this need directory creation, to avoid vchan binary being named "bin"?
> +       $(INSTALL_DIR) $(DESTDIR)$(bindir)

I guess it depends on makefile execution order. I'll add it to be on the
safe side.

> > +int main(int argc, char **argv)
> > +{
> > +    int is_server = 0;
> > +    int socket_fd;
> 
> When compiled for OpenEmbedded / OpenXT, gcc complained about socket_fd being uninitialized before possible use.

I think gcc is wrong here - in all the paths socket_fd is used, it is
initialized under the same conditions (socket_path != "-" and
!is_server). But I'll add initializer to mute this warning.

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 01/16] Document ioemu MiniOS stubdomain protocol
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 01/16] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
@ 2020-01-20 18:30   ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 18:30 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
	xen-devel

On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> 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

<snip>

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

s/stubdoma/stubdomain/

Numbering is off - 2 is duplicated.

> +3. now stubdomain is considered running

I'm not familiar with mini-os stubdom to review the content you've written.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 02/16] Document ioemu Linux stubdomain protocol
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 02/16] Document ioemu Linux " Marek Marczykowski-Górecki
@ 2020-01-20 18:54   ` Jason Andryuk
  2020-01-21 21:08     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 18:54 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
	xen-devel

On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:

<snip>

> +
> +Limitations:
> + - PCI passthrough require permissive mode
> + - only one nic is supported

Why is only 1 nic supported?  Multiple were supported previously, but
peeking ahead in the series, script=/etc/qemu-ifup is no longer
specified.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 04/16] libxl: Allow running qemu-xen in stubdomain
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 04/16] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
@ 2020-01-20 18:56   ` Jason Andryuk
  2020-01-21 21:12     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 18:56 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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
> ---

<snip>

> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2299,6 +2299,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)

This is unused in the series, as far as I can tell.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options.
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
@ 2020-01-20 19:24   ` Jason Andryuk
  2020-01-21 21:18     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 19:24 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Wei Liu, Ian Jackson, Simon Gaiser, Anthony PERARD, xen-devel,
	Eric Shelton

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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

Since this is here...

> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
> [drop Qubes-specific parts]

...maybe mention dropping downscript here?  Otherwise the commit
message and contents don't match.

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

<snip>

> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 142b960..a6d40b7 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -169,6 +169,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",

Not to bikeshed, but this came up in a conversation a little while
ago.  Stubdom is a generic name, and this code is for a device model.
So some combination of qemu{,-dm}{,-linux}-kernel seems more
descriptive.

Having said that, I'm fine with it as is since I don't imagine more
stubdoms showing up.

> +                                libxl__xenfirmwaredir_path());
> +                    b_info->stubdomain_ramdisk =
> +                        libxl__abs_path(NOGC,
> +                                "stubdom-linux-rootfs",
> +                                libxl__xenfirmwaredir_path());
> +                    break;
> +                default:
> +                    abort();

Can we return an error instead?

> +            }
> +        }
> +    }
> +
>      if (!b_info->max_vcpus)
>          b_info->max_vcpus = 1;
>      if (!b_info->avail_vcpus.size) {

<snip>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

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

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

* Re: [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
@ 2020-01-20 19:36   ` Jason Andryuk
  2020-01-21 21:19     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 19:36 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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>
> ---
> 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
> ---

The code looks good.

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

One thought I have is dmargs is a string for mini-os and a directory
for linux stubdom.  It's toolstack managed, so it's not a problem.
But would a different xenstore node be less surprising to humans?

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
@ 2020-01-20 19:41   ` Jason Andryuk
  2020-01-21 21:22     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 19:41 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:40 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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 245d3f9..6ae0bd0 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -2720,10 +2720,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.

Also:

+=item B<stubdomain_memory=MBYTES>
+
+Start the stubdomain with MBYTES megabytes of RAM.

Regards,
Jason

>  =item B<device_model_stubdomain_override=BOOLEAN>
>
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index b881184..fc5dd65 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2525,6 +2525,13 @@ skip_usbdev:
>      xlu_cfg_replace_string(config, "device_model_user",
>                             &b_info->device_model_user, 0);
>
> +    xlu_cfg_replace_string (config, "stubdomain_kernel",
> +                            &b_info->stubdomain_kernel, 0);
> +    xlu_cfg_replace_string (config, "stubdomain_ramdisk",
> +                            &b_info->stubdomain_ramdisk, 0);
> +    if (!xlu_cfg_get_long (config, "stubdomain_memory", &l, 0))
> +        b_info->stubdomain_memkb = l * 1024;
> +
>  #define parse_extra_args(type)                                            \
>      e = xlu_cfg_get_list_as_string_list(config, "device_model_args"#type, \
>                                      &b_info->extra##type, 0);            \
> --
> 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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
@ 2020-01-20 19:44   ` Jason Andryuk
  2020-01-21 21:28     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 19:44 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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);

Looks like you used 4 spaces, but the upstream file uses hard tabs.

Regards,
Jason

>   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

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

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

* Re: [Xen-devel] [PATCH v4 10/16] tools: add missing libxenvchan cflags
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 10/16] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
@ 2020-01-20 19:58   ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-20 19:58 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> 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>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

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

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

* Re: [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy Marek Marczykowski-Górecki
  2020-01-15 11:02   ` Jan Beulich
  2020-01-17 18:44   ` Rich Persaud
@ 2020-01-21 19:43   ` Jason Andryuk
  2020-01-21 23:09     ` Marek Marczykowski-Górecki
  2 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-21 19:43 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> Add a simple proxy for tunneling socket connection over vchan. This is
> based on existing vchan-node* applications, but extended with socket
> support. vchan-socket-proxy serves both as a client and as a server,
> depending on parameters. It can be used to transparently communicate
> with an application in another domian that normally expose UNIX socket
> interface. Specifically, it's written to communicate with qemu running
> within stubdom.
>
> Server mode listens for vchan connections and when one is opened,
> connects to a pointed UNIX socket.  Client mode listens on UNIX
> socket and when someone connects, opens a vchan connection.  Only
> a single connection at a time is supported.
>
> Additionally, socket can be provided as a number - in which case it's
> interpreted as already open FD (in case of UNIX listening socket -
> listen() needs to be already called). Or "-" meaning stdin/stdout - in
> which case it is reduced to vchan-node2 functionality.
>
> Example usage:
>
> 1. (in dom0) vchan-socket-proxy --mode=client <DOMID>
>     /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
>
> 2. (in DOMID) vchan-socket-proxy --mode=server 0
>    /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
>
> This will listen on /run/qemu.(DOMID) in dom0 and whenever connection is
> made, it will connect to DOMID, where server process will connect to
> /run/qemu.(DOMID) there. When client disconnects, vchan connection is
> terminated and server vchan-socket-proxy process also disconnects from
> qemu.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---

Looks good.  A few typos and string updates below.

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

<snip>

> diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
> new file mode 100644
> index 0000000..6b4ae09
> --- /dev/null
> +++ b/tools/libvchan/vchan-socket-proxy.c

<snip>

> +static void usage(char** argv)
> +{
> +    fprintf(stderr, "usage:\n"
> +        "\t%s [options] domainid nodepath [socket-path|file-no|-]\n"
> +        "\n"
> +        "options:\n"
> +        "\t-m, --mode=client|server - vchan connection mode\n"

Add "(client by default)"?

> +        "\t-m, --state-path=path - xenstore path where write \"running\" to at startup\n"

-s is the short option here.

> +        "\t-v, --verbose - verbose logging\n"
> +        "\n"
> +        "client: client of a vchan connection, fourth parameter can be:\n"
> +        "\tsocket-path: listen on a UNIX socket at this path and connect to vchan\n"
> +        "\t  whenever new connection is accepted;\n"
> +        "\t  handle multiple _subsequent_ connections, until terminated\n"
> +        "\tfile-no: except open FD of a socket in listen mode; otherwise similar to socket-path\n"

Many of these lines are long and hard to read on 80 column terminals.

> +        "\t-: open vchan connection immediately and pass the data from stdin/stdout;\n"
> +        "\t  terminate when vchan connection is closed\n"
> +        "server: server of a vchan connection, fourth parameter can be:\n"
> +        "\tsocket-path: connect to this UNIX socket when new vchan connection is accepted\n"
> +        "\t  handle multiple _subsequent_ connections, until terminated\n"
> +        "\tfile-no: pass data to/from this FD; terminate when vchan connection is closed\n"
> +        "\t-: pass data to/from stdin/stdout; terminatate when vchan connection is closed\n",

s/terminatate/terminate/

> +        argv[0]);
> +    exit(1);
> +}
> +
> +#define BUFSIZE 8192
> +char inbuf[BUFSIZE];
> +char outbuf[BUFSIZE];
> +int insiz = 0;
> +int outsiz = 0;
> +int verbose = 0;
> +
> +static void vchan_wr(struct libxenvchan *ctrl) {
> +    int ret;
> +
> +    if (!insiz)
> +        return;
> +    ret = libxenvchan_write(ctrl, inbuf, insiz);
> +    if (ret < 0) {
> +        fprintf(stderr, "vchan write failed\n");
> +        exit(1);
> +    }
> +    if (verbose)
> +        fprintf(stderr, "written %d bytes to vchan\n", ret);

s/written/wrote/

> +    if (ret > 0) {
> +        insiz -= ret;
> +        memmove(inbuf, inbuf + ret, insiz);
> +    }
> +}
> +

<snip>

> +
> +int data_loop(struct libxenvchan *ctrl, int input_fd, int output_fd)
> +{
> +    int ret;
> +    int libxenvchan_fd;
> +    int max_fd;
> +
> +    libxenvchan_fd = libxenvchan_fd_for_select(ctrl);
> +    for (;;) {
> +        fd_set rfds;
> +        fd_set wfds;
> +        FD_ZERO(&rfds);
> +        FD_ZERO(&wfds);
> +
> +        max_fd = -1;
> +        if (input_fd != -1 && insiz != BUFSIZE) {
> +            FD_SET(input_fd, &rfds);
> +            if (input_fd > max_fd)
> +                max_fd = input_fd;
> +        }
> +        if (output_fd != -1 && outsiz) {
> +            FD_SET(output_fd, &wfds);
> +            if (output_fd > max_fd)
> +                max_fd = output_fd;
> +        }
> +        FD_SET(libxenvchan_fd, &rfds);
> +        if (libxenvchan_fd > max_fd)
> +            max_fd = libxenvchan_fd;
> +        ret = select(max_fd + 1, &rfds, &wfds, NULL, NULL);
> +        if (ret < 0) {
> +            perror("select");
> +            exit(1);
> +        }
> +        if (FD_ISSET(libxenvchan_fd, &rfds)) {
> +            libxenvchan_wait(ctrl);
> +            if (!libxenvchan_is_open(ctrl)) {
> +                if (verbose)
> +                    fprintf(stderr, "vchan client disconnected\n");
> +                while (outsiz)
> +                    socket_wr(output_fd);
> +                close(output_fd);
> +                close(input_fd);
> +                discard_buffers(ctrl);
> +                break;
> +            }
> +            vchan_wr(ctrl);
> +        }
> +
> +        /* socket_fd guaranteed to be != -1 */

Old comment?

> +        if (FD_ISSET(input_fd, &rfds)) {
> +            ret = read(input_fd, inbuf + insiz, BUFSIZE - insiz);
> +            if (ret < 0 && errno != EAGAIN)
> +                exit(1);
> +            if (verbose)
> +                fprintf(stderr, "from-unix: %.*s\n", ret, inbuf + insiz);
> +            if (ret == 0) {
> +                /* EOF on socket, write everything in the buffer and close the
> +                 * socket */

Change to socket to input_fd?

> +                while (insiz) {
> +                    vchan_wr(ctrl);
> +                    libxenvchan_wait(ctrl);
> +                }
> +                close(input_fd);
> +                input_fd = -1;
> +                /* TODO: maybe signal the vchan client somehow? */
> +                break;
> +            }
> +            if (ret)
> +                insiz += ret;
> +            vchan_wr(ctrl);
> +        }
> +        if (FD_ISSET(output_fd, &wfds))
> +            socket_wr(output_fd);
> +        while (libxenvchan_data_ready(ctrl) && outsiz < BUFSIZE) {
> +            ret = libxenvchan_read(ctrl, outbuf + outsiz, BUFSIZE - outsiz);
> +            if (ret < 0)
> +                exit(1);
> +            if (verbose)
> +                fprintf(stderr, "from-vchan: %.*s\n", ret, outbuf + outsiz);
> +            outsiz += ret;
> +            socket_wr(output_fd);
> +        }
> +    }
> +    return 0;
> +}
> +

<snip>

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

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

* Re: [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain Marek Marczykowski-Górecki
@ 2020-01-21 20:17   ` Jason Andryuk
  2020-01-21 23:46     ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 53+ messages in thread
From: Jason Andryuk @ 2020-01-21 20:17 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> Access to QMP of QEMU in Linux stubdomain is possible over vchan
> connection. Handle the actual vchan connection in a separate process
> (vchan-socket-proxy). This simplified integration with QMP (already
> quite complex), but also allows preliminary filtering of (potentially
> malicious) QMP input.
> Since only one client can be connected to vchan server at the same time
> and it is not enforced by the libxenvchan itself, additional client-side
> locking is needed. It is implicitly implemented by vchan-socket-proxy,
> as it handle only one connection at a time. Note that qemu supports only
> one simultaneous client on a control socket anyway (but in UNIX socket
> case, it enforce it server-side), so it doesn't add any extra
> limitation.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> Changes in v4:
>  - new patch, in place of both "libxl: use vchan for QMP access ..."
> ---
>  tools/configure.ac           |   9 ++-
>  tools/libxl/libxl_dm.c       | 159 ++++++++++++++++++++++++++++++++++--
>  tools/libxl/libxl_internal.h |   1 +-
>  3 files changed, 161 insertions(+), 8 deletions(-)
>
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 8d86c42..20bbdbf 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -192,6 +192,15 @@ AC_SUBST(qemu_xen)
>  AC_SUBST(qemu_xen_path)
>  AC_SUBST(qemu_xen_systemd)
>
> +AC_ARG_WITH([stubdom-qmp-proxy],
> +    AC_HELP_STRING([--stubdom-qmp-proxy@<:@=PATH@:>@],
> +        [Use supplied binary PATH as a QMP proxy into stubdomain]),[

Thanks for making it configurable :)

> +    stubdom_qmp_proxy="$withval"
> +],[
> +    stubdom_qmp_proxy="$bindir/vchan-socket-proxy"
> +])
> +AC_DEFINE_UNQUOTED([STUBDOM_QMP_PROXY_PATH], ["$stubdom_qmp_proxy"], [QMP proxy path])
> +
>  AC_ARG_WITH([system-seabios],
>      AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@],
>         [Use system supplied seabios PATH instead of building and installing
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 528ca3e..23ac7e4 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1183,7 +1183,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, stubdomain setup it itself */

I think this would be clearer:
/* QMP access to qemu running in stubdomain is done over vchan.  The
stubdomain init script
 * adds the appropriate monitor options for vchan-socket-proxy. */

In the block below, the -no-shutdown option is added to qemu, which
will not be done for linux stubdomain.
-no-shutdown
       Don't exit QEMU on guest shutdown, but instead only stop the
       emulation.  This allows for instance switching to monitor to commit
       changes to the disk image.

It's something I noticed, but I don't know if it matters to us.

>      if (!is_stubdom) {
>          flexarray_append(dm_args, "-chardev");
>          if (state->dm_monitor_fd >= 0) {
> @@ -2178,6 +2178,23 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc,

<snip>

> @@ -2460,24 +2477,150 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
>              goto out;
>      }
>
> +    sdss->qmp_proxy_spawn.ao = ao;
> +    if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
> +        spawn_qmp_proxy(egc, sdss);
> +    } else {
> +        qmp_proxy_spawn_outcome(egc, sdss, 0);
> +    }
> +
> +    return;
> +
> +out:
> +    assert(ret);
> +    qmp_proxy_spawn_outcome(egc, sdss, ret);
> +}
> +
> +static void spawn_qmp_proxy(libxl__egc *egc,
> +                            libxl__stub_dm_spawn_state *sdss)
> +{
> +    STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
> +    const uint32_t guest_domid = sdss->dm.guest_domid;
> +    const uint32_t dm_domid = sdss->pvqemu.guest_domid;
> +    const char *dom_path = libxl__xs_get_dompath(gc, dm_domid);
> +    char **args;
> +    int nr = 0;
> +    int rc, logfile_w, null;
> +
> +    if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) {
> +        LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid);
> +    sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path);
> +    sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path);

Since this is the vchan-socket-proxy in dom0, should it write to
"device-model/%u/qmp-proxy-state" underneath dom0?

> +
> +    sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
> +    sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid;
> +    sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm;
> +    sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed;
> +    sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached;
> +
> +    const int arraysize = 6;
> +    GCNEW_ARRAY(args, arraysize);
> +    args[nr++] = STUBDOM_QMP_PROXY_PATH;
> +    args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath);
> +    args[nr++] = GCSPRINTF("%u", dm_domid);
> +    args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid);

Thinking of OpenXT"s qmp-helper, this path isn't useful.  But it is
for vchan-socket-proxy, so qmp-helper could just change to ignore it.

> +    args[nr++] = (char*)libxl__qemu_qmp_path(gc, guest_domid);

qmp-helper takes just the stub_domid and domid.  The domid is just
used to generate the above path, but taking the path would be cleaner.

> +    args[nr++] = NULL;
> +    assert(nr == arraysize);

This generally looks good.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 15/16] libxl: ignore emulated IDE disks beyond the first 4
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 15/16] libxl: ignore emulated IDE disks beyond the first 4 Marek Marczykowski-Górecki
@ 2020-01-21 20:24   ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-21 20:24 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> Qemu supports only 4 emulated IDE disks, when given more (or with higher
> indexes), it will fail to start. Since the disks can still be accessible
> using PV interface, just ignore emulated path and log a warning, instead
> of rejecting the configuration altogether.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

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

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

* Re: [Xen-devel] [PATCH v4 16/16] libxl: consider also qemu in stubdomain in libxl__dm_active check
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 16/16] libxl: consider also qemu in stubdomain in libxl__dm_active check Marek Marczykowski-Górecki
@ 2020-01-21 20:25   ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-21 20:25 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> Since qemu-xen can now run in stubdomain too, handle this case when
> checking it's state too.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

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

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

* Re: [Xen-devel] [PATCH v4 13/16] Regenerate autotools files
  2020-01-15 21:57   ` Rich Persaud
@ 2020-01-21 20:56     ` Marek Marczykowski-Górecki
  2020-01-21 21:28       ` Rich Persaud
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 20:56 UTC (permalink / raw)
  To: Rich Persaud; +Cc: xen-devel, Ian Jackson, Wei Liu, Samuel Thibault

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

On Wed, Jan 15, 2020 at 04:57:29PM -0500, Rich Persaud wrote:
> > On Jan 14, 2020, at 21:42, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
> > Since we have those generated files committed to the repo (why?!),
> > update them after changing configure.ac.
> 
> Is there any reason not to remove the generated configure files?  A developer using generated files on system B would be incorporating configuration assumptions from system A where the configure script was generated.  If we are going to ship configure scripts, do we need to document a "system A" reference distro/environment where all configure scripts from Xen will be generated?
> 
> 
> Other notes:
> 
> 1.  Debian autoreconf works in the Xen root directory, but the default OpenEmbedded autoreconf uses Gnu libtoolize and fails because some Xen build subdirectories don't have configure.ac/.in.   
> 
> 2.  If OpenEmbedded autoreconf is run only in the tools directory (where it works and generates a new tools configure), then root configure (generated from older configure.ac) will silently ignore the newer tools configure and write config.h _without_ tools-specific config, such as the vchan QMP proxy.
> 
> 3. If autoreconf runs successfully in the root directory, then tools-specific configure is correctly generated and everything works as expected.
> 
> This silent failure could be avoided by deleting the generated configure scripts.  There may be other failure modes for using System A generated scripts on downstream build system B.

Yes, I think general good practices are:
1. don't keep generated autotools files in version control system
2. generate them into release tarballs

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 02/16] Document ioemu Linux stubdomain protocol
  2020-01-20 18:54   ` Jason Andryuk
@ 2020-01-21 21:08     ` Marek Marczykowski-Górecki
  2020-01-22 14:04       ` Jason Andryuk
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 21:08 UTC (permalink / raw)
  To: Jason Andryuk; +Cc: xen-devel, Ian Jackson, Wei Liu, Samuel Thibault

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

On Mon, Jan 20, 2020 at 01:54:04PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> 
> <snip>
> 
> > +
> > +Limitations:
> > + - PCI passthrough require permissive mode
> > + - only one nic is supported
> 
> Why is only 1 nic supported?  Multiple were supported previously, but
> peeking ahead in the series, 

This is mostly limitation of stubdomain side, not toolstack side.
Startup script setup eth0 only.

> script=/etc/qemu-ifup is no longer
> specified.

Yes, that's to allow -sandbox ...,spawn=deny inside stubdomain.
The equivalent actions are handled by listening for qmp events.


-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 04/16] libxl: Allow running qemu-xen in stubdomain
  2020-01-20 18:56   ` Jason Andryuk
@ 2020-01-21 21:12     ` Marek Marczykowski-Górecki
  0 siblings, 0 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 21:12 UTC (permalink / raw)
  To: Jason Andryuk; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

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

On Mon, Jan 20, 2020 at 01:56:51PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> 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
> > ---
> 
> <snip>
> 
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -2299,6 +2299,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)
> 
> This is unused in the series, as far as I can tell.

Yes, all the calls are commented out, as exact same condition is implied
from the context. But I think a canonical function to do that is still
useful, if needed anywhere in the codebase in the future.

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options.
  2020-01-20 19:24   ` Jason Andryuk
@ 2020-01-21 21:18     ` Marek Marczykowski-Górecki
  2020-01-22 14:25       ` Jason Andryuk
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 21:18 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: Wei Liu, Ian Jackson, Simon Gaiser, Anthony PERARD, xen-devel,
	Eric Shelton

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

On Mon, Jan 20, 2020 at 02:24:18PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> 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
> 
> Since this is here...
> 
> > Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
> > [drop Qubes-specific parts]
> 
> ...maybe mention dropping downscript here?  Otherwise the commit
> message and contents don't match.

Ah, indeed.

> 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> 
> <snip>
> 
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index 142b960..a6d40b7 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -169,6 +169,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",
> 
> Not to bikeshed, but this came up in a conversation a little while
> ago.  Stubdom is a generic name, and this code is for a device model.
> So some combination of qemu{,-dm}{,-linux}-kernel seems more
> descriptive.

Minios-based use ioemu-stubdom, so maybe
ioemu-stubdom-linux-{kernel,rootfs}?

> Having said that, I'm fine with it as is since I don't imagine more
> stubdoms showing up.
> 
> > +                                libxl__xenfirmwaredir_path());
> > +                    b_info->stubdomain_ramdisk =
> > +                        libxl__abs_path(NOGC,
> > +                                "stubdom-linux-rootfs",
> > +                                libxl__xenfirmwaredir_path());
> > +                    break;
> > +                default:
> > +                    abort();
> 
> Can we return an error instead?

For invalid enum value? 

> > +            }
> > +        }
> > +    }
> > +
> >      if (!b_info->max_vcpus)
> >          b_info->max_vcpus = 1;
> >      if (!b_info->avail_vcpus.size) {
> 
> <snip>
> 
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys
  2020-01-20 19:36   ` Jason Andryuk
@ 2020-01-21 21:19     ` Marek Marczykowski-Górecki
  2020-01-22 14:39       ` Jason Andryuk
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 21:19 UTC (permalink / raw)
  To: Jason Andryuk; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

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

On Mon, Jan 20, 2020 at 02:36:08PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> 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>
> > ---
> > 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
> > ---
> 
> The code looks good.
> 
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> 
> One thought I have is dmargs is a string for mini-os and a directory
> for linux stubdom.  It's toolstack managed, so it's not a problem.
> But would a different xenstore node be less surprising to humans?

dmargs_list?

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser
  2020-01-20 19:41   ` Jason Andryuk
@ 2020-01-21 21:22     ` Marek Marczykowski-Górecki
  2020-01-22 14:39       ` Jason Andryuk
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 21:22 UTC (permalink / raw)
  To: Jason Andryuk; +Cc: xen-devel, Ian Jackson, Wei Liu

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

On Mon, Jan 20, 2020 at 02:41:07PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:40 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> 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 245d3f9..6ae0bd0 100644
> > --- a/docs/man/xl.cfg.5.pod.in
> > +++ b/docs/man/xl.cfg.5.pod.in
> > @@ -2720,10 +2720,25 @@ model which they were installed with.
> >
> Also:
> 
> +=item B<stubdomain_memory=MBYTES>
> +
> +Start the stubdomain with MBYTES megabytes of RAM.

Added, together with default value.

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected
  2020-01-20 19:44   ` Jason Andryuk
@ 2020-01-21 21:28     ` Marek Marczykowski-Górecki
  2020-01-22 14:43       ` Jason Andryuk
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 21:28 UTC (permalink / raw)
  To: Jason Andryuk; +Cc: xen-devel, Ian Jackson, Wei Liu

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

On Mon, Jan 20, 2020 at 02:44:58PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> 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);
> 
> Looks like you used 4 spaces, but the upstream file uses hard tabs.

Indeed. CODING_STYLE says spaces, but it also says some tools/* are not
directly covered by this file. Should I use this occasion to convert
tools/libvchan/* to spaces (in a separate patch), or keep tabs (and
adjust my patch)?

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 13/16] Regenerate autotools files
  2020-01-21 20:56     ` Marek Marczykowski-Górecki
@ 2020-01-21 21:28       ` Rich Persaud
  2020-01-22  8:57         ` Lars Kurth
  0 siblings, 1 reply; 53+ messages in thread
From: Rich Persaud @ 2020-01-21 21:28 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki, Lars Kurth, xen-devel
  Cc: Ian Jackson, Andrew Cooper, Jason Andryuk, Wei Liu, Samuel Thibault

On Jan 21, 2020, at 15:58, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
> 
> On Wed, Jan 15, 2020 at 04:57:29PM -0500, Rich Persaud wrote:
>>>> On Jan 14, 2020, at 21:42, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
>>> Since we have those generated files committed to the repo (why?!),
>>> update them after changing configure.ac.
>> 
>> Is there any reason not to remove the generated configure files?  A developer using generated files on system B would be incorporating configuration assumptions from system A where the configure script was generated.  If we are going to ship configure scripts, do we need to document a "system A" reference distro/environment where all configure scripts from Xen will be generated?
>> 
>> 
>> Other notes:
>> 
>> 1.  Debian autoreconf works in the Xen root directory, but the default OpenEmbedded autoreconf uses Gnu libtoolize and fails because some Xen build subdirectories don't have configure.ac/.in.   
>> 
>> 2.  If OpenEmbedded autoreconf is run only in the tools directory (where it works and generates a new tools configure), then root configure (generated from older configure.ac) will silently ignore the newer tools configure and write config.h _without_ tools-specific config, such as the vchan QMP proxy.
>> 
>> 3. If autoreconf runs successfully in the root directory, then tools-specific configure is correctly generated and everything works as expected.
>> 
>> This silent failure could be avoided by deleting the generated configure scripts.  There may be other failure modes for using System A generated scripts on downstream build system B.
> 
> Yes, I think general good practices are:
> 1. don't keep generated autotools files in version control system
> 2. generate them into release tarballs

A potential topic for the next Xen community call:  can we delete generated autotools files from the Xen tree and update the release process to generate+bundle them with release tarballs?

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

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

* Re: [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy
  2020-01-21 19:43   ` Jason Andryuk
@ 2020-01-21 23:09     ` Marek Marczykowski-Górecki
  0 siblings, 0 replies; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 23:09 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel

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

On Tue, Jan 21, 2020 at 02:43:03PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> >
> > Add a simple proxy for tunneling socket connection over vchan. This is
> > based on existing vchan-node* applications, but extended with socket
> > support. vchan-socket-proxy serves both as a client and as a server,
> > depending on parameters. It can be used to transparently communicate
> > with an application in another domian that normally expose UNIX socket
> > interface. Specifically, it's written to communicate with qemu running
> > within stubdom.
> >
> > Server mode listens for vchan connections and when one is opened,
> > connects to a pointed UNIX socket.  Client mode listens on UNIX
> > socket and when someone connects, opens a vchan connection.  Only
> > a single connection at a time is supported.
> >
> > Additionally, socket can be provided as a number - in which case it's
> > interpreted as already open FD (in case of UNIX listening socket -
> > listen() needs to be already called). Or "-" meaning stdin/stdout - in
> > which case it is reduced to vchan-node2 functionality.
> >
> > Example usage:
> >
> > 1. (in dom0) vchan-socket-proxy --mode=client <DOMID>
> >     /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
> >
> > 2. (in DOMID) vchan-socket-proxy --mode=server 0
> >    /local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
> >
> > This will listen on /run/qemu.(DOMID) in dom0 and whenever connection is
> > made, it will connect to DOMID, where server process will connect to
> > /run/qemu.(DOMID) there. When client disconnects, vchan connection is
> > terminated and server vchan-socket-proxy process also disconnects from
> > qemu.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> 
> Looks good.  A few typos and string updates below.

Thanks, adjusted.

> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain
  2020-01-21 20:17   ` Jason Andryuk
@ 2020-01-21 23:46     ` Marek Marczykowski-Górecki
  2020-01-24 14:05       ` Jason Andryuk
  0 siblings, 1 reply; 53+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-21 23:46 UTC (permalink / raw)
  To: Jason Andryuk; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

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

On Tue, Jan 21, 2020 at 03:17:39PM -0500, Jason Andryuk wrote:
> On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> >
> > Access to QMP of QEMU in Linux stubdomain is possible over vchan
> > connection. Handle the actual vchan connection in a separate process
> > (vchan-socket-proxy). This simplified integration with QMP (already
> > quite complex), but also allows preliminary filtering of (potentially
> > malicious) QMP input.
> > Since only one client can be connected to vchan server at the same time
> > and it is not enforced by the libxenvchan itself, additional client-side
> > locking is needed. It is implicitly implemented by vchan-socket-proxy,
> > as it handle only one connection at a time. Note that qemu supports only
> > one simultaneous client on a control socket anyway (but in UNIX socket
> > case, it enforce it server-side), so it doesn't add any extra
> > limitation.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> > Changes in v4:
> >  - new patch, in place of both "libxl: use vchan for QMP access ..."
> > ---
> >  tools/configure.ac           |   9 ++-
> >  tools/libxl/libxl_dm.c       | 159 ++++++++++++++++++++++++++++++++++--
> >  tools/libxl/libxl_internal.h |   1 +-
> >  3 files changed, 161 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/configure.ac b/tools/configure.ac
> > index 8d86c42..20bbdbf 100644
> > --- a/tools/configure.ac
> > +++ b/tools/configure.ac
> > @@ -192,6 +192,15 @@ AC_SUBST(qemu_xen)
> >  AC_SUBST(qemu_xen_path)
> >  AC_SUBST(qemu_xen_systemd)
> >
> > +AC_ARG_WITH([stubdom-qmp-proxy],
> > +    AC_HELP_STRING([--stubdom-qmp-proxy@<:@=PATH@:>@],
> > +        [Use supplied binary PATH as a QMP proxy into stubdomain]),[
> 
> Thanks for making it configurable :)
> 
> > +    stubdom_qmp_proxy="$withval"
> > +],[
> > +    stubdom_qmp_proxy="$bindir/vchan-socket-proxy"
> > +])
> > +AC_DEFINE_UNQUOTED([STUBDOM_QMP_PROXY_PATH], ["$stubdom_qmp_proxy"], [QMP proxy path])
> > +
> >  AC_ARG_WITH([system-seabios],
> >      AS_HELP_STRING([--with-system-seabios@<:@=PATH@:>@],
> >         [Use system supplied seabios PATH instead of building and installing
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 528ca3e..23ac7e4 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -1183,7 +1183,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, stubdomain setup it itself */
> 
> I think this would be clearer:
> /* QMP access to qemu running in stubdomain is done over vchan.  The
> stubdomain init script
>  * adds the appropriate monitor options for vchan-socket-proxy. */

Yes, clearer.

> In the block below, the -no-shutdown option is added to qemu, which
> will not be done for linux stubdomain.
> -no-shutdown
>        Don't exit QEMU on guest shutdown, but instead only stop the
>        emulation.  This allows for instance switching to monitor to commit
>        changes to the disk image.
> 
> It's something I noticed, but I don't know if it matters to us.

I'll move it outside, looks like unrelated change.

> >      if (!is_stubdom) {
> >          flexarray_append(dm_args, "-chardev");
> >          if (state->dm_monitor_fd >= 0) {
> > @@ -2178,6 +2178,23 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc,
> 
> <snip>
> 
> > @@ -2460,24 +2477,150 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
> >              goto out;
> >      }
> >
> > +    sdss->qmp_proxy_spawn.ao = ao;
> > +    if (libxl__stubdomain_is_linux(&guest_config->b_info)) {
> > +        spawn_qmp_proxy(egc, sdss);
> > +    } else {
> > +        qmp_proxy_spawn_outcome(egc, sdss, 0);
> > +    }
> > +
> > +    return;
> > +
> > +out:
> > +    assert(ret);
> > +    qmp_proxy_spawn_outcome(egc, sdss, ret);
> > +}
> > +
> > +static void spawn_qmp_proxy(libxl__egc *egc,
> > +                            libxl__stub_dm_spawn_state *sdss)
> > +{
> > +    STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
> > +    const uint32_t guest_domid = sdss->dm.guest_domid;
> > +    const uint32_t dm_domid = sdss->pvqemu.guest_domid;
> > +    const char *dom_path = libxl__xs_get_dompath(gc, dm_domid);
> > +    char **args;
> > +    int nr = 0;
> > +    int rc, logfile_w, null;
> > +
> > +    if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) {
> > +        LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH);
> > +        rc = ERROR_FAIL;
> > +        goto out;
> > +    }
> > +
> > +    sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid);
> > +    sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path);
> > +    sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path);
> 
> Since this is the vchan-socket-proxy in dom0, should it write to
> "device-model/%u/qmp-proxy-state" underneath dom0?

Yes, that would be more consistent. But pid should stay where it is
(it's also where dom0 qemu pid is being written).

> > +
> > +    sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
> > +    sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid;
> > +    sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm;
> > +    sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed;
> > +    sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached;
> > +
> > +    const int arraysize = 6;
> > +    GCNEW_ARRAY(args, arraysize);
> > +    args[nr++] = STUBDOM_QMP_PROXY_PATH;
> > +    args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath);
> > +    args[nr++] = GCSPRINTF("%u", dm_domid);
> > +    args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid);
> 
> Thinking of OpenXT"s qmp-helper, this path isn't useful.  But it is
> for vchan-socket-proxy, so qmp-helper could just change to ignore it.

For vchan we could use also a port number (and then it will encode it
into a xenstore path). This is in fact how we use libvchan in Qubes. I
opted for explicit path only because of lack of idea for some meaningful
port number ;) But I'm open for suggestions.
I guess that would be useful for Argo version then.

> > +    args[nr++] = (char*)libxl__qemu_qmp_path(gc, guest_domid);
> 
> qmp-helper takes just the stub_domid and domid.  The domid is just
> used to generate the above path, but taking the path would be cleaner.
> 
> > +    args[nr++] = NULL;
> > +    assert(nr == arraysize);
> 
> This generally looks good.
> 
> Regards,
> Jason

-- 
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] 53+ messages in thread

* Re: [Xen-devel] [PATCH v4 13/16] Regenerate autotools files
  2020-01-21 21:28       ` Rich Persaud
@ 2020-01-22  8:57         ` Lars Kurth
  0 siblings, 0 replies; 53+ messages in thread
From: Lars Kurth @ 2020-01-22  8:57 UTC (permalink / raw)
  To: Rich Persaud, Marek Marczykowski-Górecki, xen-devel
  Cc: Ian Jackson, Andrew Cooper, Jason Andryuk, Wei Liu, Samuel Thibault



On 21/01/2020, 21:29, "Rich Persaud" <persaur@gmail.com> wrote:

    On Jan 21, 2020, at 15:58, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
    > 
    > On Wed, Jan 15, 2020 at 04:57:29PM -0500, Rich Persaud wrote:
    >>>> On Jan 14, 2020, at 21:42, Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> wrote:
    >>> Since we have those generated files committed to the repo (why?!),
    >>> update them after changing configure.ac.
    >> 
    >> Is there any reason not to remove the generated configure files?  A developer using generated files on system B would be incorporating configuration assumptions from system A where the configure script was generated.  If we are going to ship configure scripts, do we need to document a "system A" reference distro/environment where all configure scripts from Xen will be generated?
    >> 
    >> 
    >> Other notes:
    >> 
    >> 1.  Debian autoreconf works in the Xen root directory, but the default OpenEmbedded autoreconf uses Gnu libtoolize and fails because some Xen build subdirectories don't have configure.ac/.in.   
    >> 
    >> 2.  If OpenEmbedded autoreconf is run only in the tools directory (where it works and generates a new tools configure), then root configure (generated from older configure.ac) will silently ignore the newer tools configure and write config.h _without_ tools-specific config, such as the vchan QMP proxy.
    >> 
    >> 3. If autoreconf runs successfully in the root directory, then tools-specific configure is correctly generated and everything works as expected.
    >> 
    >> This silent failure could be avoided by deleting the generated configure scripts.  There may be other failure modes for using System A generated scripts on downstream build system B.
    > 
    > Yes, I think general good practices are:
    > 1. don't keep generated autotools files in version control system
    > 2. generate them into release tarballs
    
    A potential topic for the next Xen community call:  can we delete generated autotools files from the Xen tree and update the release process to generate+bundle them with release tarballs?
    
I am happy to put this on the agenda, if someone reminds me closer to the time
Lars

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

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

* Re: [Xen-devel] [PATCH v4 02/16] Document ioemu Linux stubdomain protocol
  2020-01-21 21:08     ` Marek Marczykowski-Górecki
@ 2020-01-22 14:04       ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-22 14:04 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: xen-devel, Ian Jackson, Wei Liu, Samuel Thibault

On Tue, Jan 21, 2020 at 4:08 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Mon, Jan 20, 2020 at 01:54:04PM -0500, Jason Andryuk wrote:
> > On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> wrote:
> >
> > <snip>
> >
> > > +
> > > +Limitations:
> > > + - PCI passthrough require permissive mode
> > > + - only one nic is supported
> >
> > Why is only 1 nic supported?  Multiple were supported previously, but
> > peeking ahead in the series,
>
> This is mostly limitation of stubdomain side, not toolstack side.
> Startup script setup eth0 only.

I peeked the script, and it looks like the nic ifname= sed expression
only handles one nic.  Since dmargs is now an array, it should to
handle multiple.

Anyway, there doesn't seem to be an hard limitation.

> > script=/etc/qemu-ifup is no longer
> > specified.
>
> Yes, that's to allow -sandbox ...,spawn=deny inside stubdomain.
> The equivalent actions are handled by listening for qmp events.

Ah, okay.  Yeah, that's a good idea.

Thanks,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options.
  2020-01-21 21:18     ` Marek Marczykowski-Górecki
@ 2020-01-22 14:25       ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-22 14:25 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Wei Liu, Ian Jackson, Simon Gaiser, Anthony PERARD, xen-devel,
	Eric Shelton

On Tue, Jan 21, 2020 at 4:18 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Mon, Jan 20, 2020 at 02:24:18PM -0500, Jason Andryuk wrote:
> > On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> 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
> >
> > Since this is here...
> >
> > > Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
> > > [drop Qubes-specific parts]
> >
> > ...maybe mention dropping downscript here?  Otherwise the commit
> > message and contents don't match.
>
> Ah, indeed.
>
> >
> > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > > ---
> >
> > <snip>
> >
> > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > > index 142b960..a6d40b7 100644
> > > --- a/tools/libxl/libxl_create.c
> > > +++ b/tools/libxl/libxl_create.c
> > > @@ -169,6 +169,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",
> >
> > Not to bikeshed, but this came up in a conversation a little while
> > ago.  Stubdom is a generic name, and this code is for a device model.
> > So some combination of qemu{,-dm}{,-linux}-kernel seems more
> > descriptive.
>
> Minios-based use ioemu-stubdom, so maybe
> ioemu-stubdom-linux-{kernel,rootfs}?

I think ioemu is the name of the legacy fork of qemu.  Linux stubdoms
are running close to upstream qemu, so that's why I suggested that
name.  But ioemu does match Mini-os, and convey the purpose of the
stubdom, so it works from the perspective.  I leave the name up to
you.

> > Having said that, I'm fine with it as is since I don't imagine more
> > stubdoms showing up.
> >
> > > +                                libxl__xenfirmwaredir_path());
> > > +                    b_info->stubdomain_ramdisk =
> > > +                        libxl__abs_path(NOGC,
> > > +                                "stubdom-linux-rootfs",
> > > +                                libxl__xenfirmwaredir_path());

I set stubdomain_ramdisk, but not stubdomain_kernel, and the ramdisk
option wasn't honored.  This assignment needs to be under 'if
(!b_info->stubdomain_ramdisk) {'

> > > +                    break;
> > > +                default:
> > > +                    abort();
> >
> > Can we return an error instead?
>
> For invalid enum value?

Okay, that use makes sense.  It was a reflexive response to seeing
abort in a library.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys
  2020-01-21 21:19     ` Marek Marczykowski-Górecki
@ 2020-01-22 14:39       ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-22 14:39 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 21, 2020 at 4:19 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Mon, Jan 20, 2020 at 02:36:08PM -0500, Jason Andryuk wrote:
> > On Tue, Jan 14, 2020 at 9:41 PM Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> 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>
> > > ---
> > > 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
> > > ---
> >
> > The code looks good.
> >
> > Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> >
> > One thought I have is dmargs is a string for mini-os and a directory
> > for linux stubdom.  It's toolstack managed, so it's not a problem.
> > But would a different xenstore node be less surprising to humans?
>
> dmargs_list?

dmargs_list works.  dmargv to mimic argv?  That might be too subtle.

I'm not asking for the change.  I just wanted to bring it up for discussion.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser
  2020-01-21 21:22     ` Marek Marczykowski-Górecki
@ 2020-01-22 14:39       ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-22 14:39 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 21, 2020 at 4:22 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Mon, Jan 20, 2020 at 02:41:07PM -0500, Jason Andryuk wrote:
> > On Tue, Jan 14, 2020 at 9:40 PM Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> 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 245d3f9..6ae0bd0 100644
> > > --- a/docs/man/xl.cfg.5.pod.in
> > > +++ b/docs/man/xl.cfg.5.pod.in
> > > @@ -2720,10 +2720,25 @@ model which they were installed with.
> > >
> > Also:
> >
> > +=item B<stubdomain_memory=MBYTES>
> > +
> > +Start the stubdomain with MBYTES megabytes of RAM.
>
> Added, together with default value.

Thanks.  Good idea to add the default.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected
  2020-01-21 21:28     ` Marek Marczykowski-Górecki
@ 2020-01-22 14:43       ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-22 14:43 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki; +Cc: xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 21, 2020 at 4:28 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Mon, Jan 20, 2020 at 02:44:58PM -0500, Jason Andryuk wrote:
> > On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> 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);
> >
> > Looks like you used 4 spaces, but the upstream file uses hard tabs.
>
> Indeed. CODING_STYLE says spaces, but it also says some tools/* are not
> directly covered by this file. Should I use this occasion to convert
> tools/libvchan/* to spaces (in a separate patch), or keep tabs (and
> adjust my patch)?

Maybe adjust your patch for tabs in case someone wants to backport it.
And then convert to spaces in a separate patch.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain.
  2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
                   ` (15 preceding siblings ...)
  2020-01-15  2:39 ` [Xen-devel] [PATCH v4 16/16] libxl: consider also qemu in stubdomain in libxl__dm_active check Marek Marczykowski-Górecki
@ 2020-01-22 16:50 ` Jason Andryuk
  16 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-22 16:50 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Simon Gaiser, xen-devel, Wei Liu, Ian Jackson, Eric Shelton

On Tue, Jan 14, 2020 at 9:42 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:

<snip>

> Later patches add QMP over libvchan connection support. The actual connection
> is made in a separate process. As discussed on Xen Summit 2019, this allows to
> apply some basic checks and/or filtering (not part of this series), to limit
> libxl exposure for potentially malicious stubdomain.

Thanks for working on this!  I think the separate process is nicer.

> 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.

Just as an FYI, Marek's use of dracut is mainly for dracut-install to
copy a binary & dependent libraries when generating the initramfs
(https://github.com/marmarek/qubes-vmm-xen-stubdom-linux/blob/master/rootfs/gen).
The initramfs isn't running dracut scripts.  Using initramfs-tools
hook-functions:copy_exec() for similar functionality is a possibility.

> 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).

A mostly unpatched QEMU works for networking & disk.  The exception is
PCI passthrough, which needs some patches.  I tested this by removing
patches from Marek's repo, except for the seccomp ones and
disable-nic-option-rom.patch.  Without disable-nic-option-rom.patch,
QEMU fails to start with 'failed to find romfile "efi-rtl8139.rom"'

One issue I've noticed is QEMU ~4.1 calls getrandom() during startup.
In a stubdom there is insufficient entropy, so QEMU blocks and stubdom
startup times out.  You can avoid getrandom() blocking with
CONFIG_RANDOM_TRUST_CPU or
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=50ee7529ec4500c88f8664560770a7a1b65db72b
or some other way of adding entropy.

Regards,
Jason

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

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

* Re: [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain
  2020-01-21 23:46     ` Marek Marczykowski-Górecki
@ 2020-01-24 14:05       ` Jason Andryuk
  0 siblings, 0 replies; 53+ messages in thread
From: Jason Andryuk @ 2020-01-24 14:05 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Wei Liu

On Tue, Jan 21, 2020 at 6:46 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:

<snip>

> > > +static void spawn_qmp_proxy(libxl__egc *egc,
> > > +                            libxl__stub_dm_spawn_state *sdss)
> > > +{
> > > +    STATE_AO_GC(sdss->qmp_proxy_spawn.ao);
> > > +    const uint32_t guest_domid = sdss->dm.guest_domid;
> > > +    const uint32_t dm_domid = sdss->pvqemu.guest_domid;
> > > +    const char *dom_path = libxl__xs_get_dompath(gc, dm_domid);
> > > +    char **args;
> > > +    int nr = 0;
> > > +    int rc, logfile_w, null;
> > > +
> > > +    if (access(STUBDOM_QMP_PROXY_PATH, X_OK) < 0) {
> > > +        LOGED(ERROR, guest_domid, "qmp proxy %s is not executable", STUBDOM_QMP_PROXY_PATH);
> > > +        rc = ERROR_FAIL;
> > > +        goto out;
> > > +    }
> > > +
> > > +    sdss->qmp_proxy_spawn.what = GCSPRINTF("domain %d device model qmp proxy", guest_domid);
> > > +    sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid", dom_path);
> > > +    sdss->qmp_proxy_spawn.xspath = GCSPRINTF("%s/image/qmp-proxy-state", dom_path);
> >
> > Since this is the vchan-socket-proxy in dom0, should it write to
> > "device-model/%u/qmp-proxy-state" underneath dom0?
>
> Yes, that would be more consistent. But pid should stay where it is
> (it's also where dom0 qemu pid is being written).

Hmm, that split between pids and device-model info is a little weird.
But it is specified in docs misc/xenstore-paths.pandoc

> > > +
> > > +    sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
> > > +    sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid;
> > > +    sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm;
> > > +    sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed;
> > > +    sdss->qmp_proxy_spawn.detached_cb = qmp_proxy_detached;
> > > +
> > > +    const int arraysize = 6;
> > > +    GCNEW_ARRAY(args, arraysize);
> > > +    args[nr++] = STUBDOM_QMP_PROXY_PATH;
> > > +    args[nr++] = GCSPRINTF("--state-path=%s", sdss->qmp_proxy_spawn.xspath);
> > > +    args[nr++] = GCSPRINTF("%u", dm_domid);
> > > +    args[nr++] = GCSPRINTF("%s/device-model/%u/qmp-vchan", dom_path, guest_domid);
> >
> > Thinking of OpenXT"s qmp-helper, this path isn't useful.  But it is
> > for vchan-socket-proxy, so qmp-helper could just change to ignore it.
>
> For vchan we could use also a port number (and then it will encode it
> into a xenstore path). This is in fact how we use libvchan in Qubes. I
> opted for explicit path only because of lack of idea for some meaningful
> port number ;) But I'm open for suggestions.
> I guess that would be useful for Argo version then.

The argo version hard codes the port number, so it's not a command
line argument.  The port number would need to get passed to the
stubdom or it would need to be standardized.

I think the arguments for vchan-socket-proxy make sense.  Since it's
the one that's submitted upstream, it makes sense to use them.

Put another way, do we want this to support alternate implementations
for a qmp proxy?  Should the arguments be generic for that case?
Since the existing arguments have enough information for either proxy,
I think it's fine to leave it as is.  While you could have a wrapper
generate all the above from just the domid & stub_domid, that's kinda
hacky.

Thanks,
Jason

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

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

end of thread, back to index

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-15  2:39 [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Marek Marczykowski-Górecki
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 01/16] Document ioemu MiniOS stubdomain protocol Marek Marczykowski-Górecki
2020-01-20 18:30   ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 02/16] Document ioemu Linux " Marek Marczykowski-Górecki
2020-01-20 18:54   ` Jason Andryuk
2020-01-21 21:08     ` Marek Marczykowski-Górecki
2020-01-22 14:04       ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 03/16] libxl: fix qemu-trad cmdline for no sdl/vnc case Marek Marczykowski-Górecki
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 04/16] libxl: Allow running qemu-xen in stubdomain Marek Marczykowski-Górecki
2020-01-20 18:56   ` Jason Andryuk
2020-01-21 21:12     ` Marek Marczykowski-Górecki
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options Marek Marczykowski-Górecki
2020-01-20 19:24   ` Jason Andryuk
2020-01-21 21:18     ` Marek Marczykowski-Górecki
2020-01-22 14:25       ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 06/16] libxl: write qemu arguments into separate xenstore keys Marek Marczykowski-Górecki
2020-01-20 19:36   ` Jason Andryuk
2020-01-21 21:19     ` Marek Marczykowski-Górecki
2020-01-22 14:39       ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 07/16] xl: add stubdomain related options to xl config parser Marek Marczykowski-Górecki
2020-01-20 19:41   ` Jason Andryuk
2020-01-21 21:22     ` Marek Marczykowski-Górecki
2020-01-22 14:39       ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 08/16] tools/libvchan: notify server when client is connected Marek Marczykowski-Górecki
2020-01-20 19:44   ` Jason Andryuk
2020-01-21 21:28     ` Marek Marczykowski-Górecki
2020-01-22 14:43       ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 09/16] libxl: add save/restore support for qemu-xen in stubdomain Marek Marczykowski-Górecki
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 10/16] tools: add missing libxenvchan cflags Marek Marczykowski-Górecki
2020-01-20 19:58   ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 11/16] tools: add simple vchan-socket-proxy Marek Marczykowski-Górecki
2020-01-15 11:02   ` Jan Beulich
2020-01-16 17:11     ` Marek Marczykowski-Górecki
2020-01-17  8:13       ` Jan Beulich
2020-01-17 18:44   ` Rich Persaud
2020-01-17 18:56     ` Marek Marczykowski-Górecki
2020-01-21 19:43   ` Jason Andryuk
2020-01-21 23:09     ` Marek Marczykowski-Górecki
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 12/16] libxl: use vchan for QMP access with Linux stubdomain Marek Marczykowski-Górecki
2020-01-21 20:17   ` Jason Andryuk
2020-01-21 23:46     ` Marek Marczykowski-Górecki
2020-01-24 14:05       ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 13/16] Regenerate autotools files Marek Marczykowski-Górecki
2020-01-15 21:57   ` Rich Persaud
2020-01-21 20:56     ` Marek Marczykowski-Górecki
2020-01-21 21:28       ` Rich Persaud
2020-01-22  8:57         ` Lars Kurth
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 14/16] libxl: require qemu in dom0 even if stubdomain is in use Marek Marczykowski-Górecki
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 15/16] libxl: ignore emulated IDE disks beyond the first 4 Marek Marczykowski-Górecki
2020-01-21 20:24   ` Jason Andryuk
2020-01-15  2:39 ` [Xen-devel] [PATCH v4 16/16] libxl: consider also qemu in stubdomain in libxl__dm_active check Marek Marczykowski-Górecki
2020-01-21 20:25   ` Jason Andryuk
2020-01-22 16:50 ` [Xen-devel] [PATCH v4 00/16] Add support for qemu-xen runnning in a Linux-based stubdomain Jason Andryuk

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git