libxl: make creation of xenstore 'suspend event channel' node optional...

... and, if it is not created, make the top level 'device' node in
xenstore writable by the guest instead.

The purpose and semantics of the suspend event channel 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 a 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 (missing system disk).

This patch adds a boolean 'xend_suspend_evtchn_compat' field into
libxl_create_info and a similarly named option in xl.cfg to set it.
If the value is true then the xenstore node is not created. Instead the
old xend behaviour of making top level device node writable by the guest is
re-instated. If the value is false (the default) then the current libxl
behaviour persists.

xenstore-paths.pandoc is also modified to say that the suspend event
channel node may not exist and, if it does not exist, then the guest may
create it. A note is also added concerning the writability of the top
level device node.

[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_... definition into
      libxl.h, this patch corrects the previous stanza which erroneously
      implies libxl_domain_create_info is a function.

Signed-off-by: Paul Durrant <paul@xen.org>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 0cad56137595c5a48a83f02d6b1c6f523be92416..0e9e58a41a80296fab848d7d9e214a4b9630cde9 100644 (file)
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -668,6 +668,19 @@ file.
 
 =back
 
+=item B<xend_suspend_evtchn_compat=BOOLEAN>
+
+If this option is B<true> the xenstore path for the domain's suspend
+event channel will not be created. Instead the old xend behaviour of
+making the whole xenstore B<device> sub-tree writable by the domain will
+be re-instated.
+
+The existence of the suspend event channel path can cause problems with
+certain PV drivers running in the guest (e.g. old Red Hat PV drivers for
+Windows).
+
+If this option is not specified then it will default to B<false>.
+
 =back
 
 =head2 Devices

diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc
index e2ab5da54e7f973cb3c6e9f62e767d8562c4a8eb..ff3ca0406946039da55543c3c6590f6c0aad1d54 100644 (file)
--- a/docs/misc/xenstore-paths.pandoc
+++ b/docs/misc/xenstore-paths.pandoc
@@ -176,10 +176,12 @@ The size of the video RAM this domain is configured with.
 
 #### ~/device/suspend/event-channel = ""|EVTCHN [w]
 
-The domain's suspend event channel. The toolstack will create this
-path with an empty value which the guest may choose to overwrite.
+The domain's suspend event channel. The toolstack may create this
+path with an empty value which the guest may choose to overwrite. If
+the path does not exist then the ~/device path will be writable by the
+guest and hence it may create the suspend event channel path.
 
-If the guest overwrites this, it will be with the number of an unbound
+If the guest writes this, it will be with the number of an unbound
 event channel port it has acquired.  The toolstack is expected to use
 an interdomain bind, and then, when it wishes to ask the guest to
 suspend, to signal the event channel.
@@ -267,7 +269,9 @@ circumstances where the generation ID needs to be changed.
 Paravirtual device frontends are generally specified by their own
 directory within the XenStore hierarchy. Usually this is under
 ~/device/$TYPE/$DEVID although there are exceptions, e.g. ~/console
-for the first PV console.
+for the first PV console. The top level ~/device path itself is normally
+read-only to the guest. However it may writable if the
+'xend_suspend_evtchn_compat' guest configuration option is enabled.
 
 #### ~/device/vbd/$DEVID/* []
 

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 35e13428b2a1f3cd19b7d6b2eb2d28b5eb1ec03c..71709dc5859fa5400e69adb38e516ecb41085dbf 100644 (file)
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1272,10 +1272,19 @@ 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_XEND_SUSPEND_EVTCHN_COMPAT
+ *
+ * libxl_domain_create_info contains a boolean 'xend_suspend_evtchn_compat'
+ * value to control creation of the xenstore path for a domain's suspend
+ * event channel.
+ */
+#define LIBXL_HAVE_CREATEINFO_XEND_SUSPEND_EVTCHN_COMPAT
+
 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 e18aad43b556a43fee88d22ecac3ba64661b62e6..e7cb2dbc2b2bf0d2ff6e0ed3accfe5041bfda223 100644 (file)
--- 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->xend_suspend_evtchn_compat, false);
+
     return 0;
 }
 
@@ -748,9 +750,21 @@ retry_transaction:
     libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/memory", dom_path),
                     roperm, ARRAY_SIZE(roperm));
-    libxl__xs_mknod(gc, t,
-                    GCSPRINTF("%s/device", dom_path),
-                    roperm, ARRAY_SIZE(roperm));
+
+    if (!libxl_defbool_val(info->xend_suspend_evtchn_compat)) {
+        libxl__xs_mknod(gc, t,
+                        GCSPRINTF("%s/device", dom_path),
+                        roperm, ARRAY_SIZE(roperm));
+        libxl__xs_mknod(gc, t,
+                        GCSPRINTF("%s/device/suspend/event-channel",
+                                  dom_path),
+                        rwperm, ARRAY_SIZE(rwperm));
+    } else {
+        libxl__xs_mknod(gc, t,
+                        GCSPRINTF("%s/device", dom_path),
+                        rwperm, ARRAY_SIZE(rwperm));
+    }
+
     libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/control", dom_path),
                     roperm, ARRAY_SIZE(roperm));
@@ -782,9 +796,7 @@ 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));
+
     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 d0d431614fa16d61dcfe1d319e45255a2d78cbb3..f7c473be7401670df5a2c1ed2c88105b4faece89 100644 (file)
--- 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),
+    ("xend_suspend_evtchn_compat",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 b88118480485136dfa4821e0727477c6bc084942..4450d59f16ca6b9f578598fdf5f738379e9c501e 100644 (file)
--- 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, "xend_suspend_evtchn_compat",
+                        &c_info->xend_suspend_evtchn_compat, 0);
+
     xlu_cfg_destroy(config);
 }