[PATCH v2] implement libvirt-like 'channels' via PV consoles

[PATCH v2] implement libvirt-like 'channels' via PV consoles

[David Scott]

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 = [ "type=socket,path=/tmp/mysock,name=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 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>

[PATCH v2 1/6] libxl: add a list of abstract 'channels' to the domain config

[David Scott]

A 'channel' is a low-bandwidth private communication channel that
resembles a physical serial port. Example uses include:

  * providing initial VM configuration without having to use the
    network
  * signalling a guest agent

Every channel has a string 'name' which the VM can use to find
the appropriate handler. Each channel has an implementation 'type'
which currently includes:

  * NONE: reads will block, writes will be thrown away
  * PTY: the I/O surfaces as a pty in the backend domain
  * PATH: writes are appended to a log file in the backend domain
  * SOCKET: a listening Unix domain socket accepts a connection in
    the backend domain and proxies

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 tools/libxl/libxl_types.idl |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 52f1aa9..5671d86 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -51,6 +51,17 @@ libxl_domain_type = Enumeration("domain_type", [
     (2, "PV"),
     ], init_val = -1)
 
+libxl_channel_type = Enumeration("channel_type", [
+    # Connected to nothing:
+    (0, "NONE"),
+    # Connect to a pty in the backend domain:
+    (1, "PTY"),
+    # Spool output to a file in the backend domain:
+    (2, "PATH"),
+    # Listen on a Unix domain socket in the backend domain:
+    (3, "SOCKET"),
+    ])
+
 libxl_device_model_version = Enumeration("device_model_version", [
     (0, "UNKNOWN"),
     (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
@@ -457,6 +468,15 @@ 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),
+    ("type", libxl_channel_type),
+    ("path", string),
+])
+
 libxl_domain_config = Struct("domain_config", [
     ("c_info", libxl_domain_create_info),
     ("b_info", libxl_domain_build_info),
@@ -467,6 +487,7 @@ 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")),
+    ("channels", Array(libxl_device_channel, "num_channels")),
 
     ("on_poweroff", libxl_action_on_shutdown),
     ("on_reboot", libxl_action_on_shutdown),
-- 
1.7.10.4

[PATCH v2 2/6] xl: add support for channels

[David Scott]

This adds support for channel declarations of the form:

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

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

If 'type = none' then the channel is connected to /dev/null
If 'type = pty' then the channel is connected to a pty in the
  backend domain
If 'type = path' then data is read from the channel and written
  to the file given by 'path = ...' in the backend domain.
If 'type = socket' then the channel is connected to a Unix domain
  socket given by 'path = ...' in the backend domain.

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

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5195914..392f1a0 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -736,7 +736,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;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
@@ -1289,6 +1289,62 @@ 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;
+            char *buf2 = strdup(buf);
+            char *p, *p2;
+            chn = ARRAY_EXTEND_INIT(d_config->channels, d_config->num_channels,
+                                    libxl_device_channel_init);
+
+            p = strtok(buf2, ",");
+            if (!p)
+                goto skip_channel;
+            do {
+                while (*p == ' ')
+                    p++;
+                if ((p2 = strchr(p, '=')) == NULL)
+                    break;
+                *p2 = '\0';
+                if (!strcmp(p, "backend")) {
+                    free(chn->backend_domname);
+                    chn->backend_domname = strdup(p2 + 1);
+                } else if (!strcmp(p, "name")) {
+                    free(chn->name);
+                    chn->name = strdup(p2 + 1);
+                } else if (!strcmp(p, "type")) {
+                    if (chn->type != LIBXL_CHANNEL_TYPE_NONE) {
+                        fprintf(stderr, "a channel may have only one output\n");
+                        exit(1);
+                    }
+                    if (!strcmp(p2 + 1, "none")) {
+                        chn->type = LIBXL_CHANNEL_TYPE_NONE;
+                    } else if (!strcmp(p2 + 1, "pty")) {
+                        chn->type = LIBXL_CHANNEL_TYPE_PTY;
+                    } else if (!strcmp(p2 + 1, "path")) {
+                        chn->type = LIBXL_CHANNEL_TYPE_PATH;
+                    } else if (!strcmp(p2 + 1, "socket")) {
+                        chn->type = LIBXL_CHANNEL_TYPE_SOCKET;
+                    } else {
+                        fprintf(stderr, "unknown channel type '%s'\n", p2 + 1);
+                        exit(1);
+                    }
+                } else if (!strcmp(p, "path")) {
+                    free(chn->path);
+                    chn->path = strdup(p2 + 1);
+                } else {
+                    fprintf(stderr, "unknown channel parameter '%s',"
+                                    " ignoring\n", p);
+                }
+            } while ((p = strtok(NULL, ",")) != NULL);
+skip_channel:
+            free(buf2);
+        }
+    }
+
     if (!xlu_cfg_get_list (config, "vif", &nics, 0, 0)) {
         d_config->num_nics = 0;
         d_config->nics = NULL;
-- 
1.7.10.4

[PATCH v2 3/6] libxl: implement channels via PV console rings

[David Scott]

We extend the (internal) console type with a 'name' (string)
which isn't used by the default built-in console 0.

For every channel we create a console, starting at index 1,
by default which is handled by the qemu 'chardev' mechanism
(ie has 'output=chardev:libxl-channel%d' in xenstore)

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 tools/libxl/libxl.c                  |    5 +++
 tools/libxl/libxl_create.c           |   78 +++++++++++++++++++++++++++++++---
 tools/libxl/libxl_types_internal.idl |    1 +
 3 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 900b8d4..51b6572 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3219,6 +3219,11 @@ 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);
+    }
+
     flexarray_append(front, "backend-id");
     flexarray_append(front, libxl__sprintf(gc, "%d", console->backend_domid));
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index d015cf4..6356e55 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -355,17 +355,70 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
     return 0;
 }
 
