All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Durrant <pdurrant@amazon.com>
To: <xen-devel@lists.xenproject.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>,
	Paul Durrant <pdurrant@amazon.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Wei Liu <wl@xen.org>
Subject: [Xen-devel] [PATCH 2/3] libxl: make creation of xenstore suspend event channel node optional
Date: Wed, 26 Feb 2020 16:08:47 +0000	[thread overview]
Message-ID: <20200226160848.1854-3-pdurrant@amazon.com> (raw)
In-Reply-To: <20200226160848.1854-1-pdurrant@amazon.com>

The purpose and semantics of the node are explained in
xenstore-paths.pandoc [1]. It was originally introduced in xend by commit
17636f47a474 "Teach xc_save to use event-channel-based domain suspend if
available.". Note that, because, the top-level frontend 'device' node was
created writable by the guest in xend, there was no need to explicitly
create the 'suspend-event-channel' node as writable node.

However, libxl creates the 'device' node as read-only by the guest and so
explicit creation of the 'suspend-event-channel' node is necessary to make
it usable. This unfortunately has the side-effect of making some old
Windows PV drivers [2] cease to function. This is because they scan the top
level 'device' node, find the 'suspend' node and expect it to contain the
usual sub-nodes describing a PV frontend. When this is found not to be the
case, enumeration ceases and (because the 'suspend' node is observed before
the 'vbd' node) no system disk is enumerated. Windows will then crash with
bugcheck code 0x7B.

This patch adds a boolean 'suspend_event_channel' field into
libxl_create_info to control whether the xenstore node is created and a
similarly named option in xl.cfg which, for compatibility with previous
libxl behaviour, defaults to true.

[1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/xenstore-paths.pandoc;hb=HEAD#l177
[2] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/5/html/para-virtualized_windows_drivers_guide/sect-para-virtualized_windows_drivers_guide-installing_and_configuring_the_para_virtualized_drivers-installing_the_para_virtualized_drivers

NOTE: While adding the new LIBXL_HAVE_CREATEINFO_SUSPEND_EVENT_CHANNEL
      definition into libxl.h, this patch corrects the previous stanza
      which erroneously implies ibxl_domain_create_info is a function.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
---
 docs/man/xl.cfg.5.pod.in    |  7 +++++++
 tools/libxl/libxl.h         | 13 ++++++++++++-
 tools/libxl/libxl_create.c  | 12 +++++++++---
 tools/libxl/libxl_types.idl |  1 +
 tools/xl/xl_parse.c         |  3 +++
 5 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 0cad561375..5f476f1e1d 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -668,6 +668,13 @@ file.
 
 =back
 
+=item B<suspend_event_channel=BOOLEAN>
+
+Create the xenstore path for the domain's suspend event channel. The
+existence of this path can cause problems with older PV drivers running
+in the guest. If this option is not specified then it will default to
+B<true>.
+
 =back
 
 =head2 Devices
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 35e13428b2..d2afe48512 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1272,10 +1272,21 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  * LIBXL_HAVE_CREATEINFO_DOMID
  *
  * libxl_domain_create_new() and libxl_domain_create_restore() will use
- * a domid specified in libxl_domain_create_info().
+ * a domid specified in libxl_domain_create_info.
  */
 #define LIBXL_HAVE_CREATEINFO_DOMID
 
+/*
+ * LIBXL_HAVE_CREATEINFO_SUSPEND_EVENT_CHANNEL
+ *
+ * libxl_domain_create_info contains a boolean 'suspend_event_channel'
+ * value to control whether the xenstore path:
+ *
+ * /local/domain/$DOMID/device/suspend/event-channel (RW)
+ *
+ * is created.
+ */
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 27627cb199..7119e95412 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -57,6 +57,8 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
     if (!c_info->ssidref)
         c_info->ssidref = SECINITSID_DOMU;
 
+    libxl_defbool_setdefault(&c_info->suspend_event_channel, true);
+
     return 0;
 }
 
@@ -782,9 +784,13 @@ retry_transaction:
     libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/control/sysrq", dom_path),
                     rwperm, ARRAY_SIZE(rwperm));
-    libxl__xs_mknod(gc, t,
-                    GCSPRINTF("%s/device/suspend/event-channel", dom_path),
-                    rwperm, ARRAY_SIZE(rwperm));
+
+    if (libxl_defbool_val(info->suspend_event_channel))
+        libxl__xs_mknod(gc, t,
+                        GCSPRINTF("%s/device/suspend/event-channel",
+                                  dom_path),
+                        rwperm, ARRAY_SIZE(rwperm));
+
     libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/data", dom_path),
                     rwperm, ARRAY_SIZE(rwperm));
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index d0d431614f..2bce19bcf0 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -418,6 +418,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
     ("run_hotplug_scripts",libxl_defbool),
     ("driver_domain",libxl_defbool),
     ("passthrough",  libxl_passthrough),
+    ("suspend_event_channel",libxl_defbool),
     ], dir=DIR_IN)
 
 libxl_domain_restore_params = Struct("domain_restore_params", [
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index b881184804..122c6eb641 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2725,6 +2725,9 @@ skip_usbdev:
 
     parse_vkb_list(config, d_config);
 
+    xlu_cfg_get_defbool(config, "suspend_event_channel",
+                        &c_info->suspend_event_channel, 0);
+
     xlu_cfg_destroy(config);
 }
 
-- 
2.20.1


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

  parent reply	other threads:[~2020-02-26 16:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-26 16:08 [Xen-devel] [PATCH 0/3] PV driver compatibility fixes Paul Durrant
2020-02-26 16:08 ` [Xen-devel] [PATCH 1/3] libxl: create domain 'error' node in xenstore Paul Durrant
2020-02-26 16:25   ` Ian Jackson
2020-02-26 16:08 ` Paul Durrant [this message]
2020-02-27 22:51   ` [Xen-devel] [PATCH 2/3] libxl: make creation of xenstore suspend event channel node optional Julien Grall
2020-02-28  9:28     ` Durrant, Paul
2020-02-28 10:25       ` Julien Grall
2020-02-28 10:46         ` Durrant, Paul
2020-02-26 16:08 ` [Xen-devel] [PATCH 3/3] libxl: make the top level 'device' node in xenstore writable Paul Durrant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200226160848.1854-3-pdurrant@amazon.com \
    --to=pdurrant@amazon.com \
    --cc=anthony.perard@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.