Re: [Xen-devel] xl, libxl: add support for 'channels'

[Dave Scott <Dave.Scott@xxxxxxxxxx>]

Hi Konrad,

On 25 Sep 2014, at 20:13, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:

> On Wed, Sep 24, 2014 at 09:48:00PM +0100, David Scott wrote:
>> Hi,
>> 
>> I've split out the xl string fiddling into a couple of patches to make
>> them easier to review. I've added __attribute__((unused)) in these commits
>> to keep it building and taken them away again in the last patch.
>> 
>> Most of the rest of the changes are to docs, and comments in idl/headers.
>> I think I'm too familiar with the low-level details to see where the docs
>> are obviously missing :-)
> 
> I looked at the patches and I had mostly syntax questions and maybe
> spotted two memory leaks.

Thanks for those, I’ll fix up the problems.

> The major piece of information that is missing is - who/what generates
> the udev information in the guest? I could not find it in the Linux
> hvc_console.c driver so I am unclear of how it is suppose to
> work there?

I believe all secondary consoles (in device/console/ in xenstore) generate the
event. I’m not familiar with the kernel side of things; Stefano: can you point
us in the right direction?

For reference I used a udev rule to catch all secondary consoles:

# Set up secondary Xen consoles
SUBSYSTEM=="xen", DEVPATH=="/devices/console-[0-9]", RUN+="xenconsole-setup-tty”

and then I wrote a simple shell script to read the optional ‘name’ key from
xenstore and, if present, create a device node with a well-known name rather
than the default one.

For reference my hacky test script is here: 

https://github.com/djs55/mirage-console/blob/master/udev/xenconsole-setup-tty

For an end-to-end test I took cloudstack and added this udev rule to its
helper VM. I tweaked the helper VM software to open my named device node
rather than the KVM /dev/vport one. IIRC I also had to also switch the tty
into raw mode. After that it worked: the cloudstack agent connected to the
dom0 socket and was able to exchange data with the domU software.

This was all inspired by the Daniel B from libvirt describing the libvirt
channels as “consoles with names”. I took him literally :-)

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

Indeed.

Cheers,
Dave

> 
>> 
>> 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 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.
>>  Channels are user-configurable and do 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
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel


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