-static int init_console_info(libxl__device_console *console, int dev_num)
+static int init_console_info(libxl__gc *gc,
+                             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");
-    if (!console->output)
-        return ERROR_NOMEM;
+    console->output = libxl__strdup(NOGC, "pty");
+    /* console->name is NULL on normal consoles. Only 'channels' when mapped
+       to consoles have a string name. */
     return 0;
 }
 
+static int 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);
+    console->devid = dev_num;
+    console->consback = LIBXL__CONSOLE_BACKEND_IOEMU;
+    if (!channel->name){
+        LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+                   "channel %d has no name", channel->devid);
+        return ERROR_INVAL;
+    }
+    console->name = libxl__strdup(NOGC, channel->name);
+
+    if (channel->backend_domname) {
+        rc = libxl_domain_qualifier_to_domid(CTX, channel->backend_domname,
+                                             &channel->backend_domid);
+        if (rc < 0) return rc;
+    }
+
+    console->backend_domid = channel->backend_domid;
+
+    switch (channel->type) {
+        case LIBXL_CHANNEL_TYPE_NONE:
+        case LIBXL_CHANNEL_TYPE_PTY:
+            /* No path is needed */
+            break;
+        case LIBXL_CHANNEL_TYPE_PATH:
+        case LIBXL_CHANNEL_TYPE_SOCKET:
+            if (!channel->path) {
+                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+                           "channel %d has no path", channel->devid);
+                return ERROR_INVAL;
+            }
+            break;
+        default:
+            /* We've forgotten to add the clause */
+            LOG(ERROR, "%s: unknown channel type %d", __func__, channel->type);
+            return ERROR_INVAL;
+    }
+
+    /* Use qemu chardev for every channel */
+    console->output = libxl__sprintf(NOGC, "chardev:libxl-channel%d",
+                                     channel->devid);
+
+    return 0;
+}
+
+
 int libxl__domain_build(libxl__gc *gc,
                         libxl_domain_config *d_config,
                         uint32_t domid,
@@ -1110,13 +1163,24 @@ 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;
+        ret = init_console_from_channel(gc, &console, i + 1, &d_config->channels[i]);
+        if ( ret )
+            goto error_out;
+        libxl__device_console_add(gc, domid, &console, NULL);
+        libxl__device_console_dispose(&console);
+    }
+
     switch (d_config->c_info.type) {
     case LIBXL_DOMAIN_TYPE_HVM:
     {
         libxl__device_console console;
         libxl_device_vkb vkb;
 
-        ret = init_console_info(&console, 0);
+        ret = init_console_info(gc, &console, 0);
         if ( ret )
             goto error_out;
         console.backend_domid = state->console_domid;
@@ -1144,7 +1208,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
             libxl__device_vkb_add(gc, domid, &d_config->vkbs[i]);
         }
 
-        ret = init_console_info(&console, 0);
+        ret = init_console_info(gc, &console, 0);
         if ( ret )
             goto error_out;
 
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index cb9444f..2a509a9 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -32,6 +32,7 @@ libxl__device_console = Struct("device_console", [
     ("devid", integer),
     ("consback", libxl__console_backend),
     ("output", string),
+    ("name", string),
     ])
 
 libxl__device_action = Enumeration("device_action", [
-- 
1.7.10.4

[PATCH v2 4/6] libxl: spawn a qemu to implement channels

[David Scott]

Since every channel is mapped to a console device in xenstore
with 'output=chardev:libxl-channel%d', we need to tell qemu
to create the appropriate chardevs.

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 tools/libxl/libxl_create.c   |    3 ++-
 tools/libxl/libxl_dm.c       |   39 +++++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_internal.h |    3 ++-
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 6356e55..e178672 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1214,7 +1214,8 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
 
         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);
 
         console.backend_domid = state->console_domid;
         libxl__device_console_add(gc, domid, &console, state);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 51ab2bf..32c1362 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -394,8 +394,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, type, devid;
     uint64_t ram_size;
+    const char *path, *chardev;
 
     dm_args = flexarray_make(gc, 16, 1);
 
@@ -412,6 +413,34 @@ 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++) {
+        type = guest_config->channels[i].type;
+        path = guest_config->channels[i].path;
+        devid = guest_config->channels[i].devid;
+        switch (type) {
+            case LIBXL_CHANNEL_TYPE_NONE:
+                chardev = GCSPRINTF("null,id=libxl-channel%d", devid);
+                break;
+            case LIBXL_CHANNEL_TYPE_PTY:
+                chardev = GCSPRINTF("pty,id=libxl-channel%d", devid);
+                break;
+            case LIBXL_CHANNEL_TYPE_PATH:
+                chardev = GCSPRINTF("file,id=libxl-channel%d,path=%s",
+                                    devid, path);
+                break;
+            case LIBXL_CHANNEL_TYPE_SOCKET:
+                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 type %d", __func__, type);
+                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.
@@ -1517,7 +1546,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)
 {
     int i, ret = 0;
     uint32_t domid;
@@ -1557,6 +1587,11 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
         }
     }
 
+    if (nr_channels > 0) {
+        ret = 1;
+        goto out;
+    }
+
 out:
     return ret;
 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 082749e..a3b1f57 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1450,7 +1450,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);
 
 /*
  * This function will cause the whole libxl process to hang
-- 
1.7.10.4

[PATCH v2 5/6] Add a section in xl.cfg(5) describing 'channels'

[David Scott]

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 docs/man/xl.cfg.pod.5 |   63 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a94d037..00f8905 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -470,6 +470,69 @@ 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 bidirectional byte stream, a private connection between
+the backend and the frontend, which resembles a serial port. 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>
+settings, from the following list:
+
+=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 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.
+This parameter is mandatory.
+
+=item C<type=TYPE>
+
+Specify how the backend should manifest. This should be one of the
+following possibilities:
+
+=over 4
+
+=item B<type=NONE>:
+
+The backend should be connected to the equivalent of
+B</dev/null>.
+
+=item B<type=FILE>:
+
+The backend should be connected to a file such that the
+console output is read and spooled to the file (e.g. for logging).
+
+=item B<type=SOCKET>:
+
+The backend should be connected to the equivalent of
+a Unix domain socket.
+
+=item B<type=PTY>:
+
+The backend should be connected to the equivalent of a
+pty.
+
+=back
+
+=item C<path=PATH>
+
+If C<type=FILE> then this path is the file to which the console output
+should be logged. If C<type=SOCKET> then this is the path of the Unix domain
+socket which should be connected to the channel.
+
+=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>
-- 
1.7.10.4

[PATCH v2 6/6] Add a general description of the channel mechanism to docs/misc/

[David Scott]

Signed-off-by: David Scott <dave.scott@citrix.com>
---
 docs/misc/channel.txt |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 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..b5ab385
--- /dev/null
+++ b/docs/misc/channel.txt
@@ -0,0 +1,49 @@
+Xen PV Channels
+------------------------------------------------------------------------
+                                                             David Scott
+                                                   dave.scott@citrix.com
+
+
+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 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.
+
+From the frontend's point of view, a channel is a PV console with a
+name, a where the name can be used to locate the correct device. The
+name is stored in the frontend xenstore directory:
+
+  /local/domain/$DOMID/device/console/$DEVID/name
+
+The frontend can check for this key when the console is hotplugged,
+and handle the device appropriately. For example the frontend could
+spawn a guest agent when a channel with a well-known name is created,
+and still spawn regular getty processes when a normal console is created.
+
+The backend is configured by 2 xenstore keys:
+
+  type = (NONE | FILE | PTY | SOCKET | ... )
+  path = <some path>
+
+In the default implementation where the backend is run in the toolstack
+domain, the qemu "chardev" mechanism is used. This implementation
+interprets the configuration as follows:
+
+  type = NONE: the backend will be connected to /dev/null
+  type = FILE: the channel will be read the the output appended
+    to the file given by 'path'
+  type = PTY: the backend will connect to a PTY like a regular console
+  type = SOCKET: the backend will accept a connection on the Unix
+    domain socket given by 'path' and proxy data to and from the device.
+
+If the implementation is in another domain (for example via a Mirage
+console backend) then the behaviour will be defined by this other domain.
-- 
1.7.10.4

Re: [PATCH v2] implement libvirt-like 'channels' via PV consoles

[Konrad Rzeszutek Wilk]

On Mon, Jun 16, 2014 at 10:49:49AM +0100, David Scott wrote:
> 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 = [ "type=socket,path=/tmp/mysock,name=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).

Are these udev scripts somewhere implemented?

Thanks!

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

Re: [PATCH v2] implement libvirt-like 'channels' via PV consoles

[Dave Scott]

Hi Konrad,

On 16 Jun 2014, at 14:34, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Mon, Jun 16, 2014 at 10:49:49AM +0100, David Scott wrote:
>> 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 = [ "type=socket,path=/tmp/mysock,name=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).
> 
> Are these udev scripts somewhere implemented?

I’ve been playing with these ones:

https://github.com/djs55/mirage-console/tree/master/udev

(ignore the reference to Mirage — you just need the .rules file and the helper script)

Do you think an example udev script like this should live in the tree?

> 
> Thanks!

Cheers,
Dave

> 
>> 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 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
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v2] implement libvirt-like 'channels' via PV consoles

[Konrad Rzeszutek Wilk]

On Mon, Jun 16, 2014 at 01:49:36PM +0000, Dave Scott wrote:
> Hi Konrad,
> 
> On 16 Jun 2014, at 14:34, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> > On Mon, Jun 16, 2014 at 10:49:49AM +0100, David Scott wrote:
> >> 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 = [ "type=socket,path=/tmp/mysock,name=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).
> > 
> > Are these udev scripts somewhere implemented?
> 
> I’ve been playing with these ones:
> 
> https://github.com/djs55/mirage-console/tree/master/udev
> 
> (ignore the reference to Mirage — you just need the .rules file and the helper script)
> 
> Do you think an example udev script like this should live in the tree?

I think it would be fantastic if it was in the docs/ that is part of
this patchset.
> 
> > 
> > Thanks!
> 
> Cheers,
> Dave
> 
> > 
> >> 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 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
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

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

Re: [PATCH v2 1/6] libxl: add a list of abstract 'channels' to the domain config

[Ian Campbell]

On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
> A 'channel' is a low-bandwidth private communication channel that
> resembles a physical serial port. Example uses include:
> 
>   * providing initial VM configuration without having to use the
>     network
>   * signalling a guest agent
> 
> Every channel has a string 'name' which the VM can use to find
> the appropriate handler. Each channel has an implementation 'type'
> which currently includes:
> 
>   * NONE: reads will block, writes will be thrown away

What's this useful for?

>   * PTY: the I/O surfaces as a pty in the backend domain
>   * PATH: writes are appended to a log file in the backend domain

PATH is quite a generic name for that.

Do reads block?

>   * SOCKET: a listening Unix domain socket accepts a connection in
>     the backend domain and proxies
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>
> ---
>  tools/libxl/libxl_types.idl |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 52f1aa9..5671d86 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -51,6 +51,17 @@ libxl_domain_type = Enumeration("domain_type", [
>      (2, "PV"),
>      ], init_val = -1)
>  
> +libxl_channel_type = Enumeration("channel_type", [
> +    # Connected to nothing:
> +    (0, "NONE"),
> +    # Connect to a pty in the backend domain:
> +    (1, "PTY"),
> +    # Spool output to a file in the backend domain:
> +    (2, "PATH"),
> +    # Listen on a Unix domain socket in the backend domain:
> +    (3, "SOCKET"),
> +    ])
> +
>  libxl_device_model_version = Enumeration("device_model_version", [
>      (0, "UNKNOWN"),
>      (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
> @@ -457,6 +468,15 @@ 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),
> +    ("type", libxl_channel_type),
> +    ("path", string),

Is path valid for all types?

If not then a KeyedUnion would be the way to go.

> +])
> +
>  libxl_domain_config = Struct("domain_config", [
>      ("c_info", libxl_domain_create_info),
>      ("b_info", libxl_domain_build_info),
> @@ -467,6 +487,7 @@ 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")),
> +    ("channels", Array(libxl_device_channel, "num_channels")),
>  
>      ("on_poweroff", libxl_action_on_shutdown),
>      ("on_reboot", libxl_action_on_shutdown),

Re: [PATCH v2 3/6] libxl: implement channels via PV console rings

[Ian Campbell]

On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
> We extend the (internal) console type with a 'name' (string)
> which isn't used by the default built-in console 0.

Normally this would be part of the same patch which added the idl
change.

You also need to add a #define LIBXL_HAVE_FOO to libxl.h to flag the
presence of this feature.

Lastly - I don't see any libxl_device_channel_* ops added here, is it
intentional that it not be possible to dynamically add these things?


> +    switch (channel->type) {
> +        case LIBXL_CHANNEL_TYPE_NONE:
> +        case LIBXL_CHANNEL_TYPE_PTY:
> +            /* No path is needed */

How do I go about finding the PTY for a channel?

Ian.

Re: [PATCH v2 4/6] libxl: spawn a qemu to implement channels

[Ian Campbell]

On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
> Since every channel is mapped to a console device in xenstore
> with 'output=chardev:libxl-channel%d', we need to tell qemu
> to create the appropriate chardevs.
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>

Looks good but could have been squashed with all the other libxl
patches.

> ---
>  tools/libxl/libxl_create.c   |    3 ++-
>  tools/libxl/libxl_dm.c       |   39 +++++++++++++++++++++++++++++++++++++--
>  tools/libxl/libxl_internal.h |    3 ++-
>  3 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 6356e55..e178672 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -1214,7 +1214,8 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
>  
>          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);

It's really time that we started passing d_config to this function...
(not your problem here, carry on...)

Re: [PATCH v2 5/6] Add a section in xl.cfg(5) describing 'channels'

[Ian Campbell]

On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
> Signed-off-by: David Scott <dave.scott@citrix.com>
> ---
>  docs/man/xl.cfg.pod.5 |   63 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index a94d037..00f8905 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5

Fold into the xl patch please.

> @@ -470,6 +470,69 @@ 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 bidirectional byte stream, a private connection between
> +the backend and the frontend, which resembles a serial port. 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.

You can add that here too, or first.

Perhaps insert words like "low bandwidth" into this?

> +=item C<name=NAME>
> +
> +Specify the string name for this device. 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.
> +This parameter is mandatory.

Is there a registry of well known names somewhere?

> +=item B<type=FILE>:
> +
> +The backend should be connected to a file such that the
> +console output is read and spooled to the file (e.g. for logging).

Are there any provisions for rotating this log or does this option let
me fill up dom0's filesystem?

> +=item B<type=SOCKET>:
> +
> +The backend should be connected to the equivalent of
> +a Unix domain socket.

equivalent to or actually to one?

Are you trying to suggest that a FIFO or named pipe etc would be
suitable too?

> +
> +=item B<type=PTY>:
> +
> +The backend should be connected to the equivalent of a
> +pty.

master or slave? I think the backend is the master than the client,
whatever that is, would be the slave, right?

(I think your "should be" should all be "will", since this is up to
xl/libxl to arrange, not the user of xl, correct?)

> +
> +=back
> +
> +=item C<path=PATH>
> +
> +If C<type=FILE> then this path is the file to which the console output
> +should be logged. If C<type=SOCKET> then this is the path of the Unix domain
> +socket which should be connected to the channel.

Should the socket be preexisting then?

I think all your type definitions should reference PATH where it makes
sense. e.g. "The backend will be connected to the pty at B<PATH>" etc.

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

