[PATCH v7 for-4.5]: xl/libxl: add support for 'channels'

[PATCH v7 for-4.5]: xl/libxl: add support for 'channels'

[David Scott]

Hi,

Here's the latest spin of these patches, hopefully with everything
addressed (mainly memory leaks).

Quoting IanC in
http://lists.xenproject.org/archives/html/xen-devel/2014-10/msg00943.html

> The most major comment I saw was this one from Ian J which Wei reposted
> as:
>
>       Ian Jackson asked one question which remained unanswered in the last
>       iteration.
>
>       QUOTE
>       There are also unfortunate security implications to reading the
>       backend directory like that - if we have a driver domain, the qemu
>       might get untrustworthy paths.
>       ENDQUOTE
>
>       Has that question become moot? Did I miss anything?
>
>       Wei.

When Wei highlighted my omission I went back to the original thread and
tried to answer there. The answer may have gotten buried -- I'll repeat
it inline here.

Quoting myself in
http://lists.xenproject.org/archives/html/xen-devel/2014-09/msg04206.html

> I agree that we must be careful not to start a driver domain and tell it
> ‘talk to the socket at path x’ when in fact ‘x’ is in a different domain.
> At the moment paths are in the driver domain and the person who writes
> the domain config must consider this when they choose their driver domain.
>
> Thinking about the future, although we could arrange it such that the
> data gets routed to other places (e.g. run qemu/xenconsoled in the driver
> domain, have it proxy the data over a vchan, have a thing in dom0 proxy
> to the right socket) I suspect that we’ll want to add additional
> ‘connection’ behaviours which reference resources via global names rather
> than local ones. I quite like the idea of adding some kind of URI to
> name a network endpoint and have the driver domain proxy all the data
> to/from there.
> 
> I’m not totally sure that’s what you were getting at — sorry if I’ve missed
> the point!

I'm still not totally sure that I've adequately answered IanJ's question.

Sorry for the confusion!

The blurb:

Several libvirt applications (e.g. oVirt, CloudStack) make use of 'channels': 
low-bandwidth private host <-> guest communication links which resemble serial 
ports. Typical uses include: 

* initial VM configuration without using the network: read the config data 
directly from the channel on boot 

* controlling a guest agent: signal shutdown reqests, exchange clipboard data, 
trigger resolution changes 

This patch set re-uses the existing PV console protocol implemented by qemu 
to provide this service. 

If you declare a channel in your .xl file as follows: 

channel = [ "connection=socket,path=/tmp/mysock,name=org.mydomain.guest-agent" ] 

then an extra PV console will be added to your guest. This console has the 
extra key in the frontend 

name = guest-agent 

which allows udev scripts in the VM to create a named device in a well-known 
location (e.g. /dev/xen-channels/guest-agent, similar to the KVM /dev/vports). 
The qemu process in the backend domain will proxy the data to/from the named 
Unix domain socket (in this case /tmp/mysock). 

Note this mechanism is intended for low-bandwidth communication only. If an 
application requires a high-bandwith connection then it should use a direct 
vchan connection (and not proxy it via a qemu). 

Changes since v6:
* libxl: init_console_from_channel: perform validation before allocation
* libxl: init_console_info now reurns void (NB libxl__strdup handles
  allocation failure by _exit, so no need to return ERROR_ENOMEM)
* libxl: split out replace of memset with libxl__device_console_init
  to make review easier
* libxl: call libxl_channel_dispose on the error path to be safe
* xl: add a case for CONNECTION_PTY and exit(1) for an unexpected case
* xl: trim: always set output to NULL to avoid returning garbage
* xl: trim: fix a broken comment
* docs: add example udev rule (although the actual udev rule should go
  upstream to udev)
* docs: name the linux driver (CONFIG_HVC_XEN_FRONTEND)
* lots of whitespace fixes

Changes since v5:
* Fix use of <ctype.h> functions, and add a warning note
* Split out string fiddling into separate squash-able patches.
  (Note I've added __attribute__((unused)) to keep it building)
* doc: clearly describe leading/trailing whitespace handling and that
  [,="] are banned characters
* doc: mention the xenstore key 'name' (oops)
* doc: mention in the IDL and libxl.h that channels manifest as consoles
  with names. Note that consoles are still considered an internal type
  (in libxl_types_internal.idl) and don't appear in libxl_domain_config.

Changes since v4:
* doc: highlight that the 'output' key in the console configuration
  should be considered an internal implementation artifact
* return EINVAL if a primary console has invalid configuration
* Use LOG(ERROR,... rather than LIBXL__LOG(CTX, LIBXL__LOG_ERROR
* Use an abort() when implementation code is missing
* move READ_BACKEND to the top of the file (since it's generally useful)
* Remove stray tab

Changes since v3:
* docs: coalesce the docs patch into the libxl patch
* docs: in channels.txt give high-level usage information, pitfalls and
        channgle registry
* docs: move the xenstore paths into the existing console.txt, which is
        referenced from xenstore-paths.markdown
* idl: rename 'kind' to 'connection'
* xl: add parser functions for comma-separated lists of pairs

Changes since v2:
* trim down the 'kinds' of channels to PTY and SOCKET -- these seem the most
 useful and we can add more later
* add a channelinfo (queryable by 'channel-list') to check the state of each
 channel, and for a kind=PTY discover the slave device path
* IDL: switched to KeyedUnion for both the channel and channelinfo since
 each 'kind' will have different parameters (e.g. only SOCKET has PATH)
* write all the backend configuration parameters to xenstore -- where we were
 using qemu -chardev some crucial information was only on the command-line.
* add LIBXL_HAVE_DEVICE_CHANNEL
* docs: replace 'should' with 'will' e.g. the backend will be connected to
 a Unix domain socket
* squash all the libxl patches together into a coherent whole
* explain that there is no registry of channel names and so people should use
 unique information to create them (e.g. include domain name and interface
 version)

Changes since v1: 
* extend xl.cfg(5) 
* add docs/misc/channel.txt 
* libxl_types.idl: omit unnecessary init_val = 0 
* xl_cmdimpl.c: fixed over-long lines 
* xl_cmdimpl.c: use xrealloc (via ARRAY_EXTEND_INIT) 
* xl_cmdimpl.c: exit() on parse failure rather than ignore configuration 
* libxl_create.c: use libxl__device_console_init instead of memset 
* libxl_create.c: use libxl__strdup(NOGC rather than raw strdup 
* libxl.c: add name=<name> to console frontend 
* libxl.c: resolve the backend_domid from backend_domname 
* libxl_dm.c: channels[i].devid rather than i 
* libxl_dm.c: fix indentation 
* libxl_dm.c: use GCSPRINTF convenience macro 

Signed-off-by: David Scott <dave.scott@citrix.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v7 for-4.5 1/7] libxl: replace memset() with libxl__device_console_init

[David Scott]

The current implementation of libxl__device_console_init does the same
memset() so this is identical for now.

If libxl__device_console_init changes in future (e.g. to prefer a non-zero
default value for some field) then this will continue to work.

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 tools/libxl/libxl_create.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 8b82584..543c1b3 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -389,7 +389,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
 
 static int init_console_info(libxl__device_console *console, int dev_num)
 {
-    memset(console, 0x00, sizeof(libxl__device_console));
+    libxl__device_console_init(console);
     console->devid = dev_num;
     console->consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
     console->output = strdup("pty");
-- 
1.7.10.4

[PATCH v7 for-4.5 2/7] libxl: add support for 'channels'

[David Scott]

A 'channel':
  - is a low-bandwidth private communication channel that resembles
    a physical serial port.
  - is implemented as a PV console with a well-known string name
    which is used to hook the channel to the appropriate software
    in the guest (i.e. some kind of guest agent).
  - has a backend 'connection' which describes what should happen
    to the data.

The following 'connection' types are defined:

 * PTY: the I/O surfaces as a pty in the backend domain
 * SOCKET: a listening Unix domain socket accepts a connection in
   the backend domain and proxies

Channels may be listed but don't currently support hotplug/unplug.

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 docs/misc/channel.txt                |  106 +++++++++++++
 docs/misc/console.txt                |   69 ++++++---
 tools/libxl/libxl.c                  |  273 +++++++++++++++++++++++++++++++---
 tools/libxl/libxl.h                  |   20 +++
 tools/libxl/libxl_create.c           |   43 ++++--
 tools/libxl/libxl_dm.c               |   46 +++++-
 tools/libxl/libxl_internal.h         |   10 +-
 tools/libxl/libxl_types.idl          |   37 +++++
 tools/libxl/libxl_types_internal.idl |    5 +
 9 files changed, 555 insertions(+), 54 deletions(-)
 create mode 100644 docs/misc/channel.txt

diff --git a/docs/misc/channel.txt b/docs/misc/channel.txt
new file mode 100644
index 0000000..9fc701a
--- /dev/null
+++ b/docs/misc/channel.txt
@@ -0,0 +1,106 @@
+Xen PV Channels
+===============
+
+A channel is a low-bandwidth private byte stream similar to a serial
+link. Typical uses of channels are
+
+  1. to provide initial configuration information to a VM on boot
+     (example use: CloudStack's cloud-early-config service)
+  2. to signal/query an in-guest agent
+     (example use: oVirt's guest agent)
+
+Channels are similar to virtio-serial devices and emulated serial links.
+Channels are intended to be used in the implementation of libvirt <channel>s
+when running on Xen.
+
+Note: if an application requires a high-bandwidth link then it should use
+vchan instead.
+
+How to use channels: an example
+-------------------------------
+
+Consider a cloud deployment where VMs are cloned from pre-made templates,
+and customised on first boot by an in-guest agent which sets the IP address,
+hostname, ssh keys etc. To install the system the cloud administrator would
+first:
+
+  1. Install a guest as normal (no channel configuration necessary)
+  2. Install the in-guest agent specific to the cloud software. This will
+     prepare the guest to communicate over the channel, and also prepare
+     the guest to be cloned safely (sometimes known as "sysprepping")
+  3. Shutdown the guest
+  4. Register the guest as a template with the cloud orchestration software
+  5. Install the cloud orchestration agent in dom0
+
+At runtime, when a cloud tenant requests that a VM is created from the template,
+the sequence of events would be: (assuming a Linux domU)
+
+  1. A VM is "cloned" from the template
+  2. A unique Unix domain socket path in dom0 is allocated
+     (e.g. /my/cloud/software/talk/to/domain/<vm uuid>)
+  3. Domain configuration is created for the VM, listing the channel
+     name expected by the in-guest agent. In xl syntax this would be:
+
+     channel = [ "connection=socket, name=org.my.cloud.software.agent.version1,
+                  path = /my/cloud/software/talk/to/domain/<vm uuid>" ]
+
+  4. The VM is started
+  5. In dom0 the cloud orchestration agent connects to the Unix domain
+     socket, writes a handshake message and waits for a reply
+  6. Assuming the guest kernel has CONFIG_HVC_XEN_FRONTEND set then the console
+     driver will generate a hotplug event
+  7. A udev rule is activated by the hotplug event.
+
+     The udev rule would look something like:
+
+     SUBSYSTEM=="xen", DEVPATH=="/devices/console-[0-9]", RUN+="xen-console-setup"
+
+     where the "xen-console-setup" script would read the channel name and
+     make a symlink in /dev/xen-channel/org.my.cloud.software.agent.version1
+
+  8. The in-guest agent uses inotify to see the creation of the /dev/xen-channel
+     symlink and opens the device.
+  9. The in-guest agent completes the handshake with the dom0 agent
+ 10. The dom0 agent transmits the unique VM configuration: hostname, IP
+     address, ssh keys etc etc
+ 11. The in-guest agent receives the configuration and applies it.
+
+Using channels avoids having to use a temporary disk device or network
+connection.
+
+Design recommendations and pitfalls
+-----------------------------------
+
+It's necessary to install channel-specific software (an "agent") into the guest
+before you can use a channel. By default a channel will appear as a device
+which could be mistaken for a serial port or regular console. It is known
+that some software will proactively seek out serial ports and issue AT commands
+at them; make sure such software is disabled!
+
+Since channels are identified by names, application authors must ensure their
+channel names are unique to avoid clashes. We recommend that channel names
+include parts unique to the application such as a domain names. To assist
+prevent clashes we recommend authors add their names to our global channel
+registry at the end of this document.
+
+Limitations
+-----------
+
+Hotplug and unplug of channels is not currently implemented.
+
+Channel name registry
+---------------------
+
+It is important that channel names are globally unique. To help ensure
+that no-one's name clashes with yours, please add yours to this list.
+
+Key:
+N: Name
+C: Contact
+D: Short description of use, possibly including a URL to your software
+   or API
+
+N: org.xenproject.guest.clipboard.0.1
+C: David Scott <dave.scott@citrix.com>
+D: Share clipboard data via an in-guest agent. See:
+   http://wiki.xenproject.org/wiki/Clipboard_sharing_protocol
diff --git a/docs/misc/console.txt b/docs/misc/console.txt
index 8a53a95..ed7b795 100644
--- a/docs/misc/console.txt
+++ b/docs/misc/console.txt
@@ -9,10 +9,11 @@ relevant information in xenstore under /local/domain/$DOMID/console.
 
 Now many years after the introduction of the pv console we have
 multiple pv consoles support for pv and hvm guests; multiple pv
-console backends (qemu and xenconsoled) and emulated serial cards too.
+console backends (qemu and xenconsoled, see limitations below) and
+emulated serial cards too.
 
 This document tries to describe how the whole system works and how the
-different components interact with each others.
+different components interact with each other.
 
 The first PV console path in xenstore remains:
 
@@ -23,28 +24,63 @@ live in:
 
 /local/domain/$DOMID/device/console/$DEVID.
 
-The output of a PV console, whether it should be a file, a pty, a
-socket, or something else, is specified by the toolstack in the xenstore
-node "output", under the relevant console section.
-For example:
+PV consoles have
+* (optional) string names;
+* 'connection' information describing to what they should be
+  connected; and
+* a 'type' indicating which daemon should process the data.
+
+We call a PV console with a name a "channel", in reference to the libvirt
+concept with the same name and purpose. The file "channels.txt" describes
+how to use channels and includes a registry of well-known channel names.
+
+If the PV console has a name (i.e. it is a "channel") then the name
+is written to the frontend directory:
+
+name = <name>
+
+If the PV console has no name (i.e. it is a regular console) then the "name"
+key is omitted.
+
+The toolstack writes 'connection' information in the xenstore backend in
+the keys
+* connection: either 'pty' or 'socket'
+* path: only present if connection = 'socket', the path of the socket to
+  glue the channel to
+
+An artifact of the current implementation, the toolstack will write an
+extra backend key
+* output: an identifier only meaningful for qemu/xenconsoled
 
-# xenstore-read /local/domain/26/device/console/1/output
-pty
+If the toolstack wants the console to be connected to a pty, it will write
+to the backend:
 
-The backend chosen for a particular console is specified by the
-toolstack in the "type" node on xenstore, under the relevant console
-section.
+connection = pty
+output = pty
+
+The backend will write the pty device name to the "tty" node in the
+console frontend.
+
+If the toolstack wants a listening Unix domain socket to be created at path
+<path>, a connection accepted and data proxied to the console, it will write:
+
+connection = socket
+path = <path>
+output = chardev:<some internal identifier>
+
+where chardev:<some internal identifier> matches a qemu character device
+configured on the qemu command-line.
+
+The backend implementation daemon chosen for a particular console is specified
+by the toolstack in the "type" node in the xenstore frontend.
 For example:
 
 # xenstore-read /local/domain/26/console/1/type
-xenconsoled
+ioemu
 
 The supported values are only xenconsoled or ioemu; xenconsoled has
 several limitations: it can only be used for the first PV console and it
-can only have a pty as output.
-
-If the output is a pty, backends write the device name to the "tty" node
-in xenstore under the relevant console path.
+can only connect to a pty.
 
 Emulated serials are provided by qemu-dm only to hvm guests; the number
 of emulated serials depends on how many "-serial" command line options
@@ -54,7 +90,6 @@ xenstore in the following path:
 
 /local/domain/$DOMID/serial/$SERIAL_NUM/tty
 
-
 xenconsole is the tool to connect to a PV console or an emulated serial
 that has a pty as output. Xenconsole takes a domid as parameter plus an
 optional console type (pv for PV consoles or serial for emulated
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 77672d0..a4f4dca 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -21,6 +21,15 @@
 #define PAGE_TO_MEMKB(pages) ((pages) * 4)
 #define BACKEND_STRING_SIZE 5
 
+/* Utility to read backend xenstore keys */
+#define READ_BACKEND(tgc, subpath) ({                                   \
+        rc = libxl__xs_read_checked(tgc, XBT_NULL,                      \
+                                    GCSPRINTF("%s/" subpath, be_path),  \
+                                    &tmp);                              \
+        if (rc) goto out;                                               \
+        (char*)tmp;                                                     \
+    });
+
 int libxl_ctx_alloc(libxl_ctx **pctx, int version,
                     unsigned flags, xentoollog_logger * lg)
 {
@@ -3319,14 +3328,6 @@ static int libxl__device_nic_from_xs_be(libxl__gc *gc,
 
     libxl_device_nic_init(nic);
 
-#define READ_BACKEND(tgc, subpath) ({                                   \
-        rc = libxl__xs_read_checked(tgc, XBT_NULL,                      \
-                                    GCSPRINTF("%s/" subpath, be_path),  \
-                                    &tmp);                              \
-        if (rc) goto out;                                               \
-        (char*)tmp;                                                     \
-    });
-
     tmp = READ_BACKEND(gc, "handle");
     if (tmp)
         nic->devid = atoi(tmp);
@@ -3506,28 +3507,33 @@ const char *libxl__device_nic_devname(libxl__gc *gc,
 /******************************************************************************/
 int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                               libxl__device_console *console,
-                              libxl__domain_build_state *state)
+                              libxl__domain_build_state *state,
+                              libxl__device *device)
 {
     flexarray_t *front, *ro_front;
     flexarray_t *back;
-    libxl__device device;
     int rc;
 
     if (console->devid && state) {
         rc = ERROR_INVAL;
         goto out;
     }
+    if (!console->devid && (console->name || console->path)) {
+        LOG(ERROR, "Primary console has invalid configuration");
+        rc = ERROR_INVAL;
+        goto out;
+    }
 
     front = flexarray_make(gc, 16, 1);
     ro_front = flexarray_make(gc, 16, 1);
     back = flexarray_make(gc, 16, 1);
 
-    device.backend_devid = console->devid;
-    device.backend_domid = console->backend_domid;
-    device.backend_kind = LIBXL__DEVICE_KIND_CONSOLE;
-    device.devid = console->devid;
-    device.domid = domid;
-    device.kind = LIBXL__DEVICE_KIND_CONSOLE;
+    device->backend_devid = console->devid;
+    device->backend_domid = console->backend_domid;
+    device->backend_kind = LIBXL__DEVICE_KIND_CONSOLE;
+    device->devid = console->devid;
+    device->domid = domid;
+    device->kind = LIBXL__DEVICE_KIND_CONSOLE;
 
     flexarray_append(back, "frontend-id");
     flexarray_append(back, libxl__sprintf(gc, "%d", domid));
@@ -3540,6 +3546,19 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
     flexarray_append(back, "protocol");
     flexarray_append(back, LIBXL_XENCONSOLE_PROTOCOL);
 
+    if (console->name) {
+        flexarray_append(ro_front, "name");
+        flexarray_append(ro_front, console->name);
+    }
+    if (console->connection) {
+        flexarray_append(back, "connection");
+        flexarray_append(back, console->connection);
+    }
+    if (console->path) {
+        flexarray_append(back, "path");
+        flexarray_append(back, console->path);
+    }
+
     flexarray_append(front, "backend-id");
     flexarray_append(front, libxl__sprintf(gc, "%d", console->backend_domid));
 
@@ -3566,8 +3585,7 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
         flexarray_append(front, "protocol");
         flexarray_append(front, LIBXL_XENCONSOLE_PROTOCOL);
     }
-
-    libxl__device_generic_add(gc, XBT_NULL, &device,
+    libxl__device_generic_add(gc, XBT_NULL, device,
                               libxl__xs_kvs_of_flexarray(gc, back, back->count),
                               libxl__xs_kvs_of_flexarray(gc, front, front->count),
                               libxl__xs_kvs_of_flexarray(gc, ro_front, ro_front->count));
@@ -3578,6 +3596,221 @@ out:
 
 /******************************************************************************/
 
+int libxl__init_console_from_channel(libxl__gc *gc,
+                                     libxl__device_console *console,
+                                     int dev_num,
+                                     libxl_device_channel *channel)
+{
+    int rc;
+
+    libxl__device_console_init(console);
+
+    /* Perform validation first, allocate second. */
+
+    if (!channel->name) {
+        LOG(ERROR, "channel %d has no name", channel->devid);
+        return ERROR_INVAL;
+    }
+
+    if (channel->backend_domname) {
+        rc = libxl_domain_qualifier_to_domid(CTX, channel->backend_domname,
+                                             &channel->backend_domid);
+        if (rc < 0) return rc;
+    }
+
+    /* The xenstore 'output' node tells the backend what to connect the console
+       to. If the channel has "connection = pty" then the "output" node will be
+       set to "pty". If the channel has "connection = socket" then the "output"
+       node will be set to "chardev:libxl-channel%d". This tells the qemu
+       backend to proxy data between the console ring and the character device
+       with id "libxl-channel%d". These character devices are currently defined
+       on the qemu command-line via "-chardev" options in libxl_dm.c */
+
+    switch (channel->connection) {
+        case LIBXL_CHANNEL_CONNECTION_UNKNOWN:
+            LOG(ERROR, "channel %d has no defined connection; "
+                "to where should it be connected?", channel->devid);
+            return ERROR_INVAL;
+        case LIBXL_CHANNEL_CONNECTION_PTY:
+            console->connection = libxl__strdup(NOGC, "pty");
+            console->output = libxl__sprintf(NOGC, "pty");
+            break;
+        case LIBXL_CHANNEL_CONNECTION_SOCKET:
+            if (!channel->u.socket.path) {
+                LOG(ERROR, "channel %d has no path", channel->devid);
+                return ERROR_INVAL;
+            }
+            console->connection = libxl__strdup(NOGC, "socket");
+            console->path = libxl__strdup(NOGC, channel->u.socket.path);
+            console->output = libxl__sprintf(NOGC, "chardev:libxl-channel%d",
+                                             channel->devid);
+            break;
+        default:
+            /* We've forgotten to add the clause */
+            LOG(ERROR, "%s: missing implementation for channel connection %d",
+                __func__, channel->connection);
+            abort();
+    }
+
+    console->devid = dev_num;
+    console->consback = LIBXL__CONSOLE_BACKEND_IOEMU;
+    console->backend_domid = channel->backend_domid;
+    console->name = libxl__strdup(NOGC, channel->name);
+
+    return 0;
+}
+
+static int libxl__device_channel_from_xs_be(libxl__gc *gc,
+                                            const char *be_path,
+                                            libxl_device_channel *channel)
+{
+    const char *tmp;
+    int rc;
+
+    libxl_device_channel_init(channel);
+
+    /* READ_BACKEND is from libxl__device_nic_from_xs_be above */
+    channel->name = READ_BACKEND(NOGC, "name");
+    tmp = READ_BACKEND(gc, "connection");
+    if (!strcmp(tmp, "pty")) {
+        channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
+    } else if (!strcmp(tmp, "socket")) {
+        channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
+        channel->u.socket.path = READ_BACKEND(NOGC, "path");
+    } else {
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    rc = 0;
+ out:
+    return rc;
+}
+
+static int libxl__append_channel_list_of_type(libxl__gc *gc,
+                                              uint32_t domid,
+                                              const char *type,
+                                              libxl_device_channel **channels,
+                                              int *nchannels)
+{
+    char *fe_path = NULL, *be_path = NULL;
+    char **dir = NULL;
+    unsigned int n = 0, devid = 0;
+    libxl_device_channel *next = NULL;
+    int rc = 0, i;
+
+    fe_path = GCSPRINTF("%s/device/%s",
+                        libxl__xs_get_dompath(gc, domid), type);
+    dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &n);
+    if (!dir || !n)
+      goto out;
+
+    for (i = 0; i < n; i++) {
+        const char *p, *name;
+        libxl_device_channel *tmp;
+
+        p = libxl__sprintf(gc, "%s/%s", fe_path, dir[i]);
+        name = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/name", p));
+        /* 'channels' are consoles with names, so ignore all consoles
+           without names */
+        if (!name) continue;
+        be_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/backend", p));
+        tmp = realloc(*channels,
+                      sizeof(libxl_device_channel) * (*nchannels + devid + 1));
+        if (!tmp) {
+          rc = ERROR_NOMEM;
+          goto out;
+        }
+        *channels = tmp;
+        next = *channels + *nchannels + devid;
+        rc = libxl__device_channel_from_xs_be(gc, be_path, next);
+        if (rc) goto out;
+        next->devid = devid;
+        devid++;
+    }
+    *nchannels += devid;
+    return 0;
+
+ out:
+    return rc;
+}
+
+libxl_device_channel *libxl_device_channel_list(libxl_ctx *ctx,
+                                                uint32_t domid,
+                                                int *num)
+{
+    GC_INIT(ctx);
+    libxl_device_channel *channels = NULL;
+    int rc;
+
+    *num = 0;
+
+    rc = libxl__append_channel_list_of_type(gc, domid, "console", &channels, num);
+    if (rc) goto out_err;
+
+    GC_FREE;
+    return channels;
+
+out_err:
+    LOG(ERROR, "Unable to list channels");
+    while (*num) {
+        (*num)--;
+        libxl_device_channel_dispose(&channels[*num]);
+    }
+    free(channels);
+    return NULL;
+}
+
+int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
+                                 libxl_device_channel *channel,
+                                 libxl_channelinfo *channelinfo)
+{
+    GC_INIT(ctx);
+    char *dompath, *fe_path;
+    char *val;
+
+    dompath = libxl__xs_get_dompath(gc, domid);
+    channelinfo->devid = channel->devid;
+
+    fe_path = libxl__sprintf(gc, "%s/device/console/%d", dompath,
+                             channelinfo->devid + 1);
+    channelinfo->backend = xs_read(ctx->xsh, XBT_NULL,
+                                   libxl__sprintf(gc, "%s/backend",
+                                   fe_path), NULL);
+    if (!channelinfo->backend) {
+        GC_FREE;
+        return ERROR_FAIL;
+    }
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/backend-id", fe_path));
+    channelinfo->backend_id = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", fe_path));
+    channelinfo->state = val ? strtoul(val, NULL, 10) : -1;
+    channelinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
+                                    GCSPRINTF("%s/frontend",
+                                    channelinfo->backend), NULL);
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/frontend-id",
+                         channelinfo->backend));
+    channelinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/ring-ref", fe_path));
+    channelinfo->rref = val ? strtoul(val, NULL, 10) : -1;
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/port", fe_path));
+    channelinfo->evtch = val ? strtoul(val, NULL, 10) : -1;
+
+    channelinfo->connection = channel->connection;
+    switch (channel->connection) {
+         case LIBXL_CHANNEL_CONNECTION_PTY:
+             val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/tty", fe_path));
+             channelinfo->u.pty.path = strdup(val);
+             break;
+         default:
+             break;
+    }
+    GC_FREE;
+    return 0;
+}
+
+/******************************************************************************/
+
 int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
 {
     int rc;
@@ -3842,6 +4075,10 @@ DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
 DEFINE_DEVICE_REMOVE(vtpm, remove, 0)
 DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
 
+/* channel/console hotunplug is not implemented. There are 2 possibilities:
+ * 1. add support for secondary consoles to xenconsoled
+ * 2. dynamically add/remove qemu chardevs via qmp messages. */
+
 #undef DEFINE_DEVICE_REMOVE
 
 /******************************************************************************/
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index bc68cac..300e489 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -583,6 +583,15 @@ typedef struct libxl__ctx libxl_ctx;
  */
 #define LIBXL_HAVE_BUILDINFO_KERNEL 1
 
+/*
+ * LIBXL_HAVE_DEVICE_CHANNEL
+ *
+ * If this is defined, then the libxl_device_channel struct exists
+ * and channels can be attached to a domain. Channels manifest as consoles
+ * with names, see docs/misc/console.txt.
+ */
+#define LIBXL_HAVE_DEVICE_CHANNEL 1
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */
@@ -1129,6 +1138,17 @@ libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num
 int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
                               libxl_device_nic *nic, libxl_nicinfo *nicinfo);
 
+/*
+ * Virtual Channels
+ * Channels manifest as consoles with names, see docs/misc/channels.txt
+ */
+libxl_device_channel *libxl_device_channel_list(libxl_ctx *ctx,
+                                                uint32_t domid,
+                                                int *num);
+int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
+                                 libxl_device_channel *channel,
+                                 libxl_channelinfo *channelinfo);
+
 /* Virtual TPMs */
 int libxl_device_vtpm_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vtpm *vtpm,
                           const libxl_asyncop_how *ao_how)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 543c1b3..dc01fcd 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -387,15 +387,16 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
     return 0;
 }
 
-static int init_console_info(libxl__device_console *console, int dev_num)
+static void init_console_info(libxl__gc *gc,
+                             libxl__device_console *console,
+                             int dev_num)
 {
     libxl__device_console_init(console);
     console->devid = dev_num;
     console->consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
-    console->output = strdup("pty");
-    if (!console->output)
-        return ERROR_NOMEM;
-    return 0;
+    console->output = libxl__strdup(NOGC, "pty");
+    /* console->{name,connection,path} are NULL on normal consoles.
+       Only 'channels' when mapped to consoles have a string name. */
 }
 
 int libxl__domain_build(libxl__gc *gc,
@@ -1194,17 +1195,31 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         }
     }
 
