Mailing List Archive

Re: [MirageOS-devel] Xenstore (client) updates
On 17 Jul 2014, at 13:49, David Scott <scott.dj@gmail.com> 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

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

best,
Anil
_______________________________________________
Xen-api mailing list
Xen-api@lists.xen.org
http://lists.xen.org/cgi-bin/mailman/listinfo/xen-api
Re: [MirageOS-devel] Xenstore (client) updates [ In reply to ]
On Thu, Jul 17, 2014 at 2:36 PM, Anil Madhavapeddy <anil@recoil.org> wrote:

> On 17 Jul 2014, at 13:49, David Scott <scott.dj@gmail.com> 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