Re: [PATCH v2 6/6] Add a general description of the channel mechanism to docs/misc/

[Ian Campbell]

On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
> +  type = (NONE | FILE | PTY | SOCKET | ... )

Are these the literal xenstore key values, shouty caps and all?

Is this stuff all forward compatible with the existing xenconsoled etc?
What stops it trying to manage these guys?

> +  path = <some path>
> +
> +In the default implementation where the backend is run in the toolstack
> +domain, the qemu "chardev" mechanism is used. This implementation
> +interprets the configuration as follows:
> +
> +  type = NONE: the backend will be connected to /dev/null
> +  type = FILE: the channel will be read the the output appended

"the the".

> +    to the file given by 'path'
> +  type = PTY: the backend will connect to a PTY like a regular console
> +  type = SOCKET: the backend will accept a connection on the Unix
> +    domain socket given by 'path' and proxy data to and from the device.
> +
> +If the implementation is in another domain (for example via a Mirage
> +console backend) then the behaviour will be defined by this other domain.

Re: [PATCH v2] implement libvirt-like 'channels' via PV consoles

[Ian Campbell]

On Mon, 2014-06-16 at 09:52 -0400, Konrad Rzeszutek Wilk wrote:
> > 
> > I’ve been playing with these ones:
> > 
> > https://github.com/djs55/mirage-console/tree/master/udev
> > 
> > (ignore the reference to Mirage — you just need the .rules file and the helper script)
> > 
> > Do you think an example udev script like this should live in the tree?
> 
> I think it would be fantastic if it was in the docs/ that is part of
> this patchset.

Even more fantastic to upstream them into udev so they appear in all the
distros by default.

Or otherwise into the appropriate upstream projects (qga etc)

Ian.



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

Re: [PATCH v2 1/6] libxl: add a list of abstract 'channels' to the domain config

[Dave Scott]

Hi,

On 18 Jun 2014, at 14:27, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
>> A 'channel' is a low-bandwidth private communication channel that
>> resembles a physical serial port. Example uses include:
>> 
>>  * providing initial VM configuration without having to use the
>>    network
>>  * signalling a guest agent
>> 
>> Every channel has a string 'name' which the VM can use to find
>> the appropriate handler. Each channel has an implementation 'type'
>> which currently includes:
>> 
>>  * NONE: reads will block, writes will be thrown away
> 
> What's this useful for?

I was thinking of this particular one as a “/dev/null” equivalent — the virtual device is present in the guest (which might be needed to trigger some side-effect) but the output isn’t important.

It appealed to me as a kind of a base case, but I grant you that it’s not the most useful feature in the world :-)

> 
>>  * PTY: the I/O surfaces as a pty in the backend domain
>>  * PATH: writes are appended to a log file in the backend domain
> 
> PATH is quite a generic name for that.

Hm, perhaps calling it FILE would be better — this is what libvirt and qemu both use.

> Do reads block?

I believe so.

>>  * SOCKET: a listening Unix domain socket accepts a connection in
>>    the backend domain and proxies

The SOCKET type is the most useful kind IMHO.

So libvirt and qemu support a bunch of other types: TCP connect to …; receive connection and proxy … (and probably more in future). Instead of trying to mirror the libvirt/qemu APIs we could fall back and only support the “SOCKET” type, and require users to move the data wherever they want via a proxy like “socat”. These channels are low-bandwidth so efficiency is not really a concern.

>> 
>> Signed-off-by: David Scott <dave.scott@citrix.com>
>> ---
>> tools/libxl/libxl_types.idl |   21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>> 
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index 52f1aa9..5671d86 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -51,6 +51,17 @@ libxl_domain_type = Enumeration("domain_type", [
>>     (2, "PV"),
>>     ], init_val = -1)
>> 
>> +libxl_channel_type = Enumeration("channel_type", [
>> +    # Connected to nothing:
>> +    (0, "NONE"),
>> +    # Connect to a pty in the backend domain:
>> +    (1, "PTY"),
>> +    # Spool output to a file in the backend domain:
>> +    (2, "PATH"),
>> +    # Listen on a Unix domain socket in the backend domain:
>> +    (3, "SOCKET"),
>> +    ])
>> +
>> libxl_device_model_version = Enumeration("device_model_version", [
>>     (0, "UNKNOWN"),
>>     (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
>> @@ -457,6 +468,15 @@ 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),
>> +    ("type", libxl_channel_type),
>> +    ("path", string),
> 
> Is path valid for all types?
> 
> If not then a KeyedUnion would be the way to go.

OK.

So I think the question I’m most unsure about is: should we keep the notion of ‘type’ here (and switch to a KeyedUnion) or treat everything in a uniform way (e.g. socket) and let someone else to the plumbing?

Thanks,
Dave

> 
>> +])
>> +
>> libxl_domain_config = Struct("domain_config", [
>>     ("c_info", libxl_domain_create_info),
>>     ("b_info", libxl_domain_build_info),
>> @@ -467,6 +487,7 @@ 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")),
>> +    ("channels", Array(libxl_device_channel, "num_channels")),
>> 
>>     ("on_poweroff", libxl_action_on_shutdown),
>>     ("on_reboot", libxl_action_on_shutdown),

Re: [PATCH v2 6/6] Add a general description of the channel mechanism to docs/misc/

[Dave Scott]

Hi Ian,

On 18 Jun 2014, at 14:41, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
>> +  type = (NONE | FILE | PTY | SOCKET | ... )
> 
> Are these the literal xenstore key values, shouty caps and all?

In the current qemu-based implementation they don’t appear in xenstore. Instead xenstore has a reference to a “-chardev” on the qemu command line.

  /local/domain/$DOMID/device/console/$DEVID/output = chardev:libxl-channel0

(for some reason the ‘output’ key is in the frontend. At least it’s readonly)

  qemu-system-i386 -M xenpv -chardev socket,id=libxl-channel0,path=%s,server,nowait

Actually I think it would probably be good to include these keys in xenstore as well even if they aren’t currently used (minus the shouty caps) since then they could be read by console backends in other domains.

> Is this stuff all forward compatible with the existing xenconsoled etc?
> What stops it trying to manage these guys?

To handle this through xenconsoled would involve:

1. extending xenconsoled to watch for console device backends (like any other device backend). Currently xenconsoled is limited to the initial console only.

2. changing the libxl implementation to remove the -chardevs from the qemu command-line and add the keys (type/path) to the backend directory

It ought to be possible to get xenconsoled running in multiple domains and choose between them by setting the ‘backend’ in the libxl device.

Importantly no libxl client would need to change.

I took the qemu approach for the initial implementation because this is how libxl arranges for  the second/third/fourth/… consoles to be served (possibly because xenconsoled has never been extended). Personally I’d like to switch over to xenconsoled eventually rather than rely on qemu for ever more stuff.

Cheers,
Dave

> 
>> +  path = <some path>
>> +
>> +In the default implementation where the backend is run in the toolstack
>> +domain, the qemu "chardev" mechanism is used. This implementation
>> +interprets the configuration as follows:
>> +
>> +  type = NONE: the backend will be connected to /dev/null
>> +  type = FILE: the channel will be read the the output appended
> 
> "the the".
> 
>> +    to the file given by 'path'
>> +  type = PTY: the backend will connect to a PTY like a regular console
>> +  type = SOCKET: the backend will accept a connection on the Unix
>> +    domain socket given by 'path' and proxy data to and from the device.
>> +
>> +If the implementation is in another domain (for example via a Mirage
>> +console backend) then the behaviour will be defined by this other domain.
> 
> 

Re: [PATCH v2 1/6] libxl: add a list of abstract 'channels' to the domain config

[Ian Campbell]

On Wed, 2014-06-18 at 15:05 +0100, Dave Scott wrote:
> Hi,
> 
> On 18 Jun 2014, at 14:27, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Mon, 2014-06-16 at 10:49 +0100, David Scott wrote:
> >> A 'channel' is a low-bandwidth private communication channel that
> >> resembles a physical serial port. Example uses include:
> >> 
> >>  * providing initial VM configuration without having to use the
> >>    network
> >>  * signalling a guest agent
> >> 
> >> Every channel has a string 'name' which the VM can use to find
> >> the appropriate handler. Each channel has an implementation 'type'
> >> which currently includes:
> >> 
> >>  * NONE: reads will block, writes will be thrown away
> > 
> > What's this useful for?
> 
> I was thinking of this particular one as a “/dev/null” equivalent —
> the virtual device is present in the guest (which might be needed to
> trigger some side-effect) but the output isn’t important.
> 
> It appealed to me as a kind of a base case, but I grant you that it’s
> not the most useful feature in the world :-)

:-)

It seems to me that triggering a side-effect in the guest without
needing the actual channel is the sort of thing which would be prone to
abuse.

I'd be inclined to leave it out unless/until there is a concrete user
which we can decide is reasonable (or not).

> >>  * PTY: the I/O surfaces as a pty in the backend domain
> >>  * PATH: writes are appended to a log file in the backend domain
> > 
> > PATH is quite a generic name for that.
> 
> Hm, perhaps calling it FILE would be better — this is what libvirt and qemu both use.

That misses the append+write-only aspects of it. I'd naïvely expect to
be able to point it at a file to be slurped in (like we do for the
qemu-stubdm restore file).

> >>  * SOCKET: a listening Unix domain socket accepts a connection in
> >>    the backend domain and proxies
> 
> The SOCKET type is the most useful kind IMHO.
> 
> So libvirt and qemu support a bunch of other types: TCP connect to …;
> receive connection and proxy … (and probably more in future). Instead
> of trying to mirror the libvirt/qemu APIs we could fall back and only
> support the “SOCKET” type, and require users to move the data wherever
> they want via a proxy like “socat”. These channels are low-bandwidth
> so efficiency is not really a concern.

I don't think we'd have a problem adding more transports as demand
arises, but lets concentrate on the simple ones which meet your usecases
for now instead of imaging all the mad things one might do with this
functionality.

(unless you were suggesting that you wanted the TCP thing, in which case
sorry for calling your use case mad ;-))

> > Is path valid for all types?
> > 
> > If not then a KeyedUnion would be the way to go.
> 
> OK.
> 
> So I think the question I’m most unsure about is: should we keep the
> notion of ‘type’ here (and switch to a KeyedUnion) or treat everything
> in a uniform way (e.g. socket) and let someone else to the plumbing?

PTY and SOCKET certainly seem distinctly useful to me. FILE as an append
only logging thing seems sort of OK but I've concerns about filling
disks with it.

Ian.


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

Re: [PATCH v2 6/6] Add a general description of the channel mechanism to docs/misc/

[Ian Campbell]

On Wed, 2014-06-18 at 15:37 +0100, Dave Scott wrote:
> > Is this stuff all forward compatible with the existing xenconsoled etc?
> > What stops it trying to manage these guys?
> 
> To handle this through xenconsoled would involve:

What I meant was why does xenconsoled not try and manage these today and
get in the way? To which the answer is:

> 1. extending xenconsoled to watch for console device backends (like
> any other device backend). Currently xenconsoled is limited to the
> initial console only.