+    /* For both HVM and PV the 0th console is a regular console. We
+       map channels to IOEMU consoles starting at 1 */
+    for (i = 0; i < d_config->num_channels; i++) {
+        libxl__device_console console;
+        libxl__device device;
+        ret = libxl__init_console_from_channel(gc, &console, i + 1,
+                                               &d_config->channels[i]);
+        if ( ret ) {
+            libxl__device_console_dispose(&console);
+            goto error_out;
+        }
+        libxl__device_console_add(gc, domid, &console, NULL, &device);
+        libxl__device_console_dispose(&console);
+    }
+
     switch (d_config->c_info.type) {
     case LIBXL_DOMAIN_TYPE_HVM:
     {
         libxl__device_console console;
+        libxl__device device;
         libxl_device_vkb vkb;
 
-        ret = init_console_info(&console, 0);
-        if ( ret )
-            goto error_out;
+        init_console_info(gc, &console, 0);
         console.backend_domid = state->console_domid;
-        libxl__device_console_add(gc, domid, &console, state);
+        libxl__device_console_add(gc, domid, &console, state, &device);
         libxl__device_console_dispose(&console);
 
         libxl_device_vkb_init(&vkb);
@@ -1231,22 +1246,22 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
     {
         int need_qemu = 0;
         libxl__device_console console;
+        libxl__device device;
 
         for (i = 0; i < d_config->num_vfbs; i++) {
             libxl__device_vfb_add(gc, domid, &d_config->vfbs[i]);
             libxl__device_vkb_add(gc, domid, &d_config->vkbs[i]);
         }
 
-        ret = init_console_info(&console, 0);
-        if ( ret )
-            goto error_out;
+        init_console_info(gc, &console, 0);
 
         need_qemu = libxl__need_xenpv_qemu(gc, 1, &console,
                 d_config->num_vfbs, d_config->vfbs,
-                d_config->num_disks, &d_config->disks[0]);
+                d_config->num_disks, &d_config->disks[0],
+                d_config->num_channels, &d_config->channels[0]);
 
         console.backend_domid = state->console_domid;
-        libxl__device_console_add(gc, domid, &console, state);
+        libxl__device_console_add(gc, domid, &console, state, &device);
         libxl__device_console_dispose(&console);
 
         if (need_qemu) {
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index fbc82fd..dc748be 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -416,8 +416,9 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     const libxl_sdl_info *sdl = dm_sdl(guest_config);
     const char *keymap = dm_keymap(guest_config);
     flexarray_t *dm_args;
-    int i;
+    int i, connection, devid;
     uint64_t ram_size;
+    const char *path, *chardev;
 
     dm_args = flexarray_make(gc, 16, 1);
 
@@ -434,6 +435,28 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     flexarray_append(dm_args, "-mon");
     flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
 
+    for (i = 0; i < guest_config->num_channels; i++) {
+        connection = guest_config->channels[i].connection;
+        devid = guest_config->channels[i].devid;
+        switch (connection) {
+            case LIBXL_CHANNEL_CONNECTION_PTY:
+                chardev = GCSPRINTF("pty,id=libxl-channel%d", devid);
+                break;
+            case LIBXL_CHANNEL_CONNECTION_SOCKET:
+                path = guest_config->channels[i].u.socket.path;
+                chardev = GCSPRINTF("socket,id=libxl-channel%d,path=%s,"
+                                    "server,nowait", devid, path);
+                break;
+            default:
+                /* We've forgotten to add the clause */
+                LOG(ERROR, "%s: unknown channel connection %d",
+                    __func__, connection);
+                return NULL;
+        }
+        flexarray_append(dm_args, "-chardev");
+        flexarray_append(dm_args, (void*)chardev);
+    }
+
     /*
      * Remove default devices created by qemu. Qemu will create only devices
      * defined by xen, since the devices not defined by xen are not usable.
@@ -1116,6 +1139,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
     }
 
     for (i = 0; i < num_console; i++) {
+        libxl__device device;
         console[i].devid = i;
         console[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU;
         /* STUBDOM_CONSOLE_LOGGING (console 0) is for minios logging
@@ -1146,7 +1170,8 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
                 break;
         }
         ret = libxl__device_console_add(gc, dm_domid, &console[i],
-                        i == STUBDOM_CONSOLE_LOGGING ? stubdom_state : NULL);
+                        i == STUBDOM_CONSOLE_LOGGING ? stubdom_state : NULL,
+                        &device);
         if (ret)
             goto out;
     }
@@ -1566,7 +1591,8 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
 int libxl__need_xenpv_qemu(libxl__gc *gc,
         int nr_consoles, libxl__device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
-        int nr_disks, libxl_device_disk *disks)
+        int nr_disks, libxl_device_disk *disks,
+        int nr_channels, libxl_device_channel *channels)
 {
     int i, ret = 0;
     uint32_t domid;
@@ -1606,6 +1632,20 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
         }
     }
 
+    if (nr_channels > 0) {
+        ret = libxl__get_domid(gc, &domid);
+        if (ret) goto out;
+        for (i = 0; i < nr_channels; i++) {
+            if (channels[i].backend_domid == domid) {
+                /* xenconsoled is limited to the first console only.
+                   Until this restriction is removed we must use qemu for
+                   secondary consoles which includes all channels. */
+                ret = 1;
+                goto out;
+            }
+        }
+    }
+
 out:
     return ret;
 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f61673c..151c474 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1033,7 +1033,8 @@ _hidden int libxl__device_disk_dev_number(const char *virtpath,
 
 _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                                       libxl__device_console *console,
-                                      libxl__domain_build_state *state);
+                                      libxl__domain_build_state *state,
+                                      libxl__device *device);
 
 /* Returns 1 if device exists, 0 if not, ERROR_* (<0) on error. */
 _hidden int libxl__device_exists(libxl__gc *gc, xs_transaction_t t,
@@ -1049,6 +1050,10 @@ _hidden int libxl__wait_for_backend(libxl__gc *gc, const char *be_path,
                                     const char *state);
 _hidden int libxl__nic_type(libxl__gc *gc, libxl__device *dev,
                             libxl_nic_type *nictype);
+_hidden int libxl__init_console_from_channel(libxl__gc *gc,
+                                             libxl__device_console *console,
+                                             int dev_num,
+                                             libxl_device_channel *channel);
 
 /*
  * For each aggregate type which can be used as an input we provide:
@@ -1468,7 +1473,8 @@ _hidden const char *libxl__domain_device_model(libxl__gc *gc,
 _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
         int nr_consoles, libxl__device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
-        int nr_disks, libxl_device_disk *disks);
+        int nr_disks, libxl_device_disk *disks,
+        int nr_channels, libxl_device_channel *channels);
 
 /*
  * This function will cause the whole libxl process to hang
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f1fcbc3..59c3d0b 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -69,6 +69,12 @@ libxl_domain_type = Enumeration("domain_type", [
     (2, "PV"),
     ], init_val = "LIBXL_DOMAIN_TYPE_INVALID")
 
+libxl_channel_connection = Enumeration("channel_connection", [
+    (0, "UNKNOWN"),
+    (1, "PTY"),
+    (2, "SOCKET"), # a listening Unix domain socket
+    ])
+
 libxl_device_model_version = Enumeration("device_model_version", [
     (0, "UNKNOWN"),
     (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
@@ -269,6 +275,22 @@ libxl_cpupoolinfo = Struct("cpupoolinfo", [
     ("cpumap",      libxl_bitmap)
     ], dir=DIR_OUT)
 
+libxl_channelinfo = Struct("channelinfo", [
+    ("backend", string),
+    ("backend_id", uint32),
+    ("frontend", string),
+    ("frontend_id", uint32),
+    ("devid", libxl_devid),
+    ("state", integer),
+    ("evtch", integer),
+    ("rref", integer),
+    ("u", KeyedUnion(None, libxl_channel_connection, "connection",
+           [("unknown", None),
+            ("pty", Struct(None, [("path", string),])),
+            ("socket", None),
+           ])),
+    ], dir=DIR_OUT)
+
 libxl_vminfo = Struct("vminfo", [
     ("uuid", libxl_uuid),
     ("domid", libxl_domid),
@@ -491,6 +513,18 @@ libxl_device_vtpm = Struct("device_vtpm", [
     ("uuid",             libxl_uuid),
 ])
 
+libxl_device_channel = Struct("device_channel", [
+    ("backend_domid", libxl_domid),
+    ("backend_domname", string),
+    ("devid", libxl_devid),
+    ("name", string),
+    ("u", KeyedUnion(None, libxl_channel_connection, "connection",
+           [("unknown", None),
+            ("pty", None),
+            ("socket", Struct(None, [("path", string)])),
+           ])),
+])
+
 libxl_domain_config = Struct("domain_config", [
     ("c_info", libxl_domain_create_info),
     ("b_info", libxl_domain_build_info),
@@ -501,6 +535,9 @@ libxl_domain_config = Struct("domain_config", [
     ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
     ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
     ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
+    # a channel manifests as a console with a name,
+    # see docs/misc/channels.txt
+    ("channels", Array(libxl_device_channel, "num_channels")),
 
     ("on_poweroff", libxl_action_on_shutdown),
     ("on_reboot", libxl_action_on_shutdown),
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index 800361b..5e55685 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -34,6 +34,11 @@ libxl__device_console = Struct("device_console", [
     ("devid", integer),
     ("consback", libxl__console_backend),
     ("output", string),
+    # A regular console has no name.
+    # A console with a name is called a 'channel', see docs/misc/channels.txt
+    ("name", string),
+    ("connection", string),
+    ("path", string),
     ])
 
 libxl__device_action = Enumeration("device_action", [
-- 
1.7.10.4

[PATCH v7 for-4.5 3/7] xl: add 'xstrdup' next to 'xrealloc'

[David Scott]

Signed-off-by: David Scott <dave.scott@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 tools/libxl/xl_cmdimpl.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 698b3bc..7b87298 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -300,6 +300,19 @@ static void *xrealloc(void *ptr, size_t sz) {
     return r;
 }
 
+static char *xstrdup(const char *x) __attribute__ ((unused));
+static char *xstrdup(const char *x)
+{
+    char *r;
+    r = strdup(x);
+    if (!r) {
+        fprintf(stderr, "xl: Unable to strdup a string of length %zu.\n",
+                strlen(x));
+        exit(-ERROR_FAIL);
+    }
+    return r;
+}
+
 #define ARRAY_EXTEND_INIT(array,count,initfn)                           \
     ({                                                                  \
         typeof((count)) array_extend_old_count = (count);               \
-- 
1.7.10.4

[PATCH v7 for-4.5 4/7] xl: move 'replace_string' further up the file

[David Scott]

This allows the function to be reused more easily.

Signed-off-by: David Scott <dave.scott@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 tools/libxl/xl_cmdimpl.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 7b87298..1fc2171 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -812,6 +812,12 @@ static void parse_vcpu_affinity(libxl_domain_build_info *b_info,
     }
 }
 
+static void replace_string(char **str, const char *val)
+{
+    free(*str);
+    *str = strdup(val);
+}
+
 static void parse_config_data(const char *config_source,
                               const char *config_data,
                               int config_len,
@@ -1927,13 +1933,6 @@ static int match_option_size(const char *prefix, size_t len,
 #define MATCH_OPTION(prefix, arg, oparg) \
     match_option_size((prefix "="), sizeof((prefix)), (arg), &(oparg))
 
-static void replace_string(char **str, const char *val)
-{
-    free(*str);
-    *str = strdup(val);
-}
-
-
 /* Preserve a copy of a domain under a new name. Updates *r_domid */
 static int preserve_domain(uint32_t *r_domid, libxl_event *event,
                            libxl_domain_config *d_config)
-- 
1.7.10.4

[PATCH v7 for-4.5 5/7] xl: 'replace_string' now uses xstrdup

[David Scott]

This catches the out-of-memory exception and exits the program.

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1fc2171..e5d20e6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -815,7 +815,7 @@ static void parse_vcpu_affinity(libxl_domain_build_info *b_info,
 static void replace_string(char **str, const char *val)
 {
     free(*str);
-    *str = strdup(val);
+    *str = xstrdup(val);
 }
 
 static void parse_config_data(const char *config_source,
-- 
1.7.10.4

[PATCH v7 for-4.5 6/7] xl: add 'trim' and 'split_string_into_pair' functions

[David Scott]

Signed-off-by: David Scott <dave.scott@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 tools/libxl/xl_cmdimpl.c |   68 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e5d20e6..af294e2 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -300,7 +300,6 @@ static void *xrealloc(void *ptr, size_t sz) {
     return r;
 }
 
-static char *xstrdup(const char *x) __attribute__ ((unused));
 static char *xstrdup(const char *x)
 {
     char *r;
@@ -564,6 +563,73 @@ static void split_string_into_string_list(const char *str,
     free(s);
 }
 
+/* NB: this follows the interface used by <ctype.h>. See 'man 3 ctype'
+   and look for CTYPE in libxl_internal.h */
+typedef int (*char_predicate_t)(const int c);
+
+static void trim(char_predicate_t predicate, const char *input, char **output) __attribute__ ((unused));
+static void trim(char_predicate_t predicate, const char *input, char **output)
+{
+    char *p, *q, *tmp;
+
+    *output = NULL;
+    if (*input == '\000')
+        return;
+    /* Input has length >= 1 */
+
+    p = tmp = xstrdup(input);
+    /* Skip past the characters for which predicate is true */
+    while ((*p != '\000') && (predicate((unsigned char)*p)))
+        p ++;
+    q = p + strlen(p) - 1;
+    /* q points to the last non-NULL character */
+    while ((q > p) && (predicate((unsigned char)*q)))
+        q --;
+    /* q points to the last character we want */
+    q ++;
+    *q = '\000';
+    *output = xstrdup(p);
+    free(tmp);
+}
+
+static int split_string_into_pair(const char *str,
+                                  const char *delim,
+                                  char **a,
+                                  char **b) __attribute__ ((unused));
+static int split_string_into_pair(const char *str,
+                                  const char *delim,
+                                  char **a,
+                                  char **b)
+{
+    char *s, *p, *saveptr, *aa = NULL, *bb = NULL;
+    int rc = 0;
+
+    s = xstrdup(str);
+
+    p = strtok_r(s, delim, &saveptr);
+    if (p == NULL) {
+        rc = ERROR_INVAL;
+        goto out;
+    }
+    aa = xstrdup(p);
+    p = strtok_r(NULL, delim, &saveptr);
+    if (p == NULL) {
+        rc = ERROR_INVAL;
+        goto out;
+    }
+    bb = xstrdup(p);
+
+    *a = aa;
+    aa = NULL;
+    *b = bb;
+    bb = NULL;
+out:
+    free(s);
+    free(aa);
+    free(bb);
+    return rc;
+}
+
 static int parse_range(const char *str, unsigned long *a, unsigned long *b)
 {
     const char *nstr;
-- 
1.7.10.4

[PATCH v7 for-4.5 7/7] xl: add support for 'channels'

[David Scott]

This adds support for channel declarations of the form:

 channel = [ "name=...,connection=...[,path=...][,backend=...]" ]

where 'name' is a label to identify the channel to the frontend.

If 'connection = pty' then the channel is connected to a pty in the
 backend domain
If 'connection = socket' then the channel is connected to a Unix domain
 socket given by 'path = ...' in the backend domain.

This patch also adds the command:

 xl channel-list <domain>

which allows the state of channels to be queried. In particular if
'connection=pty' this will show the path of the pty slave device.

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 docs/man/xl.cfg.pod.5     |   53 ++++++++++++++++++
 docs/man/xl.pod.1         |   10 ++++
 tools/libxl/xl.h          |    1 +
 tools/libxl/xl_cmdimpl.c  |  133 ++++++++++++++++++++++++++++++++++++++++++---
 tools/libxl/xl_cmdtable.c |    5 ++
 5 files changed, 194 insertions(+), 8 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 517ae2f..2c53b5c 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -530,6 +530,59 @@ L<qemu(1)> manpage. The default is B<en-us>.
 
 =back
 
+=item B<channel=[ "CHANNEL_SPEC_STRING", "CHANNEL_SPEC_STRING", ...]>
+
+Specifies the virtual channels to be provided to the guest. A
+channel is a low-bandwidth, bidirectional byte stream, which resembles
+a serial link. Typical uses for channels include transmitting VM
+configuration after boot and signalling to in-guest agents. Please see
+F<docs/misc/channels.txt> for more details.
+
+Each B<CHANNEL_SPEC_STRING> is a comma-separated list of C<KEY=VALUE>
+seettings. Leading and trailing whitespace is ignored in both KEY and
+VALUE. Neither KEY nor VALUE may contain ',', '=' or '"'. Defined values
+are:
+
+=over 4
+
+=item C<backend=DOMAIN>
+
+Specify the backend domain name or id. This parameter is optional. If
+this parameter is omitted then the toolstack domain will be assumed.
+
+=item C<name=NAME>
+
+Specify the string name for this device. This parameter is mandatory.
+This should be a well-known name for the specific application (e.g.
+guest agent) and should be used by the frontend to connect the
+application to the right channel device. There is no formal registry
+of channel names, so application authors are encouraged to make their
+names unique by including domain name and version number in the string
+(e.g. org.mydomain.guestagent.1).
+
+=item C<connection=CONNECTION>
+
+Specify how the backend will be implemented. This following options are
+available:
+
+=over 4
+
+=item B<connection=SOCKET>
+
+The backend will bind a Unix domain socket (at the path given by
+B<path=PATH>), call listen and accept connections. The backend will proxy
+data between the channel and the connected socket.
+
+=item B<connection=PTY>
+
+The backend will create a pty and proxy data between the channel and the
+master device. The command B<xl channel-list> can be used to discover the
+assigned slave device.
+
+=back
+
+=back
+
 =item B<pci=[ "PCI_SPEC_STRING", "PCI_SPEC_STRING", ... ]>
 
 Specifies the host PCI devices to passthrough to this guest. Each B<PCI_SPEC_STRING>
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index f1e95db..35aa3a4 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1215,6 +1215,16 @@ List virtual network interfaces for a domain.
 
 =back
 
+=head2 CHANNEL DEVICES
+
+=over 4
+
+=item B<channel-list> I<domain-id>
+
+List virtual channel interfaces for a domain.
+
+=back
+
 =head2 VTPM DEVICES
 
 =over 4
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 10a2e66..afc5f8b 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -78,6 +78,7 @@ int main_top(int argc, char **argv);
 int main_networkattach(int argc, char **argv);
 int main_networklist(int argc, char **argv);
 int main_networkdetach(int argc, char **argv);
+int main_channellist(int argc, char **argv);
 int main_blockattach(int argc, char **argv);
 int main_blocklist(int argc, char **argv);
 int main_blockdetach(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index af294e2..359cab3 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -530,7 +530,7 @@ static void split_string_into_string_list(const char *str,
 
     s = strdup(str);
     if (s == NULL) {
-        fprintf(stderr, "unable to allocate memory to parse bootloader args\n");
+        fprintf(stderr, "unable to allocate memory to split string\n");
         exit(-1);
     }
 
@@ -546,7 +546,7 @@ static void split_string_into_string_list(const char *str,
 
     sl = malloc((nr+1) * sizeof (char *));
     if (sl == NULL) {
-        fprintf(stderr, "unable to allocate memory for bootloader args\n");
+        fprintf(stderr, "unable to allocate memory to split string\n");
         exit(-1);
     }
 
@@ -567,7 +567,6 @@ static void split_string_into_string_list(const char *str,
    and look for CTYPE in libxl_internal.h */
 typedef int (*char_predicate_t)(const int c);
 
-static void trim(char_predicate_t predicate, const char *input, char **output) __attribute__ ((unused));
 static void trim(char_predicate_t predicate, const char *input, char **output)
 {
     char *p, *q, *tmp;
@@ -595,10 +594,6 @@ static void trim(char_predicate_t predicate, const char *input, char **output)
 static int split_string_into_pair(const char *str,
                                   const char *delim,
                                   char **a,
-                                  char **b) __attribute__ ((unused));
-static int split_string_into_pair(const char *str,
-                                  const char *delim,
-                                  char **a,
                                   char **b)
 {
     char *s, *p, *saveptr, *aa = NULL, *bb = NULL;
@@ -893,7 +888,7 @@ static void parse_config_data(const char *config_source,
     long l;
     XLU_Config *config;
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
-    XLU_ConfigList *ioports, *irqs, *iomem;
+    XLU_ConfigList *channels, *ioports, *irqs, *iomem;
     int num_ioports, num_irqs, num_iomem, num_cpus;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
@@ -1378,6 +1373,84 @@ static void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_list (config, "channel", &channels, 0, 0)) {
+        d_config->num_channels = 0;
+        d_config->channels = NULL;
+        while ((buf = xlu_cfg_get_listitem (channels,
+                d_config->num_channels)) != NULL) {
+            libxl_device_channel *chn;
+            libxl_string_list pairs;
+            char *path = NULL;
+            int len;
+
+            chn = ARRAY_EXTEND_INIT(d_config->channels, d_config->num_channels,
+                                   libxl_device_channel_init);
+
+            split_string_into_string_list(buf, ",", &pairs);
+            len = libxl_string_list_length(&pairs);
+            for (i = 0; i < len; i++) {
+                char *key, *key_untrimmed, *value, *value_untrimmed;
+                int rc;
+                rc = split_string_into_pair(pairs[i], "=",
+                                            &key_untrimmed,
+                                            &value_untrimmed);
+                if (rc != 0) {
+                    fprintf(stderr, "failed to parse channel configuration: %s",
+                            pairs[i]);
+                    exit(1);
+                }
+                trim(isspace, key_untrimmed, &key);
+                trim(isspace, value_untrimmed, &value);
+
+                if (!strcmp(key, "backend")) {
+                    replace_string(&chn->backend_domname, value);
+                } else if (!strcmp(key, "name")) {
+                    replace_string(&chn->name, value);
+                } else if (!strcmp(key, "path")) {
+                    replace_string(&path, value);
+                } else if (!strcmp(key, "connection")) {
+                    if (!strcmp(value, "pty")) {
+                        chn->connection = LIBXL_CHANNEL_CONNECTION_PTY;
+                    } else if (!strcmp(value, "socket")) {
+                        chn->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
+                    } else {
+                        fprintf(stderr, "unknown channel connection '%s'\n",
+                                value);
+                        exit(1);
+                    }
+                } else {
+                    fprintf(stderr, "unknown channel parameter '%s',"
+                                  " ignoring\n", key);
+                }
+                free(key);
+                free(key_untrimmed);
+                free(value);
+                free(value_untrimmed);
+            }
+            switch (chn->connection) {
+            case LIBXL_CHANNEL_CONNECTION_UNKNOWN:
+                fprintf(stderr, "channel has unknown 'connection'\n");
+                exit(1);
+            case LIBXL_CHANNEL_CONNECTION_SOCKET:
+                if (!path) {
+                    fprintf(stderr, "channel connection 'socket' requires path=..\n");
+                    exit(1);
+                }
+                chn->u.socket.path = xstrdup(path);
+                break;
+            case LIBXL_CHANNEL_CONNECTION_PTY:
+                /* Nothing to do since PTY has no arguments */
+                break;
+            default:
+                fprintf(stderr, "unknown channel connection: %d",
+                        chn->connection);
+                exit(1); 
+            }
+            libxl_string_list_dispose(&pairs);
+            free(path);
+        }
+    }
+
     if (!xlu_cfg_get_list (config, "vif", &nics, 0, 0)) {
         d_config->num_nics = 0;
         d_config->nics = NULL;
@@ -6047,6 +6120,50 @@ int main_networkdetach(int argc, char **argv)
     return 0;
 }
 
+int main_channellist(int argc, char **argv)
+{
+    int opt;
+    libxl_device_channel *channels;
+    libxl_channelinfo channelinfo;
+    int nb, i;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "channel-list", 1) {
+        /* No options */
+    }
+
+    /*      Idx BE state evt-ch ring-ref connection params*/
+    printf("%-3s %-2s %-5s %-6s %8s %-10s %-30s\n",
+           "Idx", "BE", "state", "evt-ch", "ring-ref", "connection", "");
+    for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
+        uint32_t domid = find_domain(*argv);
+        channels = libxl_device_channel_list(ctx, domid, &nb);
+        if (!channels)
+            continue;
+        for (i = 0; i < nb; ++i) {
+            if (!libxl_device_channel_getinfo(ctx, domid, &channels[i],
+                &channelinfo)) {
+                printf("%-3d %-2d ", channels[i].devid, channelinfo.backend_id);
+                printf("%-5d ", channelinfo.state);
+                printf("%-6d %-8d ", channelinfo.evtch, channelinfo.rref);
+                printf("%-10s ", libxl_channel_connection_to_string(
+                       channels[i].connection));
+                switch (channels[i].connection) {
+                    case LIBXL_CHANNEL_CONNECTION_PTY:
+                        printf("%-30s ", channelinfo.u.pty.path);
+                        break;
+                    default:
+                        break;
+                }
+                printf("\n");
+                libxl_channelinfo_dispose(&channelinfo);
+            }
+            libxl_device_channel_dispose(&channels[i]);
+        }
+        free(channels);
+    }
+    return 0;
+}
+
 int main_blockattach(int argc, char **argv)
 {
     int opt;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 35f02c4..a6df234 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -337,6 +337,11 @@ struct cmd_spec cmd_table[] = {
       "Destroy a domain's virtual network device",
       "<Domain> <DevId|mac>",
     },
+    { "channel-list",
+      &main_channellist, 0, 0,
+      "List virtual channel devices for a domain",
+      "<Domain(s)>",
+    },
     { "block-attach",
       &main_blockattach, 1, 1,
       "Create a new virtual block device",
-- 
1.7.10.4

Re: [PATCH v7 for-4.5 1/7] libxl: replace memset() with libxl__device_console_init

[Wei Liu]

On Thu, Oct 09, 2014 at 10:17:26AM +0100, David Scott wrote:
> The current implementation of libxl__device_console_init does the same
> memset() so this is identical for now.
> 
> If libxl__device_console_init changes in future (e.g. to prefer a non-zero
> default value for some field) then this will continue to work.
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/libxl/libxl_create.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 8b82584..543c1b3 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -389,7 +389,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>  
>  static int init_console_info(libxl__device_console *console, int dev_num)
>  {
> -    memset(console, 0x00, sizeof(libxl__device_console));
> +    libxl__device_console_init(console);
>      console->devid = dev_num;
>      console->consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
>      console->output = strdup("pty");
> -- 
> 1.7.10.4

Re: [PATCH v7 for-4.5 4/7] xl: move 'replace_string' further up the file

[Wei Liu]

On Thu, Oct 09, 2014 at 10:17:29AM +0100, David Scott wrote:
> This allows the function to be reused more easily.
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH v7 for-4.5 5/7] xl: 'replace_string' now uses xstrdup

[Wei Liu]

On Thu, Oct 09, 2014 at 10:17:30AM +0100, David Scott wrote:
> This catches the out-of-memory exception and exits the program.
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH v7 for-4.5 6/7] xl: add 'trim' and 'split_string_into_pair' functions

[Wei Liu]

On Thu, Oct 09, 2014 at 10:17:31AM +0100, David Scott wrote:
> Signed-off-by: David Scott <dave.scott@citrix.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH v7 for-4.5]: xl/libxl: add support for 'channels'

[Wei Liu]

All the mechanical changes look good to me and I've acked all of them. I
will defer the rest to Ian J.

Wei.

Re: [PATCH v7 for-4.5 2/7] libxl: add support for 'channels'

[Ian Jackson]

David Scott writes ("[PATCH v7 for-4.5 2/7] libxl: add support for 'channels'"):
> A 'channel':
>   - is a low-bandwidth private communication channel that resembles
>     a physical serial port.
>   - is implemented as a PV console with a well-known string name
>     which is used to hook the channel to the appropriate software
>     in the guest (i.e. some kind of guest agent).
>   - has a backend 'connection' which describes what should happen
>     to the data.
> 
> The following 'connection' types are defined:
> 
>  * PTY: the I/O surfaces as a pty in the backend domain
>  * SOCKET: a listening Unix domain socket accepts a connection in
>    the backend domain and proxies
> 
> Channels may be listed but don't currently support hotplug/unplug.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH v7 for-4.5 7/7] xl: add support for 'channels'

[Ian Jackson]

David Scott writes ("[PATCH v7 for-4.5 7/7] xl: add support for 'channels'"):
> This adds support for channel declarations of the form:
> 
>  channel = [ "name=...,connection=...[,path=...][,backend=...]" ]
> 
> where 'name' is a label to identify the channel to the frontend.
> 
> If 'connection = pty' then the channel is connected to a pty in the
>  backend domain
> If 'connection = socket' then the channel is connected to a Unix domain
>  socket given by 'path = ...' in the backend domain.
> 
> This patch also adds the command:
> 
>  xl channel-list <domain>
> 
> which allows the state of channels to be queried. In particular if
> 'connection=pty' this will show the path of the pty slave device.
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH v7 for-4.5]: xl/libxl: add support for 'channels'

[Ian Campbell]

On Thu, 2014-10-09 at 10:17 +0100, David Scott wrote:
> Hi,
> 
> Here's the latest spin of these patches, hopefully with everything
> addressed (mainly memory leaks)

I think this now has all the relevant maintainer acks. What's the
release-ack status of it?

I vaguely recall seeing something in a subthread of the development
status mail, but I can't find it now...

Ian.

Re: [PATCH v7 for-4.5]: xl/libxl: add support for 'channels'

[Dave Scott]

On 14 Oct 2014, at 11:30, Ian Campbell <ian.campbell@citrix.com> wrote:

> On Thu, 2014-10-09 at 10:17 +0100, David Scott wrote:
>> Hi,
>> 
>> Here's the latest spin of these patches, hopefully with everything
>> addressed (mainly memory leaks)
> 
> I think this now has all the relevant maintainer acks. What's the
> release-ack status of it?
> 
> I vaguely recall seeing something in a subthread of the development
> status mail, but I can't find it now…

I think this is the subthread here:

http://lists.xenproject.org/archives/html/xen-devel/2014-09/msg04061.html

Konrad added “Reviewed-by:” to some of the patches (3, 4, 6) and wrote:

> Also, while my Reviewed-by is nice, either of the three maintainers:
> Wei, IanJ, and IanC would need to Review these patches as well.

Konrad, what do you think?

Thanks,
Dave

Re: [PATCH v7 for-4.5]: xl/libxl: add support for 'channels'

[Konrad Rzeszutek Wilk]

On Tue, Oct 14, 2014 at 10:45:28AM +0000, Dave Scott wrote:
> 
> On 14 Oct 2014, at 11:30, Ian Campbell <ian.campbell@citrix.com> wrote:
> 
> > On Thu, 2014-10-09 at 10:17 +0100, David Scott wrote:
> >> Hi,
> >> 
> >> Here's the latest spin of these patches, hopefully with everything
> >> addressed (mainly memory leaks)
> > 
> > I think this now has all the relevant maintainer acks. What's the
> > release-ack status of it?
> > 
> > I vaguely recall seeing something in a subthread of the development
> > status mail, but I can't find it now…
> 
> I think this is the subthread here:
> 
> http://lists.xenproject.org/archives/html/xen-devel/2014-09/msg04061.html
> 
> Konrad added “Reviewed-by:” to some of the patches (3, 4, 6) and wrote:
> 
> > Also, while my Reviewed-by is nice, either of the three maintainers:
> > Wei, IanJ, and IanC would need to Review these patches as well.
> 
> Konrad, what do you think?

The questions of mine had been answered and I believe the regression
vs feature question has been answered too (it is minimal).

And since it has Acks from the maintainers, then
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> 
> Thanks,
> Dave

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v7 for-4.5]: xl/libxl: add support for 'channels'

[Ian Campbell]

On Wed, 2014-10-15 at 16:37 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 14, 2014 at 10:45:28AM +0000, Dave Scott wrote:
> > 
> > On 14 Oct 2014, at 11:30, Ian Campbell <ian.campbell@citrix.com> wrote:
> > 
> > > On Thu, 2014-10-09 at 10:17 +0100, David Scott wrote:
> > >> Hi,
> > >> 
> > >> Here's the latest spin of these patches, hopefully with everything
> > >> addressed (mainly memory leaks)
> > > 
> > > I think this now has all the relevant maintainer acks. What's the
> > > release-ack status of it?
> > > 
> > > I vaguely recall seeing something in a subthread of the development
> > > status mail, but I can't find it now…
> > 
> > I think this is the subthread here:
> > 
> > http://lists.xenproject.org/archives/html/xen-devel/2014-09/msg04061.html
> > 
> > Konrad added “Reviewed-by:” to some of the patches (3, 4, 6) and wrote:
> > 
> > > Also, while my Reviewed-by is nice, either of the three maintainers:
> > > Wei, IanJ, and IanC would need to Review these patches as well.
> > 
> > Konrad, what do you think?
> 
> The questions of mine had been answered and I believe the regression
> vs feature question has been answered too (it is minimal).
> 
> And since it has Acks from the maintainers, then
> Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Applied.

Ian.




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel