Re: [Xen-devel] [PATCH v3 1/3] docs: add a document describing the 'channels' mechanism

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

David Scott writes ("[PATCH v3 1/3] docs: add a document describing the 
'channels' mechanism"):
> A channel is a low-bandwidth private byte stream similar to a serial
> link. Typical uses of channels are
...

Thanks for this.  I have some comments on this patch (I haven't read
the others yet but will do so shortly).

A minor comment: your commit message recapitulates the contents of the
document in the patch.  If you are going to have a separate docs patch
the commit message should probably say something like "introduce the
documentation" or something.

> diff --git a/docs/misc/channel.txt b/docs/misc/channel.txt
> new file mode 100644
> index 0000000..26c2289
> --- /dev/null
> +++ b/docs/misc/channel.txt
> @@ -0,0 +1,52 @@
> +Xen PV Channels

I think much of this ought to go in context-specific documents, rather
than providing the whole stack in a single file.

> +------------------------------------------------------------------------
> +                                                             David Scott
> +                                                   dave.scott@xxxxxxxxxx

I don't think leaving authorship information in the document is
sensible.  It will just inhibit people from editing it.  If someone
wants to know the author, the vcs logs are available.

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

This is a useful overview.

> +Channels are similar to virtio-serial devices, and are intended to be
> +used in the implementation of libvirt <channel>s when running on Xen.

It might be better to say that channels are similar to emulated serial
links.

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

This information about the xenstore protocol should be in
docs/misc/xenstore-paths.markdown, not here.

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

You should set up a registry (in the channels.txt document, not here)
of the expected values for "name" with their intended semantics.

> +In the default implementation the backend is implemented via qemu
> +in "xenpv" mode (i.e. the 'console' device in xenstore will have
> +'type=ioemu'). Futhermore if 'kind=socket' then the console 'output'
> +in xenstore will be set to:
> + 
> +  output = chardev:libxl-channel$DEVID
> +
> +The qemu commandline contains one "-chardev id=libxl-channel$DEVID,..."
> +option per channel.

AFAICT this is an implementation detail, not part of any interface.
It should therefore be in a comment somewhere.  (Or perhaps even in a
commit message.)

Thanks,
Ian.

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