This (which I wasn't aware of, I thought it handled multiple).

ISTR there is a key in their which allows qemu or xenconsoled to provide
this initial console too.

> I took the qemu approach for the initial implementation because this
> is how libxl arranges for  the second/third/fourth/… consoles to be
> served (possibly because xenconsoled has never been extended).
> Personally I’d like to switch over to xenconsoled eventually rather
> than rely on qemu for ever more stuff.

I'll defer to Stefano on this. I thought the normal qemu (upstream)
approach was:

      * emulated stuff via command-line/qmp but never xenstore (in
        contrast with qemu-trad which played all sorts of tricks to
        insert IDE disks from xenstore).
      * PV stuff via watching xenstore backend paths.

Do I understand you correctly that there is a qemu patch involved in all
this, or does it just happen to work if you feed it the correct cmdline
runes?


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

Re: [PATCH v2 6/6] Add a general description of the channel mechanism to docs/misc/

[Dave Scott]

On 18 Jun 2014, at 15:43, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Wed, 2014-06-18 at 15:37 +0100, Dave Scott wrote:
>>> Is this stuff all forward compatible with the existing xenconsoled etc?
>>> What stops it trying to manage these guys?
>> 
>> To handle this through xenconsoled would involve:
> 
> What I meant was why does xenconsoled not try and manage these today and
> get in the way? To which the answer is:
> 
>> 1. extending xenconsoled to watch for console device backends (like
>> any other device backend). Currently xenconsoled is limited to the
>> initial console only.
> 
> This (which I wasn't aware of, I thought it handled multiple).
> 
> ISTR there is a key in their which allows qemu or xenconsoled to provide
> this initial console too.

That’s right — there’s a xenstore key type = (ioemu | xenconsoled). (Note to self: I’d better rename my ‘type’ key)

> 
>> I took the qemu approach for the initial implementation because this
>> is how libxl arranges for  the second/third/fourth/… consoles to be
>> served (possibly because xenconsoled has never been extended).
>> Personally I’d like to switch over to xenconsoled eventually rather
>> than rely on qemu for ever more stuff.
> 
> I'll defer to Stefano on this. I thought the normal qemu (upstream)
> approach was:
> 
>      * emulated stuff via command-line/qmp but never xenstore (in
>        contrast with qemu-trad which played all sorts of tricks to
>        insert IDE disks from xenstore).
>      * PV stuff via watching xenstore backend paths.
> 
> Do I understand you correctly that there is a qemu patch involved in all
> this, or does it just happen to work if you feed it the correct cmdline
> runes?

No qemu patch needed — once I figured out the right command line magic it stopped crashing and started working :-)

My (primitive) understanding is that, if you run a qemu for a PV guest (see libxl_dm.c:libxl__need_xenpv_qemu) then it’ll act as the backend for qdisk backends and these extra consoles where libxl has written ‘type=ioemu’. If xenconsoled were extended to support these then libxl would write ‘type=xenconsoled’ and qemu would ignore them.

In the qemu code the xen console backends (tools/qemu-xen-dir/hw/xen_console.c) are qemu “chardev” devices which gives you access to the exciting world of qemu-char.c:

static const struct {
    const char *name;
    CharDriverState *(*open)(QemuOpts *opts);
} backend_table[] = {
    { .name = "null",      .open = qemu_chr_open_null },
    { .name = "socket",    .open = qemu_chr_open_socket },
    { .name = "udp",       .open = qemu_chr_open_udp },
    { .name = "msmouse",   .open = qemu_chr_open_msmouse },
    { .name = "vc",        .open = text_console_init },
#ifdef _WIN32
    { .name = "file",      .open = qemu_chr_open_win_file_out },
    { .name = "pipe",      .open = qemu_chr_open_win_pipe },
    { .name = "console",   .open = qemu_chr_open_win_con },
    { .name = "serial",    .open = qemu_chr_open_win },
    { .name = "stdio",     .open = qemu_chr_open_win_stdio },
#else
    { .name = "file",      .open = qemu_chr_open_file_out },
    { .name = "pipe",      .open = qemu_chr_open_pipe },
    { .name = "pty",       .open = qemu_chr_open_pty },
    { .name = "stdio",     .open = qemu_chr_open_stdio },
#endif
#ifdef CONFIG_BRLAPI
    { .name = "braille",   .open = chr_baum_init },
#endif
#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
    || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
    || defined(__FreeBSD_kernel__)
    { .name = "tty",       .open = qemu_chr_open_tty },
#endif
#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__) \
    || defined(__FreeBSD_kernel__)
    { .name = "parport",   .open = qemu_chr_open_pp },
#endif
#ifdef CONFIG_SPICE
    { .name = "spicevmc",     .open = qemu_chr_open_spice },
#endif
};

Cheers,
Dave