From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: xl, libxl: add support for 'channels' Date: Thu, 25 Sep 2014 15:13:43 -0400 Message-ID: <20140925191343.GF29663@laptop.dumpdata.com> References: <1411591685-25308-1-git-send-email-dave.scott@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XXEUD-0003lw-US for xen-devel@lists.xenproject.org; Thu, 25 Sep 2014 19:13:54 +0000 Content-Disposition: inline In-Reply-To: <1411591685-25308-1-git-send-email-dave.scott@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Scott Cc: xen-devel@lists.xenproject.org, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, wei.liu2@citrix.com, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org 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. 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? Also, while my Reviewed-by is nice, either of the three maintainers: Wei, IanJ, and IanC would need to Review these patches as well. > > 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 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= 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 > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel