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

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

[Marek Marczykowski-Górecki]

General idea is to allow freely set device_model_version and
device_model_stubdomain_override and choose the right options based on this choice.
Also, allow to specific path to stubdomain kernel/ramdisk, for greater flexibility.
Right now when qemu-xen in stubdomain is selected, it is assumed it's
Linux-based and few decisions are based on it, specifically:
 - qemu args encoding (\x1b as separator, to allow spaces in arguments)
 - save/restore stream access (specific FDs passed to qemu by a wrapper script)
 - QMP access, if any

It may be a good idea to document "stubdomain API" somewhere. Is there such
document for MiniOS one? Here is an initial version for Linux one:

    Assumptions about Linux-based stubdomain for qemu-xen:
    * qemu command line is stored by toolstack in
      /vm/<target-uuid>/image/dmargs xenstore entry, arguments are separated
      with \x1b character
    * qemu can access saved state (if any) at its FD 3
    * qemu can write its state (for save/migration) to its FD 4
    * qemu expects backend for serial console at /dev/hvc3
    * disks configured for the target are available as /dev/xvd*, in
      configuration order
    * qemu can call /etc/qemu-ifup and /etc/qemu-ifdown to connect/disconnect
      network interface to appropriate network

Initial version has no QMP support - in initial patches it is completely
disabled, which means no suspend/restore. QMP normally would be used for PCI
passthrough setup, but it is worked around with MiniOS-like control protocol
over xenstore, which then is translated to QMP on stubdomain side.
Some option is to use separate console for that, but that require
xenstoled supporting multiple consoles per domain (the goal is to not have qemu
in dom0 at all). Also, it would be preferable to evaluate how libxl
handle potentially malicious QMP responses.
Another option is to use xenstore - either translate everything needed to
MiniOS-like thing, or write already json-formatted requests to xenstore and
watch there for responses. When using separate xenstore dir for that, with
per-command entries (command id as a key name?) that would solve concurrency
problem.

QMP support over separate console: patch "libxl: access QMP socket via console
for qemu-in-stubdomain" implements some early PoC of this.
Major limitation: only one connection at a time and no means of out of
band reset (and re-negotiate). I've tried adding very simple `qmp_reset`
command for in-band connection reset, but it is unreliable because of the
first limitation - xl process running in background hold this connection open
and every other xl instance interfere with it. On the other hand, for libvirt
use case one connection could be enough (leaving alone libvirtd restart).

Xenconsoled patches add support for multiple consoles in xenconsoled, to avoid the need
for qemu in dom0 for this to work. Multiple consoles for a stubdomain are used for:
 - logs (console 0)
 - save + restore (console 1, 2)
 - qmp (console 3)
 - serial terminal to target domU (console 4)
Xenconsoled patches are in fact a separate series, but I'm sending them here to
ease dependencies handling (latter libxl patches use that).

What qmp-libxenstat socket is for?

PCI passthrough support require some more love. Right now, libxl tries to setup
pcifront for both target HVM and stubdomain and the former fails (race
condition):
    xen-pciback pci-259-0: 22 Couldn't locate PCI device (0000:00:1b.0)! perhaps already in-use?
Fortunately it isn't needed. There is also a PCI related problem on
domain shutdown - it looks like first stubdomain is paused and then libxl waits
for pcifront there.
Also, MSI doesn't work (qemu output):

    [00:05.0] xen_pt_msgctrl_reg_write: setup MSI (register: 81).
    [00:05.0] msi_msix_setup: requested pirq 55 for MSI (vec: 0, entry: 0)
    [00:05.0] msi_msix_setup: Error: Mapping of MSI (err: 1, vec: 0, entry 0)
    [00:05.0] xen_pt_msgctrl_reg_write: Warning: Can not map MSI (register: 80)!
    [00:05.0] Write-back to unknown field 0x44 (partially) inhibited (0x00)
    [00:05.0] If the device doesn't work, try enabling permissive mode
    [00:05.0] (unsafe) and if it helps report the problem to xen-devel

The actual stubdomain implementation is here:

    https://github.com/marmarek/qubes-vmm-xen-stubdom-linux (branch for-upstream)

See readme there for build instructions.

Remaining parts for eliminating dom0's instance of qemu:
 - do not force QDISK backend for CDROM

This patch series is third (fourth?) attempt to get rid of limitation
"if you want to use stubdomain, you're stuck with qemu-traditional", done over
years, by many people.
I think it should be acceptable plan to gradually add features to
qemu-xen+stubdomain configuration - not necessary waiting with committing any
of those patches until full feature set of qemu-xen is supported. After all,
right now "feature set supported by qemu-xen+stubdom" is empty, so wouldn't be
worse.

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 (3):
  libxl: Add "stubdomain_version" to domain_build_info.
  libxl: Handle Linux stubdomain specific QEMU options.
  libxl: Build the domain with a Linux based stubdomain

Marek Marczykowski-Górecki (13):
  libxl: fix qemu-trad cmdline for no sdl/vnc case
  libxl: create vkb device only for guests with graphics output
  libxl: add save/restore support for qemu-xen in stubdomain
  xl: add stubdomain related options to xl config parser
  libxl: use \x1b to separate qemu arguments for linux stubdomain
  xenconsoled: install xenstore watch for all supported consoles
  xenconsoled: add support for consoles using 'state' xenstore entry
  xenconsoled: make console_type->use_gnttab less confusing
  xenconsoled: add support for up to 3 secondary consoles
  xenconsoled: deduplicate error handling
  xenconsoled: add support for non-pty output
  libxl: access QMP socket via console for qemu-in-stubdomain
  libxl: use xenconsoled even for multiple stubdomain's consoles

Simon Gaiser (1):
  libxl: use xenstore for pci hotplug qemu-in-linux-stubdom commands
  (and part of other commits, as noted in commit messages there)

 docs/man/xl.cfg.pod.5.in        |  23 ++-
 tools/console/daemon/io.c       | 222 ++++++++++++++++++++++++++++-----
 tools/libxl/libxl_create.c      |  83 ++++++++++--
 tools/libxl/libxl_dm.c          | 182 ++++++++++++++++++---------
 tools/libxl/libxl_dom_suspend.c |  10 +-
 tools/libxl/libxl_internal.c    |  22 +++-
 tools/libxl/libxl_internal.h    |   9 +-
 tools/libxl/libxl_mem.c         |   6 +-
 tools/libxl/libxl_pci.c         |  22 ++-
 tools/libxl/libxl_qmp.c         |  52 +++++++-
 tools/libxl/libxl_types.idl     |  10 +-
 tools/xl/xl_parse.c             |   7 +-
 12 files changed, 537 insertions(+), 111 deletions(-)

base-commit: e3f667bc5f51d0aa44357a64ca134cd952679c81
-- 
git-series 0.9.1

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

[RFC PATCH 01/17] libxl: fix qemu-trad cmdline for no sdl/vnc case

[Marek Marczykowski-Górecki]

When qemu is running in stubdomain, any attempt to initialize vnc/sdl
there will crash it (on failed attempt to load a keymap from a file). If
vfb is present, all those cases are skipped. But since
b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do not start dom0 qemu
for stubdomain when not needed" it is possible to create a stubdomain
without vfb and contrary to the comment -vnc none do trigger VNC
initialization code (just skips exposing it externally).
Change the implicit SDL avoiding method to -nographics option, used when
none of SDL or VNC is enabled.
---
 tools/libxl/libxl_dm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index bd18746..3b0b58e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -475,14 +475,14 @@ 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, "-nographics");
 
     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

Re: [RFC PATCH 01/17] libxl: fix qemu-trad cmdline for no sdl/vnc case

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> 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.

Missing Signed-off-by

> ---
>  tools/libxl/libxl_dm.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index bd18746..3b0b58e 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -475,14 +475,14 @@ 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, "-nographics");

The qemu option is "-nographic" without the 's'.

>
>      if (sdl) {
>          flexarray_append(dm_args, "-sdl");

With the above change:
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

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

[RFC PATCH 02/17] libxl: Add "stubdomain_version" to domain_build_info.

[Marek Marczykowski-Górecki]

From: Eric Shelton <eshelton@pobox.com>

This enum gives the ability to select between a MiniOS-based QEMU
traditional stub domain and a Linux-based QEMU upstream stub domain.  To
use the Linux-based stubdomain, the following two lines should be
included in the appropriate xl.cfg file:

device_model_version="qemu-xen"
device_model_stubdomain_override=1

To use the MiniOS-based stubdomain, the following is used instead:

device_model_version="qemu-xen-traditional"
device_model_stubdomain_override=1

Signed-off-by: Eric Shelton <eshelton@pobox.com>
---
 tools/libxl/libxl_create.c  | 34 +++++++++++++++++++++++++++++-----
 tools/libxl/libxl_types.idl |  7 +++++++
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 7d46824..9f5e6b7 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -159,12 +159,36 @@ 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->stubdomain_version) {
+            switch (b_info->device_model_version) {
+            case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+                b_info->stubdomain_version = LIBXL_STUBDOMAIN_VERSION_MINIOS;
+                break;
+            case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+                b_info->stubdomain_version = LIBXL_STUBDOMAIN_VERSION_LINUX;
+                break;
+            default: abort();
+            }
+        }
+
+        switch (b_info->device_model_version) {
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+            if (b_info->stubdomain_version != LIBXL_STUBDOMAIN_VERSION_MINIOS) {
+                LOG(ERROR,
+                    "\"qemu-xen-traditional\" require \"minios\" as stubdomain");
+                return ERROR_INVAL;
+            }
+            break;
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+            if (b_info->stubdomain_version != LIBXL_STUBDOMAIN_VERSION_LINUX) {
+                LOG(ERROR,
+                    "\"qemu-xen\" require \"linux\" as stubdomain");
+                return ERROR_INVAL;
+            }
+            break;
+        default: abort();
+        }
     }
 
     if (!b_info->max_vcpus)
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f1e50a0..946c5b5 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -101,6 +101,12 @@ libxl_device_model_version = Enumeration("device_model_version", [
     (2, "QEMU_XEN"),             # Upstream based qemu-xen device model
     ])
 
+# Give the kernel running in the stub-domain
+libxl_stubdomain_version = Enumeration("stubdomain_version", [
+    (1, "MINIOS"),
+    (2, "LINUX"),
+    ])
+
 libxl_console_type = Enumeration("console_type", [
     (0, "UNKNOWN"),
     (1, "SERIAL"),
@@ -485,6 +491,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     
     ("device_model_version", libxl_device_model_version),
     ("device_model_stubdomain", libxl_defbool),
+    ("stubdomain_version", libxl_stubdomain_version),
     # 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

Re: [RFC PATCH 02/17] libxl: Add "stubdomain_version" to domain_build_info.

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> From: Eric Shelton <eshelton@pobox.com>
>
> This enum gives the ability to select between a MiniOS-based QEMU
> traditional stub domain and a Linux-based QEMU upstream stub domain.  To
> use the Linux-based stubdomain, the following two lines should be
> included in the appropriate xl.cfg file:
>
> device_model_version="qemu-xen"
> device_model_stubdomain_override=1
>
> To use the MiniOS-based stubdomain, the following is used instead:
>
> device_model_version="qemu-xen-traditional"
> device_model_stubdomain_override=1
>
> Signed-off-by: Eric Shelton <eshelton@pobox.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

[RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

[Marek Marczykowski-Górecki]

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
---
 tools/libxl/libxl_dm.c | 106 ++++++++++++++++++++++++++++--------------
 1 file changed, 72 insertions(+), 34 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 3b0b58e..b38c1d2 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -25,9 +25,24 @@
 #include <pwd.h>
 #include <grp.h>
 
-static const char *libxl_tapif_script(libxl__gc *gc)
+static const char *libxl_tapif_script(libxl__gc *gc,
+                                      const libxl_domain_build_info *info)
 {
 #if defined(__linux__) || defined(__FreeBSD__)
+    if (info->stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX)
+        return libxl__sprintf(gc, "/etc/qemu-ifup");
+    return libxl__strdup(gc, "no");
+#else
+    return GCSPRINTF("%s/qemu-ifup", libxl__xen_script_dir_path());
+#endif
+}
+
+static const char *libxl_tapif_downscript(libxl__gc *gc,
+                                          const libxl_domain_build_info *info)
+{
+#if defined(__linux__) || defined(__FreeBSD__)
+    if (info->stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX)
+        return libxl__sprintf(gc, "/etc/qemu-ifdown");
     return libxl__strdup(gc, "no");
 #else
     return GCSPRINTF("%s/qemu-ifup", libxl__xen_script_dir_path());
@@ -616,8 +631,8 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
                                       "tap,vlan=%d,ifname=%s,bridge=%s,"
                                       "script=%s,downscript=%s",
                                       nics[i].devid, ifname, nics[i].bridge,
-                                      libxl_tapif_script(gc),
-                                      libxl_tapif_script(gc)),
+                                      libxl_tapif_script(gc, b_info),
+                                      libxl_tapif_downscript(gc, b_info)),
                                   NULL);
                 ioemu_nics++;
             }
@@ -933,6 +948,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
     const char *path, *chardev;
     char *user = NULL;
     struct passwd *user_base, user_pwbuf;
+    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);
@@ -943,24 +959,27 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                       "-xen-domid",
                       GCSPRINTF("%d", guest_domid), NULL);
 
-    flexarray_append(dm_args, "-chardev");
-    flexarray_append(dm_args,
-                     GCSPRINTF("socket,id=libxl-cmd,"
-                                    "path=%s/qmp-libxl-%d,server,nowait",
-                                    libxl__run_dir_path(), guest_domid));
+    /* There is currently no way to access the QMP socket in the stubdom */
+    if (!is_stubdom) {
+        flexarray_append(dm_args, "-chardev");
+        flexarray_append(dm_args,
+                         GCSPRINTF("socket,id=libxl-cmd,"
+                                        "path=%s/qmp-libxl-%d,server,nowait",
+                                        libxl__run_dir_path(), 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;
@@ -1004,7 +1023,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");
@@ -1043,7 +1062,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.
          */
@@ -1055,7 +1074,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);
@@ -1099,10 +1118,21 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                 return ERROR_INVAL;
             }
             if (b_info->u.hvm.serial) {
-                flexarray_vappend(dm_args,
-                                  "-serial", b_info->u.hvm.serial, NULL);
+                if (is_stubdom) {
+                    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;
+                if (is_stubdom) {
+                    flexarray_vappend(dm_args,
+                                      "-serial", "/dev/hvc1", NULL);
+                }
                 for (p = b_info->u.hvm.serial_list;
                      *p;
                      p++) {
@@ -1117,7 +1147,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)
@@ -1256,8 +1286,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                                  GCSPRINTF("type=tap,id=net%d,ifname=%s,"
                                            "script=%s,downscript=%s",
                                            nics[i].devid, ifname,
-                                           libxl_tapif_script(gc),
-                                           libxl_tapif_script(gc)));
+                                           libxl_tapif_script(gc, b_info),
+                                           libxl_tapif_downscript(gc, b_info)));
 
                 /* Userspace COLO Proxy need this */
 #define APPEND_COLO_SOCK_SERVER(sock_id, sock_ip, sock_port) ({             \
