[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-API] [MirageOS-devel] Xenstore (client) updates






On Thu, Jul 17, 2014 at 2:36 PM, Anil Madhavapeddy <anil@xxxxxxxxxx> wrote:
On 17 Jul 2014, at 13:49, David Scott <scott.dj@xxxxxxxxx> wrote:

> Hi,
>
> I've just merged updates to the Mirage Xenstore protocol and client implementation to master (but not yet released). There are backwards-incompatible API changes which I'd like to get right before release, so feedback is welcome. Note the signatures are separate from the Mirage V2_LWT ones -- this is an internal implementation library used for Xen Mirage kernels only. I'll not release this code until (a) we're happy with it; and (b) patches are available for all users in opam (typical users include Mirage device drivers and Xen toolstacks)
>
> The reasons I'm proposing backwards-incompatible changes are:
>
> 1. to hide the 'client' type from clients. Since there should only be one real Xenstore connection per process (whether Unix domain sockets or shared memory), this ended up being a singleton. There doesn't seem any point in requesting the user 'create' and 'manage' these when the library was doing it all anyway.
>
> 2. for transactions and watches, I've made this into a more monadic style. The examples in the README.md show what I mean.
>
> 3. to move away from exceptions (I'm looking at you, ENOENT) and to use option types. So we now have 'read' and 'read_exn'. Does anyone know of any other functions whose signatures could be improved?

Instead of option types, isn't the Or_error.t style better to avoid not losing the reason for the failure? ÂFor example,

type 'a t =
 | Error of exn
 | Ok of 'a

I think this is a good idea. I started by making 'read' return a 'string option' because the 'None' case means 'key doesn't exist' and isn't really an error-- it's so common client code is riddled with code that expects it. For all the 'real' errors which are actually fatal (Einval, Equota, Eperm etc etc) Or_error.t looks nice.
Â

>
> 4. (for a crash-resistant Irmin Xenstore server): there's now control over where we are in the shared memory ring. Previously a 'read' would 'consume' data immediately, which would be lost if we crashed. Now 'read' should not advance the stream, and the server must decide when is appropriate and call an 'advance' function manually.

This is a significant improvement and probably relevant to other consumers of this style of RPC (e.g. a vchan2)!

>
> A couple of house-keeping items:
>
> * The repo mirage/ocaml-xenstore which contains the Xenstore protocol implementation and client code used to be a fork of djs55/ocaml-xenstore. I've fixed this anomaly and now the mirage/ocaml-xenstore version is the authoritative version.
>
> * The license of the Xenstore protocol code and client is the Mirage standard (ISC)
>
> * The code has been re-indented with ocp-indent --syntax=lwt (Mirage standard style?)

Yes, the `ocp-indent` defaults are becoming the defacto standard for indentation. ÂIt doesn't tell you where to put newlines, so there's still some artistic style potential left for the individual programmer :-)

Cheers,
Dave
_______________________________________________
Xen-api mailing list
Xen-api@xxxxxxxxxxxxx
http://lists.xen.org/cgi-bin/mailman/listinfo/xen-api

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.