[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/3] libxl: make creation of xenstore suspend event channel node optional


On 26/02/2020 16:08, Paul Durrant wrote:
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.


       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@xxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Wei Liu <wl@xxxxxxx>
Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  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 +++

You may want to update xenstore-paths.pandoc as the document mention the node will be created by the toolstack.

  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

In the next patch you are going to make device/ rw. Do you see any issue with just not creating the node for everyone? Are PV drivers allowed to assume a node will be present?

My knowledge of xenstore is limited, so I thought I would ask the questions to understand a bit more how stable the ABI is meant to be. :).


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.