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

[David Scott <dave.scott@xxxxxxxxxx>]

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



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