@@ -1503,7 +1533,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);
@@ -1514,6 +1546,8 @@ 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)) {
+                target_path = GCSPRINTF("/dev/xvd%c", 'a' + disk);
             } else {
                 if (format == NULL) {
                     LOGD(WARN, guest_domid,
@@ -1550,8 +1584,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
 
             if (disks[i].is_cdrom) {
                 drive = libxl__sprintf(gc,
-                         "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
-                         disk, dev_number);
+                         "if=ide,readonly=on,media=cdrom,id=ide-%i",
+                         dev_number);
 
                 if (target_path)
                     drive = libxl__sprintf(gc, "%s,file=%s,format=%s",
@@ -1727,7 +1761,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) {
@@ -1737,8 +1771,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,
@@ -1795,7 +1831,7 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
 
 static int libxl__write_stub_dmargs(libxl__gc *gc,
                                     int dm_domid, int guest_domid,
-                                    char **args)
+                                    char **args, bool linux_stubdom)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int i;
@@ -1823,7 +1859,9 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
     i = 1;
     dmargs[0] = '\0';
     while (args[i] != NULL) {
-        if (strcmp(args[i], "-sdl") && strcmp(args[i], "-M") && strcmp(args[i], "xenfv")) {
+        if (linux_stubdom ||
+            (strcmp(args[i], "-sdl") &&
+             strcmp(args[i], "-M") && strcmp(args[i], "xenfv"))) {
             strcat(dmargs, " ");
             strcat(dmargs, args[i]);
         }
-- 
git-series 0.9.1

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

Re: [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 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
> ---
<snip>
> @@ -1099,10 +1118,21 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
>                  return ERROR_INVAL;
>              }
>              if (b_info->u.hvm.serial) {
> -                flexarray_vappend(dm_args,
> -                                  "-serial", b_info->u.hvm.serial, NULL);
> +                if (is_stubdom) {
> +                    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;
> +                if (is_stubdom) {
> +                    flexarray_vappend(dm_args,
> +                                      "-serial", "/dev/hvc1", NULL);

Should this also be GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL)
instead of hardcoding hvc1?

> +                }
>                  for (p = b_info->u.hvm.serial_list;
>                       *p;
>                       p++) {

<snip>

> @@ -1550,8 +1584,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
>
>              if (disks[i].is_cdrom) {
>                  drive = libxl__sprintf(gc,
> -                         "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> -                         disk, dev_number);
> +                         "if=ide,readonly=on,media=cdrom,id=ide-%i",
> +                         dev_number);

What is the impact of dropping index?

>                  if (target_path)
>                      drive = libxl__sprintf(gc, "%s,file=%s,format=%s",

Regards,
Jason

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

Re: [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

[Marek Marczykowski-Górecki]

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

On Wed, Aug 01, 2018 at 10:25:22AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 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
> > ---
> <snip>
> > @@ -1099,10 +1118,21 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
> >                  return ERROR_INVAL;
> >              }
> >              if (b_info->u.hvm.serial) {
> > -                flexarray_vappend(dm_args,
> > -                                  "-serial", b_info->u.hvm.serial, NULL);
> > +                if (is_stubdom) {
> > +                    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;
> > +                if (is_stubdom) {
> > +                    flexarray_vappend(dm_args,
> > +                                      "-serial", "/dev/hvc1", NULL);
> 
> Should this also be GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL)
> instead of hardcoding hvc1?

Yes. Anyway,  multiple serial consoles are incompatible with
stubdomain anyway - should it error out if serial_list have multiple
elements? Or silently ignore others?

> > +                }
> >                  for (p = b_info->u.hvm.serial_list;
> >                       *p;
> >                       p++) {
> 
> <snip>
> 
> > @@ -1550,8 +1584,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
> >
> >              if (disks[i].is_cdrom) {
> >                  drive = libxl__sprintf(gc,
> > -                         "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> > -                         disk, dev_number);
> > +                         "if=ide,readonly=on,media=cdrom,id=ide-%i",
> > +                         dev_number);
> 
> What is the impact of dropping index?

Leftover from Qubes-specific part of this patch (non-cdrom disks are
converted to SCSI for readonly support, dropping index avoids hitting
IDE limitation of 4 disks). I'll remove this chunk.

> >                  if (target_path)
> >                      drive = libxl__sprintf(gc, "%s,file=%s,format=%s",
> 
> 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

Re: [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

[Jason Andryuk]

On Wed, Aug 1, 2018 at 11:55 AM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Wed, Aug 01, 2018 at 10:25:22AM -0400, Jason Andryuk wrote:
> > On Mon, Jul 30, 2018 at 11:56 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
> > > ---
> > <snip>
> > > @@ -1099,10 +1118,21 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
> > >                  return ERROR_INVAL;
> > >              }
> > >              if (b_info->u.hvm.serial) {
> > > -                flexarray_vappend(dm_args,
> > > -                                  "-serial", b_info->u.hvm.serial, NULL);
> > > +                if (is_stubdom) {
> > > +                    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;
> > > +                if (is_stubdom) {
> > > +                    flexarray_vappend(dm_args,
> > > +                                      "-serial", "/dev/hvc1", NULL);
> >
> > Should this also be GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL)
> > instead of hardcoding hvc1?
>
> Yes. Anyway,  multiple serial consoles are incompatible with
> stubdomain anyway - should it error out if serial_list have multiple
> elements? Or silently ignore others?

I think error-ing out with a suitable message for multiple elements
would avoid confusion.  Silently ignoring would leave users wondering
why the extra consoles didn't show up.

Multiple serials could be supported by dynamically allocating more
consoles to stubdom, but it's probably not worth the effort.

> > > +                }
> > >                  for (p = b_info->u.hvm.serial_list;
> > >                       *p;
> > >                       p++) {
> >
> > <snip>
> >
> > > @@ -1550,8 +1584,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
> > >
> > >              if (disks[i].is_cdrom) {
> > >                  drive = libxl__sprintf(gc,
> > > -                         "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> > > -                         disk, dev_number);
> > > +                         "if=ide,readonly=on,media=cdrom,id=ide-%i",
> > > +                         dev_number);
> >
> > What is the impact of dropping index?
>
> Leftover from Qubes-specific part of this patch (non-cdrom disks are
> converted to SCSI for readonly support, dropping index avoids hitting
> IDE limitation of 4 disks). I'll remove this chunk.

Ah, okay.

Regards,
Jason

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

[RFC PATCH 04/17] libxl: Build the domain with a Linux based stubdomain

[Marek Marczykowski-Górecki]

From: Eric Shelton <eshelton@pobox.com>

This will build a Linux-based stubdomain with QEMU upstream.

Signed-off-by: Eric Shelton <eshelton@pobox.com>

Simon:
 * use initramfs instead of disk with rootfs
 * don't initialize qmp (unused in Qubes)
 * Make libxl_domain_need_memory consistent with actual stubdoma build
   code (bugfix relevant also for non-linux case)
 * Make stubdomain memory size configurable
Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>

Marek:
 * Make kernel and ramdisk paths configurable.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libxl/libxl_create.c   | 75 +++++++++++++++++++++++++++----------
 tools/libxl/libxl_dm.c       | 48 ++++++++++++++++++------
 tools/libxl/libxl_internal.c | 22 +++++++++++-
 tools/libxl/libxl_internal.h |  4 ++-
 tools/libxl/libxl_mem.c      |  6 ++-
 tools/libxl/libxl_types.idl  |  3 +-
 6 files changed, 126 insertions(+), 32 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9f5e6b7..fff1f9c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -162,32 +162,54 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         libxl_defbool_val(b_info->device_model_stubdomain)) {
         if (!b_info->stubdomain_version) {
             switch (b_info->device_model_version) {
+                case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+                    b_info->stubdomain_version = LIBXL_STUBDOMAIN_VERSION_MINIOS;
+                    break;
+                case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+                    b_info->stubdomain_version = LIBXL_STUBDOMAIN_VERSION_LINUX;
+                    break;
+                default: abort();
+            }
+        }
+
+        switch (b_info->device_model_version) {
             case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-                b_info->stubdomain_version = LIBXL_STUBDOMAIN_VERSION_MINIOS;
+                if (b_info->stubdomain_version != LIBXL_STUBDOMAIN_VERSION_MINIOS) {
+                    LOG(ERROR,
+                            "\"qemu-xen-traditional\" require \"minios\" as stubdomain");
+                    return ERROR_INVAL;
+                }
                 break;
             case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-                b_info->stubdomain_version = LIBXL_STUBDOMAIN_VERSION_LINUX;
+                if (b_info->stubdomain_version != LIBXL_STUBDOMAIN_VERSION_LINUX) {
+                    LOG(ERROR,
+                            "\"qemu-xen\" require \"linux\" as stubdomain");
+                    return ERROR_INVAL;
+                }
                 break;
             default: abort();
-            }
         }
 
-        switch (b_info->device_model_version) {
-        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-            if (b_info->stubdomain_version != LIBXL_STUBDOMAIN_VERSION_MINIOS) {
-                LOG(ERROR,
-                    "\"qemu-xen-traditional\" require \"minios\" as stubdomain");
-                return ERROR_INVAL;
+        if (!b_info->stubdomain_kernel) {
+            switch (b_info->stubdomain_version) {
+                case LIBXL_STUBDOMAIN_VERSION_MINIOS:
+                    b_info->stubdomain_kernel =
+                        libxl__abs_path(NOGC, "ioemu-stubdom.gz", libxl__xenfirmwaredir_path());
+                    b_info->stubdomain_ramdisk = NULL;
+                    break;
+                case LIBXL_STUBDOMAIN_VERSION_LINUX:
+                    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();
             }
-            break;
-        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-            if (b_info->stubdomain_version != LIBXL_STUBDOMAIN_VERSION_LINUX) {
-                LOG(ERROR,
-                    "\"qemu-xen\" require \"linux\" as stubdomain");
-                return ERROR_INVAL;
-            }
-            break;
-        default: abort();
         }
     }
 
@@ -226,6 +248,19 @@ 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) {
+        switch (b_info->stubdomain_version) {
+            case LIBXL_STUBDOMAIN_VERSION_MINIOS:
+                b_info->stubdomain_memkb = 28 * 1024;
+                break;
+            case LIBXL_STUBDOMAIN_VERSION_LINUX:
+                b_info->stubdomain_memkb = LIBXL_LINUX_STUBDOM_MEM * 1024;;
+                break;
+            default:
+                b_info->stubdomain_memkb = 0; // no stubdomain
+        }
+    }
+
     libxl_defbool_setdefault(&b_info->claim_mode, false);
 
     libxl_defbool_setdefault(&b_info->localtime, false);
@@ -1591,7 +1626,9 @@ static void domcreate_devmodel_started(libxl__egc *egc,
     if (dcs->sdss.dm.guest_domid) {
         if (d_config->b_info.device_model_version
             == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
-            libxl__qmp_initializations(gc, domid, d_config);
+            if (!libxl_defbool_val(d_config->b_info.device_model_stubdomain)) {
+                libxl__qmp_initializations(gc, domid, d_config);
+            }
         }
     }
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b38c1d2..7828444 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1880,6 +1880,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 spawn_stubdom_pvqemu_cb(libxl__egc *egc,
                                 libxl__dm_spawn_state *stubdom_dmss,
                                 int rc);
@@ -1917,10 +1927,14 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     libxl__domain_build_state *const d_state = sdss->dm.build_state;
     libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
 
-    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 (guest_config->b_info.stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX) {
+        if (d_state->saved_state) {
+            LOG(ERROR, "Save/Restore not supported yet with Linux Stubdom.");
+            ret = -1;
+            goto out;
+        }
     }
 
     sdss->pvqemu.guest_domid = 0;
@@ -1941,8 +1955,8 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     libxl_domain_build_info_init_type(&dm_config->b_info, LIBXL_DOMAIN_TYPE_PV);
 
     dm_config->b_info.max_vcpus = 1;
-    dm_config->b_info.max_memkb = 28 * 1024 +
-        guest_config->b_info.video_memkb;
+    dm_config->b_info.max_memkb = guest_config->b_info.stubdomain_memkb;
+    dm_config->b_info.max_memkb += guest_config->b_info.video_memkb;
     dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
 
     dm_config->b_info.max_grant_frames = guest_config->b_info.max_grant_frames;
@@ -1983,10 +1997,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,
@@ -2006,7 +2018,12 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
         goto out;
     }
 
-    libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
+    libxl__store_libxl_entry(gc, guest_domid, "dm-version",
+        libxl_device_model_version_to_string(dm_config->b_info.device_model_version));
+    libxl__store_libxl_entry(gc, dm_domid, "stubdom-version",
+        libxl_stubdomain_version_to_string(guest_config->b_info.stubdomain_version));
+    libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args,
+        guest_config->b_info.stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX);
     libxl__xs_printf(gc, XBT_NULL,
                      GCSPRINTF("%s/image/device-model-domid",
                                libxl__xs_get_dompath(gc, guest_domid)),
@@ -2015,6 +2032,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.stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX) {
+        /* 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",
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index f492dae..c6b7465 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -377,6 +377,28 @@ out:
     return rc;
 }
 
+int libxl__stubdomain_version_running(libxl__gc *gc, uint32_t domid)
+{
+    char *path = NULL;
+    char *stub_version = NULL;
+    libxl_stubdomain_version value;
+
+    path = libxl__xs_libxl_path(gc, domid);
+    path = libxl__sprintf(gc, "%s/stubdom-version", path);
+    stub_version = libxl__xs_read(gc, XBT_NULL, path);
+    if (!stub_version) {
+        return LIBXL_STUBDOMAIN_VERSION_MINIOS;
+    }
+
+    if (libxl_stubdomain_version_from_string(stub_version, &value) < 0) {
+        libxl_ctx *ctx = libxl__gc_owner(gc);
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "fatal: %s contain a wrong value (%s)", path, stub_version);
+        return -1;
+    }
+    return value;
+}
+
 int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid)
 {
     char *path = NULL;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 52e1e8e..9a01aa3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -117,6 +117,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
@@ -2041,6 +2042,9 @@ _hidden libxl__json_object *libxl__json_parse(libxl__gc *gc_opt, const char *s);
 _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);
+  /* Based on /libxl/$domid/stubdom-version xenstore key
+   * default is minios */
+_hidden int libxl__stubdomain_version_running(libxl__gc *gc, uint32_t domid);
 
 #define DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, fmt, _a...)              \
     libxl__sprintf(gc, "/local/domain/%u/device-model/%u" fmt, dm_domid,   \
diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
index e551e09..4cf6a73 100644
--- a/tools/libxl/libxl_mem.c
+++ b/tools/libxl/libxl_mem.c
@@ -463,8 +463,10 @@ int libxl_domain_need_memory(libxl_ctx *ctx,
     case LIBXL_DOMAIN_TYPE_PVH:
     case LIBXL_DOMAIN_TYPE_HVM:
         *need_memkb += b_info->shadow_memkb + LIBXL_HVM_EXTRA_MEMORY;
-        if (libxl_defbool_val(b_info->device_model_stubdomain))
-            *need_memkb += 32 * 1024;
+        if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+            *need_memkb += b_info->stubdomain_memkb;
+            *need_memkb += b_info->video_memkb;
+        }
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         *need_memkb += b_info->shadow_memkb + LIBXL_PV_EXTRA_MEMORY;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 946c5b5..f739754 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -492,6 +492,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("device_model_version", libxl_device_model_version),
     ("device_model_stubdomain", libxl_defbool),
     ("stubdomain_version", libxl_stubdomain_version),
+    ("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

Re: [RFC PATCH 04/17] libxl: Build the domain with a Linux based stubdomain

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> From: Eric Shelton <eshelton@pobox.com>
>
> This will build a Linux-based stubdomain with QEMU upstream.
>
> Signed-off-by: Eric Shelton <eshelton@pobox.com>
>
> Simon:
>  * use initramfs instead of disk with rootfs
>  * don't initialize qmp (unused in Qubes)
>  * Make libxl_domain_need_memory consistent with actual stubdoma build
>    code (bugfix relevant also for non-linux case)
>  * Make stubdomain memory size configurable
> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
>
> Marek:
>  * Make kernel and ramdisk paths configurable.
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---

<snip>

> @@ -1983,10 +1997,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);

This dropped line isn't restored anywhere else, afaics.  Does this
break mini-os stubdoms?

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

Regards,
Jason

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

Re: [RFC PATCH 04/17] libxl: Build the domain with a Linux based stubdomain

[Marek Marczykowski-Górecki]

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

On Wed, Aug 01, 2018 at 10:26:06AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> > From: Eric Shelton <eshelton@pobox.com>
> >
> > This will build a Linux-based stubdomain with QEMU upstream.
> >
> > Signed-off-by: Eric Shelton <eshelton@pobox.com>
> >
> > Simon:
> >  * use initramfs instead of disk with rootfs
> >  * don't initialize qmp (unused in Qubes)
> >  * Make libxl_domain_need_memory consistent with actual stubdoma build
> >    code (bugfix relevant also for non-linux case)
> >  * Make stubdomain memory size configurable
> > Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
> >
> > Marek:
> >  * Make kernel and ramdisk paths configurable.
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> 
> <snip>
> 
> > @@ -1983,10 +1997,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);
> 
> This dropped line isn't restored anywhere else, afaics.  Does this
> break mini-os stubdoms?

No, mini-os gets cmdline from xenstore anyway. And
libxl__build_device_model_args_old do add -d domid there.

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

Re: [RFC PATCH 04/17] libxl: Build the domain with a Linux based stubdomain

[Jason Andryuk]

On Wed, Aug 1, 2018 at 10:37 AM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> On Wed, Aug 01, 2018 at 10:26:06AM -0400, Jason Andryuk wrote:
>> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
>> <marmarek@invisiblethingslab.com> wrote:
>> > From: Eric Shelton <eshelton@pobox.com>
>> >
>> > This will build a Linux-based stubdomain with QEMU upstream.
>> >
>> > Signed-off-by: Eric Shelton <eshelton@pobox.com>
>> >
>> > Simon:
>> >  * use initramfs instead of disk with rootfs
>> >  * don't initialize qmp (unused in Qubes)
>> >  * Make libxl_domain_need_memory consistent with actual stubdoma build
>> >    code (bugfix relevant also for non-linux case)
>> >  * Make stubdomain memory size configurable
>> > Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
>> >
>> > Marek:
>> >  * Make kernel and ramdisk paths configurable.
>> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>> > ---
>>
>> <snip>
>>
>> > @@ -1983,10 +1997,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);
>>
>> This dropped line isn't restored anywhere else, afaics.  Does this
>> break mini-os stubdoms?
>
> No, mini-os gets cmdline from xenstore anyway. And
> libxl__build_device_model_args_old do add -d domid there.

Ah, okay.  Thanks.

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

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

[RFC PATCH 05/17] libxl: use xenstore for pci hotplug qemu-in-linux-stubdom commands

[Marek Marczykowski-Górecki]

From: Simon Gaiser <simon@invisiblethingslab.com>

There is no QMP socket access, re-use the same mechanism as for MiniOS
based stubdom.
---
 tools/libxl/libxl_pci.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 4755a0c..311fad4 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -995,6 +995,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
     uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED;
     uint32_t domainid = domid;
     bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid);
+    uint32_t dm_domid;
 
     if (type == LIBXL_DOMAIN_TYPE_INVALID)
         return ERROR_FAIL;
@@ -1010,7 +1011,15 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
                 rc = qemu_pci_add_xenstore(gc, domid, pcidev);
                 break;
             case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-                rc = libxl__qmp_pci_add(gc, domid, pcidev);
+                dm_domid = libxl_get_stubdom_id(ctx, domid);
+
+                if (dm_domid != 0
+                    && libxl__stubdomain_version_running(gc, dm_domid) ==
+                         LIBXL_STUBDOMAIN_VERSION_LINUX) {
+                    rc = qemu_pci_add_xenstore(gc, domid, pcidev);
+                } else {
+                    rc = libxl__qmp_pci_add(gc, domid, pcidev);
+                }
                 break;
             default:
                 return ERROR_INVAL;
@@ -1362,7 +1371,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
     libxl_device_pci *assigned;
     libxl_domain_type type = libxl__domain_type(gc, domid);
     int hvm = 0, rc, num;
-    int stubdomid = 0;
+    int stubdomid = libxl_get_stubdom_id(ctx, domid);
     uint32_t domainid = domid;
     bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid);
 
@@ -1390,7 +1399,13 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
             rc = qemu_pci_remove_xenstore(gc, domid, pcidev, force);
             break;
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-            rc = libxl__qmp_pci_del(gc, domid, pcidev);
+            if (stubdomid != 0
+                && libxl__stubdomain_version_running(gc, stubdomid) ==
+                     LIBXL_STUBDOMAIN_VERSION_LINUX) {
+                rc = qemu_pci_remove_xenstore(gc, domid, pcidev, force);
+            } else {
+                rc = libxl__qmp_pci_del(gc, domid, pcidev);
+            }
             break;
         default:
             rc = ERROR_INVAL;
@@ -1470,7 +1485,6 @@ out:
             LOGED(ERROR, domainid, "xc_deassign_device failed");
     }
 
-    stubdomid = libxl_get_stubdom_id(ctx, domid);
     if (stubdomid != 0) {
         libxl_device_pci pcidev_s = *pcidev;
         libxl__device_pci_remove_common(gc, stubdomid, &pcidev_s, force);
-- 
git-series 0.9.1

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

Re: [RFC PATCH 05/17] libxl: use xenstore for pci hotplug qemu-in-linux-stubdom commands

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> From: Simon Gaiser <simon@invisiblethingslab.com>
>
> There is no QMP socket access, re-use the same mechanism as for MiniOS
> based stubdom.

Later you add some QMP support.  Is this preferred because your QMP
support is unreliable?

<snip>

> @@ -1010,7 +1011,15 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
>                  rc = qemu_pci_add_xenstore(gc, domid, pcidev);
>                  break;
>              case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> -                rc = libxl__qmp_pci_add(gc, domid, pcidev);
> +                dm_domid = libxl_get_stubdom_id(ctx, domid);
> +
> +                if (dm_domid != 0
> +                    && libxl__stubdomain_version_running(gc, dm_domid) ==
> +                         LIBXL_STUBDOMAIN_VERSION_LINUX) {
> +                    rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> +                } else {
> +                    rc = libxl__qmp_pci_add(gc, domid, pcidev);
> +                }

What about refactoring the whole switch statement to something like:
bool use_qmp = libxl__device_model_version_running(gc, domid)
                 == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN &&
               libxl_get_stubdom_id(ctx, domid) == 0;
if (use_qmp) {
   rc = libxl__qmp_pci_add(gc, domid, pcidev);
} else {
    rc = qemu_pci_add_xenstore(gc, domid, pcidev);
}

>                  break;
>              default:
>                  return ERROR_INVAL;

Regards,
Jason

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

Re: [RFC PATCH 05/17] libxl: use xenstore for pci hotplug qemu-in-linux-stubdom commands

[Marek Marczykowski-Górecki]

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

On Wed, Aug 01, 2018 at 10:29:07AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> > From: Simon Gaiser <simon@invisiblethingslab.com>
> >
> > There is no QMP socket access, re-use the same mechanism as for MiniOS
> > based stubdom.
> 
> Later you add some QMP support.  Is this preferred because your QMP
> support is unreliable?

For now, until final decision how to connect QMP is made, yes.
This is also somethings we already use Qubes for almost a year, so it
got much more testing.

But if we get stable QMP support, this patch could be dropped.

> <snip>
> 
> > @@ -1010,7 +1011,15 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
> >                  rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> >                  break;
> >              case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> > -                rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > +                dm_domid = libxl_get_stubdom_id(ctx, domid);
> > +
> > +                if (dm_domid != 0
> > +                    && libxl__stubdomain_version_running(gc, dm_domid) ==
> > +                         LIBXL_STUBDOMAIN_VERSION_LINUX) {
> > +                    rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> > +                } else {
> > +                    rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > +                }
> 
> What about refactoring the whole switch statement to something like:
> bool use_qmp = libxl__device_model_version_running(gc, domid)
>                  == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN &&
>                libxl_get_stubdom_id(ctx, domid) == 0;
> if (use_qmp) {
>    rc = libxl__qmp_pci_add(gc, domid, pcidev);
> } else {
>     rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> }

That indeed looks better. But I'd delay touching this patch until
getting some plan on QMP.
> 
> >                  break;
> >              default:
> >                  return ERROR_INVAL;
> 
> 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

Re: [RFC PATCH 05/17] libxl: use xenstore for pci hotplug qemu-in-linux-stubdom commands

[Jason Andryuk]

On Wed, Aug 1, 2018 at 12:03 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Wed, Aug 01, 2018 at 10:29:07AM -0400, Jason Andryuk wrote:
> > On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> wrote:
> > > From: Simon Gaiser <simon@invisiblethingslab.com>
> > >
> > > There is no QMP socket access, re-use the same mechanism as for MiniOS
> > > based stubdom.
> >
> > Later you add some QMP support.  Is this preferred because your QMP
> > support is unreliable?
>
> For now, until final decision how to connect QMP is made, yes.
> This is also somethings we already use Qubes for almost a year, so it
> got much more testing.
>
> But if we get stable QMP support, this patch could be dropped.
>
> > <snip>
> >
> > > @@ -1010,7 +1011,15 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
> > >                  rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> > >                  break;
> > >              case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> > > -                rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > > +                dm_domid = libxl_get_stubdom_id(ctx, domid);
> > > +
> > > +                if (dm_domid != 0
> > > +                    && libxl__stubdomain_version_running(gc, dm_domid) ==
> > > +                         LIBXL_STUBDOMAIN_VERSION_LINUX) {
> > > +                    rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> > > +                } else {
> > > +                    rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > > +                }
> >
> > What about refactoring the whole switch statement to something like:
> > bool use_qmp = libxl__device_model_version_running(gc, domid)
> >                  == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN &&
> >                libxl_get_stubdom_id(ctx, domid) == 0;
> > if (use_qmp) {
> >    rc = libxl__qmp_pci_add(gc, domid, pcidev);
> > } else {
> >     rc = qemu_pci_add_xenstore(gc, domid, pcidev);
> > }
>
> That indeed looks better. But I'd delay touching this patch until
> getting some plan on QMP.

Ok.  That sounds good.

Regards,
Jason

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

[RFC PATCH 06/17] libxl: create vkb device only for guests with graphics output

[Marek Marczykowski-Górecki]

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

One can still add vkb device manually if needed.

This is continuation of b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do
not start dom0 qemu for stubdomain when not needed".
---
 tools/libxl/libxl_create.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

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

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

Re: [RFC PATCH 06/17] libxl: create vkb device only for guests with graphics output

[Jason Andryuk]

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

Prior to this change, was a dom0 qemu started for these entries?

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

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

Re: [RFC PATCH 06/17] libxl: create vkb device only for guests with graphics output

[Marek Marczykowski-Górecki]

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

On Wed, Aug 01, 2018 at 10:30:14AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> > The forced vkb device is meant for better performance of qemu access
> > (at least according to ebbd2561b4cefb299f0f68a88b2788504223de18 "libxl:
> > Add a vkbd frontend/backend pair for HVM guests"), which isn't used if
> > there is no configured channel to actually access that keyboard.
> >
> > One can still add vkb device manually if needed.
> >
> > This is continuation of b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do
> > not start dom0 qemu for stubdomain when not needed".
> 
> Prior to this change, was a dom0 qemu started for these entries?

AFAIR not for this device alone (unless some other condition in
libxl__need_xenpv_qemu triggers it). But when it is created but backend
not launched, it cause timeout on frontend side.

> 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

[RFC PATCH 07/17] libxl: add save/restore support for qemu-xen in stubdomain

[Marek Marczykowski-Górecki]

Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
relevant consoles.
---
 tools/libxl/libxl_dm.c          | 23 +++++++++++------------
 tools/libxl/libxl_dom_suspend.c | 10 ++++++++--
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 7828444..f6bd6a1 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1444,10 +1444,17 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
         flexarray_append(dm_args, "-xen-domid-restrict");
 
     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]);
@@ -1929,14 +1936,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 (guest_config->b_info.stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX) {
-        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_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 1e904ba..8c8ae84 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -73,7 +73,8 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
 {
     int ret = 0;
     uint32_t const domid = dsps->domid;
-    const char *const filename = dsps->dm_savefile;
+    uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
+    const char * filename = dsps->dm_savefile;
 
     switch (libxl__device_model_version_running(gc, domid)) {
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL: {
@@ -86,8 +87,13 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
         if (libxl__qmp_stop(gc, domid))
             return ERROR_FAIL;
         /* Save DM state into filename */
+        if (dm_domid) {
+            /* if DM is in stubdomain, instruct it to use console, which is
+             * connected to a file pointed by filename */
+            filename = "/proc/self/fd/4";
+        }
         ret = libxl__qmp_save(gc, domid, filename, dsps->live);
-        if (ret)
+        if (ret && !dm_domid)
             unlink(filename);
         break;
     default:
-- 
git-series 0.9.1

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

Re: [RFC PATCH 07/17] libxl: add save/restore support for qemu-xen in stubdomain

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
> relevant consoles.

Should FD 3 & 4 be defined in some header?  That's not useful from a
wrapper shell script, but it should be documented somewhere.

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

Regards,
Jason

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

Re: [RFC PATCH 07/17] libxl: add save/restore support for qemu-xen in stubdomain

[Marek Marczykowski-Górecki]

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

On Wed, Aug 01, 2018 at 10:35:26AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> > Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
> > relevant consoles.
> 
> Should FD 3 & 4 be defined in some header?  That's not useful from a
> wrapper shell script, but it should be documented somewhere.

Maybe. See cover letter.

> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
> 
> 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

[RFC PATCH 08/17] xl: add stubdomain related options to xl config parser

[Marek Marczykowski-Górecki]

---
 docs/man/xl.cfg.pod.5.in | 23 +++++++++++++++++++----
 tools/xl/xl_parse.c      |  7 +++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 099a28d..9c8ff31 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -2636,10 +2636,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 f9147f6..c99c77b 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2391,6 +2391,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

Re: [RFC PATCH 08/17] xl: add stubdomain related options to xl config parser

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:

No Signed-off-by

> ---
>  docs/man/xl.cfg.pod.5.in | 23 +++++++++++++++++++----
>  tools/xl/xl_parse.c      |  7 +++++++
>  2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index 099a28d..9c8ff31 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -2636,10 +2636,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.

If provided to a mini-os stubdom, it would be loaded into memory but not used?

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

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

Re: [RFC PATCH 08/17] xl: add stubdomain related options to xl config parser

[Marek Marczykowski-Górecki]

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

On Wed, Aug 01, 2018 at 10:35:59AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> 
> No Signed-off-by

Sigh, does anyone know how to configure git-series (or git commit) to add
it automatically? I know how to do that for git format-patch, but it
seems git-series doesn't use it.

> > ---
> >  docs/man/xl.cfg.pod.5.in | 23 +++++++++++++++++++----
> >  tools/xl/xl_parse.c      |  7 +++++++
> >  2 files changed, 26 insertions(+), 4 deletions(-)
> >
> > diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> > index 099a28d..9c8ff31 100644
> > --- a/docs/man/xl.cfg.pod.5.in
> > +++ b/docs/man/xl.cfg.pod.5.in
> > @@ -2636,10 +2636,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.
> 
> If provided to a mini-os stubdom, it would be loaded into memory but not used?

Yes, exactly.

Technically it is possible to package stubdomain as just kernel binary
(with initramfs embedded), so this option wouldn't be needed at all.
But it's far less convenient for debugging and customization.

> 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

[RFC PATCH 09/17] libxl: use \x1b to separate qemu arguments for linux stubdomain

[Marek Marczykowski-Górecki]

This allows using arguments with spaces, like -append.
Stubdomain side of this require "xenstore-client: Add option for raw
in-/output" commit.
---
 tools/libxl/libxl_dm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index f6bd6a1..11acbea 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1844,6 +1844,7 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
     int i;
     char *vm_path;
     char *dmargs, *path;
+    const char arg_sep = linux_stubdom ? '\x1b' : ' ';
     int dmargs_size;
     struct xs_permissions roperm[2];
     xs_transaction_t t;
@@ -1869,8 +1870,9 @@ static int libxl__write_stub_dmargs(libxl__gc *gc,
         if (linux_stubdom ||
             (strcmp(args[i], "-sdl") &&
              strcmp(args[i], "-M") && strcmp(args[i], "xenfv"))) {
-            strcat(dmargs, " ");
             strcat(dmargs, args[i]);
+            if (args[i + 1] != NULL)
+                strncat(dmargs, &arg_sep, 1);
         }
         i++;
     }
-- 
git-series 0.9.1

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

Re: [RFC PATCH 09/17] libxl: use \x1b to separate qemu arguments for linux stubdomain

[Jason Andryuk]

On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> This allows using arguments with spaces, like -append.
> Stubdomain side of this require "xenstore-client: Add option for raw
> in-/output" commit.

I had to look up \x1b - it is ascii escape.  Since I was on the
www.asciitable.com, I saw \x1e is "record seperator" which might make
more sense.

Either way, the code is fine.  It would be good to document the
separator and other interface assumptions like the FDs in a
linux-stubdom document.

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

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

Re: [RFC PATCH 09/17] libxl: use \x1b to separate qemu arguments for linux stubdomain

[Marek Marczykowski-Górecki]

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

On Wed, Aug 01, 2018 at 10:36:26AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> > This allows using arguments with spaces, like -append.
> > Stubdomain side of this require "xenstore-client: Add option for raw
> > in-/output" commit.
> 
> I had to look up \x1b - it is ascii escape.  Since I was on the
> www.asciitable.com, I saw \x1e is "record seperator" which might make
> more sense.

Simon, do you remember why you used \x1b? Otherwise indeed changing to
\x1e would make sense.

> Either way, the code is fine.  It would be good to document the
> separator and other interface assumptions like the FDs in a
> linux-stubdom document.

Yes, definitely, already proposed in cover letter, including initial
description of the interface.

> 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

Re: [RFC PATCH 09/17] libxl: use \x1b to separate qemu arguments for linux stubdomain

[Simon Gaiser]

[-- Attachment #1.1.1: Type: text/plain, Size: 909 bytes --]

Marek Marczykowski-Górecki:
> On Wed, Aug 01, 2018 at 10:36:26AM -0400, Jason Andryuk wrote:
>> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
>> <marmarek@invisiblethingslab.com> wrote:
>>> This allows using arguments with spaces, like -append.
>>> Stubdomain side of this require "xenstore-client: Add option for raw
>>> in-/output" commit.
>>
>> I had to look up \x1b - it is ascii escape.  Since I was on the
>> www.asciitable.com, I saw \x1e is "record seperator" which might make
>> more sense.
> 
> Simon, do you remember why you used \x1b? Otherwise indeed changing to
> \x1e would make sense.

As far as I recall it's a mostly arbitrary choice. I think I choose 0x1b
because I thought that ASCII escape would be very unlikely to ever
appear in an cmdline. 0x00 would be even better but you don't want to
handle 0x00 in bash. I'm not aware of a problem with 0x1e.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 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

Re: [RFC PATCH 09/17] libxl: use \x1b to separate qemu arguments for linux stubdomain

[Simon Gaiser]

[-- Attachment #1.1.1: Type: text/plain, Size: 1196 bytes --]

Simon Gaiser:
> Marek Marczykowski-Górecki:
>> On Wed, Aug 01, 2018 at 10:36:26AM -0400, Jason Andryuk wrote:
>>> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
>>> <marmarek@invisiblethingslab.com> wrote:
>>>> This allows using arguments with spaces, like -append.
>>>> Stubdomain side of this require "xenstore-client: Add option for raw
>>>> in-/output" commit.
>>>
>>> I had to look up \x1b - it is ascii escape.  Since I was on the
>>> www.asciitable.com, I saw \x1e is "record seperator" which might make
>>> more sense.
>>
>> Simon, do you remember why you used \x1b? Otherwise indeed changing to
>> \x1e would make sense.
> 
> As far as I recall it's a mostly arbitrary choice. I think I choose 0x1b
> because I thought that ASCII escape would be very unlikely to ever
> appear in an cmdline. 0x00 would be even better but you don't want to
> handle 0x00 in bash. I'm not aware of a problem with 0x1e.

PS: If you follow the Wikipedia description [1] of the "separator"
control characters 0x1f would fit even better. But I don't think it
really matters what rare character we use.

[1]: https://en.wikipedia.org/wiki/C0_and_C1_control_codes#FS


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 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

Re: [RFC PATCH 09/17] libxl: use \x1b to separate qemu arguments for linux stubdomain

[Jason Andryuk]

On Wed, Aug 1, 2018 at 9:54 PM Simon Gaiser
<simon@invisiblethingslab.com> wrote:
>
> Simon Gaiser:
> > Marek Marczykowski-Górecki:
> >> On Wed, Aug 01, 2018 at 10:36:26AM -0400, Jason Andryuk wrote:
> >>> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> >>> <marmarek@invisiblethingslab.com> wrote:
> >>>> This allows using arguments with spaces, like -append.
> >>>> Stubdomain side of this require "xenstore-client: Add option for raw
> >>>> in-/output" commit.
> >>>
> >>> I had to look up \x1b - it is ascii escape.  Since I was on the
> >>> www.asciitable.com, I saw \x1e is "record seperator" which might make
> >>> more sense.
> >>
> >> Simon, do you remember why you used \x1b? Otherwise indeed changing to
> >> \x1e would make sense.
> >
> > As far as I recall it's a mostly arbitrary choice. I think I choose 0x1b
> > because I thought that ASCII escape would be very unlikely to ever
> > appear in an cmdline. 0x00 would be even better but you don't want to
> > handle 0x00 in bash. I'm not aware of a problem with 0x1e.
>
> PS: If you follow the Wikipedia description [1] of the "separator"
> control characters 0x1f would fit even better. But I don't think it
> really matters what rare character we use.

Agreed. The choice shouldn't matter as long as it's a non-printable ascii char.

> [1]: https://en.wikipedia.org/wiki/C0_and_C1_control_codes#FS

Regards,
Jason

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

[RFC PATCH 10/17] xenconsoled: install xenstore watch for all supported consoles

[Marek Marczykowski-Górecki]

Not only for the primary one (/local/domain/<domid>/console path).
---
 tools/console/daemon/io.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index e22009a..8aae87d 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -766,24 +766,24 @@ static int console_create_ring(struct console *con)
 	return err;
 }
 
-static bool watch_domain(struct domain *dom, bool watch)
+static int watch_domain(struct console *con, struct domain *dom, void **data)
 {
+	bool watch = data;
 	char domid_str[3 + MAX_STRLEN(dom->domid)];
 	bool success;
-	struct console *con = &dom->console[0];
 
 	snprintf(domid_str, sizeof(domid_str), "dom%u", dom->domid);
 	if (watch) {
 		success = xs_watch(xs, con->xspath, domid_str);
 		if (success)
-			console_iter_int_arg1(dom, console_create_ring);
+			console_create_ring(con);
 		else
 			xs_unwatch(xs, con->xspath, domid_str);
 	} else {
 		success = xs_unwatch(xs, con->xspath, domid_str);
 	}
 
-	return success;
+	return !success;
 }
 
 static int console_init(struct console *con, struct domain *dom, void **data)
@@ -853,7 +853,7 @@ static struct domain *create_domain(int domid)
 	if (console_iter_int_arg3(dom, console_init, (void **)&con_type))
 		goto out;
 
-	if (!watch_domain(dom, true))
+	if (console_iter_int_arg3(dom, watch_domain, (void**)true))
 		goto out;
 
 	dom->next = dom_head;
@@ -927,7 +927,7 @@ static void console_close_evtchn(struct console *con)
 static void shutdown_domain(struct domain *d)
 {
 	d->is_dead = true;
-	watch_domain(d, false);
+	console_iter_int_arg3(d, watch_domain, (void**)false);
 	console_iter_void_arg1(d, console_unmap_interface);
 	console_iter_void_arg1(d, console_close_evtchn);
 }
-- 
git-series 0.9.1

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

[RFC PATCH 11/17] xenconsoled: add support for consoles using 'state' xenstore entry

[Marek Marczykowski-Górecki]

Add support for standard xenbus initialization protocol using 'state'
xenstore entry. It will be necessary for secondary consoles.
For consoles supporting it, read 'state' entry on the frontend and
proceed accordingly - either init console or close it. When closing,
make sure all the in-transit data is flushed (both from shared ring and
from local buffer), if possible. This is especially important for
MiniOS-based qemu stubdomain, which closes console just after writing
device model state to it.
For consoles without 'state' field behavior is unchanged - on any watch
event try to connect console, as long as domain is alive.
---
 tools/console/daemon/io.c | 86 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 83 insertions(+), 3 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 8aae87d..92c3dff 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -25,6 +25,7 @@
 #include <xengnttab.h>
 #include <xenstore.h>
 #include <xen/io/console.h>
+#include <xen/io/xenbus.h>
 #include <xen/grant_table.h>
 
 #include <stdlib.h>
@@ -109,6 +110,7 @@ struct console {
 	struct domain *d;
 	bool optional;
 	bool use_gnttab;
+	bool have_state;
 };
 
 struct console_type {
@@ -117,6 +119,7 @@ struct console_type {
 	char *log_suffix;
 	bool optional;
 	bool use_gnttab;
+	bool have_state;  // uses 'state' xenstore entry
 };
 
 static struct console_type console_type[] = {
@@ -156,6 +159,8 @@ typedef void (*VOID_ITER_FUNC_ARG2)(struct console *,  void *);
 typedef int (*INT_ITER_FUNC_ARG3)(struct console *,
 				  struct domain *dom, void **);
 
+static void handle_tty_write(struct console *con);
+
 static inline bool console_enabled(struct console *con)
 {
 	return con->local_port != -1;
@@ -647,6 +652,20 @@ static int xs_gather(struct xs_handle *xs, const char *dir, ...)
 	return ret;
 }
 
+static void set_backend_state(struct console *con, int state)
+{
+	char path[PATH_MAX], state_str[2], *be_path;
+
+	snprintf(state_str, sizeof(state_str), "%d", state);
+	snprintf(path, sizeof(path), "%s/backend", con->xspath);
+	be_path = xs_read(xs, XBT_NULL, path, NULL);
+	if (be_path) {
+		snprintf(path, sizeof(path), "%s/state", be_path);
+		xs_write(xs, XBT_NULL, path, state_str, 1);
+		free(be_path);
+	}
+}
+
 static void console_unmap_interface(struct console *con)
 {
 	if (con->interface == NULL)
@@ -658,7 +677,7 @@ static void console_unmap_interface(struct console *con)
 	con->interface = NULL;
 	con->ring_ref = -1;
 }
- 
+
 static int console_create_ring(struct console *con)
 {
 	int err, remote_port, ring_ref, rc;
@@ -762,10 +781,70 @@ static int console_create_ring(struct console *con)
 	if (log_guest && (con->log_fd == -1))
 		con->log_fd = create_console_log(con);
 
+	/* if everything ok, signal backend readiness, in backend tree */
+	set_backend_state(con, XenbusStateConnected);
+
  out:
 	return err;
 }
 
+/* gracefully close console */
+static int console_close(struct console *con) {
+
+	if (con->interface && con->master_fd != -1 && con->buffer.data) {
+		/* handle remaining data in buffers */
+		buffer_append(con);
+
+		/* write it out, if possible */
+		if (con->master_pollfd_idx != -1) {
+			if (fds[con->master_pollfd_idx].revents &
+					POLLOUT)
+				handle_tty_write(con);
+		}
+	}
+
+	console_close_tty(con);
+	console_unmap_interface(con);
+	set_backend_state(con, XenbusStateClosed);
+
+	return 0;
+}
+
+
+static int handle_console_state(struct console *con) {
+	int err, state;
+
+	if (!con->have_state)
+		return console_create_ring(con);
+
+	err = xs_gather(xs, con->xspath,
+			"state", "%u", &state,
+			NULL);
+	if (err)
+		/* no 'state' entry, assume removal */
+		state = XenbusStateClosed;
+
+	switch (state) {
+		case XenbusStateInitialising:
+		case XenbusStateInitWait:
+			/* wait for frontent init */
+			return 0;
+		case XenbusStateInitialised:
+		case XenbusStateConnected:
+			/* ok, init backend (also on restart) */
+			return console_create_ring(con);
+		case XenbusStateClosing:
+		case XenbusStateClosed:
+			/* close requested */
+			return console_close(con);
+		default:
+			dolog(LOG_ERR,
+			      "Invalid state %d set by console %s of domain %d\n",
+			      state, con->xspath, con->d->domid);
+			return 1;
+	}
+}
+
 static int watch_domain(struct console *con, struct domain *dom, void **data)
 {
 	bool watch = data;
@@ -776,7 +855,7 @@ static int watch_domain(struct console *con, struct domain *dom, void **data)
 	if (watch) {
 		success = xs_watch(xs, con->xspath, domid_str);
 		if (success)
-			console_create_ring(con);
+			handle_console_state(con);
 		else
 			xs_unwatch(xs, con->xspath, domid_str);
 	} else {
@@ -814,6 +893,7 @@ static int console_init(struct console *con, struct domain *dom, void **data)
 	con->log_suffix = (*con_type)->log_suffix;
 	con->optional = (*con_type)->optional;
 	con->use_gnttab = (*con_type)->use_gnttab;
+	con->have_state = (*con_type)->have_state;
 	xsname = (char *)(*con_type)->xsname;
 	xspath = xs_get_domain_path(xs, dom->domid);
 	s = realloc(xspath, strlen(xspath) +
@@ -1124,7 +1204,7 @@ static void handle_xs(void)
 		/* We may get watches firing for domains that have recently
 		   been removed, so dom may be NULL here. */
 		if (dom && dom->is_dead == false)
-			console_iter_int_arg1(dom, console_create_ring);
+			console_iter_int_arg1(dom, handle_console_state);
 	}
 
 	free(vec);
-- 
git-series 0.9.1

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

[RFC PATCH 12/17] xenconsoled: make console_type->use_gnttab less confusing

[Marek Marczykowski-Górecki]

Before this commit 'use_gnttab' means xenconsoled should first try
special GNTTAB_RESERVED_CONSOLE entry, and only then fallback to
ring-ref xenstore entry (being gfn of actual ring).
In case of secondary consoles, ring-ref entry contains grant table
reference (not gfn of it), which makes the old meaning of use_gnttab
really confusing (should be false for such consoles).
To solve this, add new entry in console_type (and console) structures
named 'use_reserverd_gnttab' with the old meaning of 'use_gnttab', then
use 'ues_gnttab' for consoles where ring-ref holds grant table
reference.
---
 tools/console/daemon/io.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 92c3dff..2cf71a0 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -110,6 +110,7 @@ struct console {
 	struct domain *d;
 	bool optional;
 	bool use_gnttab;
+	bool use_reserved_gnttab;
 	bool have_state;
 };
 
@@ -119,6 +120,7 @@ struct console_type {
 	char *log_suffix;
 	bool optional;
 	bool use_gnttab;
+	bool use_reserved_gnttab;
 	bool have_state;  // uses 'state' xenstore entry
 };
 
@@ -129,6 +131,7 @@ static struct console_type console_type[] = {
 		.log_suffix = "",
 		.optional = false,
 		.use_gnttab = true,
+		.use_reserved_gnttab = true,
 	},
 #if defined(CONFIG_ARM)
 	{
@@ -670,7 +673,7 @@ static void console_unmap_interface(struct console *con)
 {
 	if (con->interface == NULL)
 		return;
-	if (xgt_handle && con->ring_ref == -1)
+	if (xgt_handle && con->use_gnttab)
 		xengnttab_unmap(xgt_handle, con->interface, 1);
 	else
 		munmap(con->interface, XC_PAGE_SIZE);
@@ -714,12 +717,19 @@ static int console_create_ring(struct console *con)
 
 	if (!con->interface && xgt_handle && con->use_gnttab) {
 		/* Prefer using grant table */
-		con->interface = xengnttab_map_grant_ref(xgt_handle,
-			dom->domid, GNTTAB_RESERVED_CONSOLE,
-			PROT_READ|PROT_WRITE);
-		con->ring_ref = -1;
+		if (con->use_reserved_gnttab) {
+			con->interface = xengnttab_map_grant_ref(xgt_handle,
+					dom->domid, GNTTAB_RESERVED_CONSOLE,
+					PROT_READ|PROT_WRITE);
+			con->ring_ref = -1;
+		} else {
+			con->interface = xengnttab_map_grant_ref(xgt_handle,
+					dom->domid, ring_ref,
+					PROT_READ|PROT_WRITE);
+			con->ring_ref = ring_ref;
+		}
 	}
-	if (!con->interface) {
+	if (!con->interface && (!con->use_gnttab || con->use_reserved_gnttab)) {
 		/* Fall back to xc_map_foreign_range */
 		con->interface = xc_map_foreign_range(
 			xc, dom->domid, XC_PAGE_SIZE,
@@ -893,6 +903,7 @@ static int console_init(struct console *con, struct domain *dom, void **data)
 	con->log_suffix = (*con_type)->log_suffix;
 	con->optional = (*con_type)->optional;
 	con->use_gnttab = (*con_type)->use_gnttab;
+	con->use_reserved_gnttab = (*con_type)->use_reserved_gnttab;
 	con->have_state = (*con_type)->have_state;
 	xsname = (char *)(*con_type)->xsname;
 	xspath = xs_get_domain_path(xs, dom->domid);
-- 
git-series 0.9.1

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

[RFC PATCH 13/17] xenconsoled: add support for up to 3 secondary consoles

[Marek Marczykowski-Górecki]

Based on previous few commits, this adds basic support for multiple
consoles in xenconsoled. A static number of them - up to 3 (+ one
primary).

---
I know this is awful, but everything else I can think of (real support
for multiple consoles, dynamically allocated) requires major restructure
of the code. Given I'm still not sure if multiple consoles are the way
to go with stubdomain, I'd rather not invest time in something that
could never by actually useful.
---
 tools/console/daemon/io.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 2cf71a0..7761c60 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -133,6 +133,30 @@ static struct console_type console_type[] = {
 		.use_gnttab = true,
 		.use_reserved_gnttab = true,
 	},
+	{
+		.xsname = "/device/console/1",
+		.ttyname = "tty",
+		.log_suffix = "-con1",
+		.optional = true,
+		.use_gnttab = true,
+		.have_state = true,
+	},
+	{
+		.xsname = "/device/console/2",
+		.ttyname = "tty",
+		.log_suffix = "-con2",
+		.optional = true,
+		.use_gnttab = true,
+		.have_state = true,
+	},
+	{
+		.xsname = "/device/console/3",
+		.ttyname = "tty",
+		.log_suffix = "-con3",
+		.optional = true,
+		.use_gnttab = true,
+		.have_state = true,
+	},
 #if defined(CONFIG_ARM)
 	{
 		.xsname = "/vuart/0",
-- 
git-series 0.9.1

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

[RFC PATCH 14/17] xenconsoled: deduplicate error handling

[Marek Marczykowski-Górecki]

---
 tools/console/daemon/io.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 7761c60..508bc2c 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -794,9 +794,7 @@ static int console_create_ring(struct console *con)
 
 	if (rc == -1) {
 		err = errno;
-		xenevtchn_close(con->xce_handle);
-		con->xce_handle = NULL;
-		goto out;
+		goto err_xce;
 	}
 	con->local_port = rc;
 	con->remote_port = remote_port;
@@ -804,11 +802,7 @@ static int console_create_ring(struct console *con)
 	if (con->master_fd == -1) {
 		if (!console_create_tty(con)) {
 			err = errno;
-			xenevtchn_close(con->xce_handle);
-			con->xce_handle = NULL;
-			con->local_port = -1;
-			con->remote_port = -1;
-			goto out;
+			goto err_xce;
 		}
 	}
 
@@ -818,6 +812,13 @@ static int console_create_ring(struct console *con)
 	/* if everything ok, signal backend readiness, in backend tree */
 	set_backend_state(con, XenbusStateConnected);
 
+ err_xce:
+	if (err) {
+		xenevtchn_close(con->xce_handle);
+		con->xce_handle = NULL;
+		con->local_port = -1;
+		con->remote_port = -1;
+	}
  out:
 	return err;
 }
-- 
git-series 0.9.1

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

[RFC PATCH 15/17] xenconsoled: add support for non-pty output

[Marek Marczykowski-Górecki]

Handle 'output' xenstore entry, as qemu does. Right now support only few
simple options:
 - "pty" (unchanged)
 - "file:path" (overwrite file)
 - "pipe:path" (read-write file/pipe)
 - "null"

Also, when ever read() returns 0, stop reading from that source, instead
of spinning in a loop.
For now, in case of reconnect, intentionally use pty (ignore 'output'
xenstore entry), as for normal files close+open can be harmful
(especially with O_TRUNC).
---
According to qemu docs, "pipe" should behave differently - it should
open two pipes, with ".in" and ".out". According to actual qemu code, it
does that, but if it fails it fallbacks to just one file. And libxl
relies on that fallback, so I've implemented that version only.
If xenconsoled would have some man page, I'd add it there...
---
 tools/console/daemon/io.c | 62 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 5 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 508bc2c..7afff99 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -95,6 +95,8 @@ struct console {
 	int master_fd;
 	int master_pollfd_idx;
 	int slave_fd;
+	bool master_fd_can_read;
+	bool master_fd_can_write;
 	int log_fd;
 	struct buffer buffer;
 	char *xspath;
@@ -705,6 +707,53 @@ static void console_unmap_interface(struct console *con)
 	con->ring_ref = -1;
 }
 
+static int create_console_output(struct console *con)
+{
+	int err;
+	char *output, *path;
+
+	if (asprintf(&path, "%s/%s", con->xspath, "output") == -1) {
+		err = ENOMEM;
+		goto out;
+	}
+
+	con->master_fd_can_read = true;
+	con->master_fd_can_write = true;
+
+	output = xs_read(xs, XBT_NULL, path, NULL);
+
+	if (!output || !strcmp(output, "pty")) {
+		if (!console_create_tty(con)) {
+			err = errno;
+			goto out;
+		}
+	} else if (!strncmp(output, "file:", 5)) {
+		con->master_fd = open(output+5, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+		if (con->master_fd == -1) {
+			err = errno;
+			goto out;
+		}
+		/* this is write-only file */
+		con->master_fd_can_read = false;
+	} else if (!strncmp(output, "pipe:", 5)) {
+		con->master_fd = open(output+5, O_RDWR);
+		if (con->master_fd == -1) {
+			err = errno;
+			goto out;
+		}
+	} else if (!strcmp(output, "null")) {
+		con->master_fd = open("/dev/null", O_RDWR);
+		if (con->master_fd == -1) {
+			err = errno;
+			goto out;
+		}
+	}
+
+	err = 0;
+out:
+	return err;
+}
+
 static int console_create_ring(struct console *con)
 {
 	int err, remote_port, ring_ref, rc;
@@ -800,10 +849,9 @@ static int console_create_ring(struct console *con)
 	con->remote_port = remote_port;
 
 	if (con->master_fd == -1) {
-		if (!console_create_tty(con)) {
-			err = errno;
+		err = create_console_output(con);
+		if (err)
 			goto err_xce;
-		}
 	}
 
 	if (log_guest && (con->log_fd == -1))
@@ -916,6 +964,8 @@ static int console_init(struct console *con, struct domain *dom, void **data)
 
 	con->master_fd = -1;
 	con->master_pollfd_idx = -1;
+	con->master_fd_can_read = true;
+	con->master_fd_can_write = true;
 	con->slave_fd = -1;
 	con->log_fd = -1;
 	con->ring_ref = -1;
@@ -1127,6 +1177,8 @@ static void handle_tty_read(struct console *con)
 	 */
 	if (len < 0) {
 		console_handle_broken_tty(con, domain_is_valid(dom->domid));
+	} else if (len == 0) {
+		con->master_fd_can_read = false;
 	} else if (domain_is_valid(dom->domid)) {
 		prod = intf->in_prod;
 		for (i = 0; i < len; i++) {
@@ -1371,10 +1423,10 @@ static void maybe_add_console_tty_fd(struct console *con)
 {
 	if (con->master_fd != -1) {
 		short events = 0;
-		if (!con->d->is_dead && ring_free_bytes(con))
+		if (!con->d->is_dead && con->master_fd_can_read && ring_free_bytes(con))
 			events |= POLLIN;
 
-		if (!buffer_empty(&con->buffer))
+		if (con->master_fd_can_write && !buffer_empty(&con->buffer))
 			events |= POLLOUT;
 
 		if (events)
-- 
git-series 0.9.1

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

[RFC PATCH 16/17] libxl: access QMP socket via console for qemu-in-stubdomain

[Marek Marczykowski-Górecki]

Add support for talking with qemu in stubdomain via QMP connected to a
console. Since a console doesn't have out of band connect/disconnect
signaling, use (new) qmp_reset command at every connect, to force
renegotiation.

This commit doesn't deal with multiple users accessing the same console.
For example all connected libxl users will see responses for commands
send by any user. If commands IDs would be unique (they aren't),
theoretically that wouldn't be a problem. But two instances sending
commands at once is still problematic.
---
 tools/libxl/libxl_dm.c       | 18 ++++++++-----
 tools/libxl/libxl_internal.h |  5 ++--
 tools/libxl/libxl_qmp.c      | 52 +++++++++++++++++++++++++++++++++----
 3 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 11acbea..062020e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -959,18 +959,23 @@ 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 */
-    if (!is_stubdom) {
-        flexarray_append(dm_args, "-chardev");
+    flexarray_append(dm_args, "-chardev");
+    /* stubdom + qemu-xen implies linux based stubdom */
+    if (is_stubdom)
+        flexarray_append(dm_args,
+                         GCSPRINTF("serial,id=libxl-cmd,path=/dev/hvc%d",
+                                   STUBDOM_CONSOLE_QMP));
+    else
         flexarray_append(dm_args,
                          GCSPRINTF("socket,id=libxl-cmd,"
                                         "path=%s/qmp-libxl-%d,server,nowait",
                                         libxl__run_dir_path(), 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");
 
+    if (!is_stubdom) {
         flexarray_append(dm_args, "-chardev");
         flexarray_append(dm_args,
                          GCSPRINTF("socket,id=libxenstat-cmd,"
@@ -2132,6 +2137,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
         /* STUBDOM_CONSOLE_LOGGING (console 0) is for minios logging
          * STUBDOM_CONSOLE_SAVE (console 1) is for writing the save file
          * STUBDOM_CONSOLE_RESTORE (console 2) is for reading the save file
+         * STUBDOM_CONSOLE_QMP (console 3) is for accessing qmp socket (linux stubdom only)
          */
         switch (i) {
             char *filename;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9a01aa3..2aa8c8a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -115,8 +115,9 @@
 #define STUBDOM_CONSOLE_LOGGING 0
 #define STUBDOM_CONSOLE_SAVE 1
 #define STUBDOM_CONSOLE_RESTORE 2
-#define STUBDOM_CONSOLE_SERIAL 3
-#define STUBDOM_SPECIAL_CONSOLES 3
+#define STUBDOM_CONSOLE_QMP 3
+#define STUBDOM_CONSOLE_SERIAL 4
+#define STUBDOM_SPECIAL_CONSOLES 4
 #define LIBXL_LINUX_STUBDOM_MEM 128
 #define TAP_DEVICE_SUFFIX "-emu"
 #define DOMID_XS_PATH "domid"
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 0fe4281..8ba558b 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -431,6 +431,16 @@ out:
     return ret;
 }
 
+static int qmp_open_pty(libxl__qmp_handler *qmp, const char *qmp_pty_path)
+{
+    qmp->qmp_fd = open(qmp_pty_path,
+                       O_RDWR | O_NOCTTY | O_NONBLOCK | O_CLOEXEC);
+    if (qmp->qmp_fd < 0) {
+        return -1;
+    }
+    return 0;
+}
+
 static void qmp_close(libxl__qmp_handler *qmp)
 {
     callback_id_pair *pp = NULL;
@@ -720,15 +730,47 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc, uint32_t domid)
     int ret = 0;
     libxl__qmp_handler *qmp = NULL;
     char *qmp_socket;
+    uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
 
     qmp = qmp_init_handler(gc, domid);
     if (!qmp) return NULL;
 
-    qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
-    if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
-        LOGED(ERROR, domid, "Connection error");
-        qmp_free_handler(qmp);
-        return NULL;
+    if (dm_domid) {
+        /* sanity check */
+        if (libxl__stubdomain_version_running(gc, dm_domid) ==
+                LIBXL_STUBDOMAIN_VERSION_MINIOS) {
+            LOGED(ERROR, domid, "QMP socket unsupported for minios stubdomain");
+            qmp_free_handler(qmp);
+            return NULL;
+        }
+        ret = libxl_console_get_tty(CTX, dm_domid,
+                                    STUBDOM_CONSOLE_QMP,
+                                    LIBXL_CONSOLE_TYPE_PV,
+                                    &qmp_socket);
+        if (ret) {
+            LOGED(ERROR, domid, "Failed to get QMP tty path");
+            qmp_free_handler(qmp);
+            return NULL;
+        }
+        ret = qmp_open_pty(qmp, qmp_socket);
+        if (ret) {
+            LOGED(ERROR, domid, "Connection error");
+            qmp_free_handler(qmp);
+            return NULL;
+        }
+        ret = qmp_send(qmp, "qmp_reset", NULL, NULL, NULL, NULL);
+        if (ret < 0) {
+            LOGED(ERROR, domid, "Failed to send qmp_reset");
+            qmp_free_handler(qmp);
+            return NULL;
+        }
+    } else {
+        qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
+        if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
+            LOGED(ERROR, domid, "Connection error");
+            qmp_free_handler(qmp);
+            return NULL;
+        }
     }
 
     LOGD(DEBUG, domid, "connected to %s", qmp_socket);
-- 
git-series 0.9.1

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

[RFC PATCH 17/17] libxl: use xenconsoled even for multiple stubdomain's consoles

[Marek Marczykowski-Górecki]

Since multiple consoles support was added to xenconsoled, use it for
stubdomain. This makes it possible to have HVM without qemu in dom0 at
al. As long as no other feature requiring qemu in dom0 is used, like VNC
or qdisk.
---
 tools/libxl/libxl_dm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 062020e..6c9d23e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2133,7 +2133,9 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 
     for (i = 0; i < num_console; i++) {
         console[i].devid = i;
-        console[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU;
+        /* will be changed back to LIBXL__CONSOLE_BACKEND_IOEMU if qemu
+         * will be in use */
+        console[i].consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
         /* STUBDOM_CONSOLE_LOGGING (console 0) is for minios logging
          * STUBDOM_CONSOLE_SAVE (console 1) is for writing the save file
          * STUBDOM_CONSOLE_RESTORE (console 2) is for reading the save file
@@ -2149,9 +2151,6 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
                 if (ret) goto out;
                 console[i].output = GCSPRINTF("file:%s", filename);
                 free(filename);
-                /* will be changed back to LIBXL__CONSOLE_BACKEND_IOEMU if qemu
-                 * will be in use */
-                console[i].consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
                 break;
             case STUBDOM_CONSOLE_SAVE:
                 console[i].output = GCSPRINTF("file:%s",
-- 
git-series 0.9.1

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