Re: [Xen-devel] ocaml libxl bindings and KeyedUnion

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2015-03-20 at 09:10 +0100, Olaf Hering wrote:
> Some change commited to staging earlier this week breaks ocaml bindings,
> as shown below. I think it was introduced between
> a68d1b65bb1bbb9b8db2d82695d32ac09c52a2d7..d4ea77c9d7b314001ff4e2eb7618b45f11551371:
> 
> NotImplementedError: Cannot handle KeyedUnion fields which are not Structs

I don't see anything in that range which I would expect to cause
anything like this so...

> But now that I look at it, may it be my pvscsi change by any chance?
> https://github.com/olafhering/xen/commit/f0de53755f628efe64cd60d4511a9667485eba62

... yes, probably. Although surely it would have been easier for you to
confirm this yourself than asking everyone to speculate, especially if
you are going to start your mail blaming something in staging.

> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 48cd9af..2bd050d 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -556,12 +556,21 @@ libxl_vscsi_hctl = Struct("vscsi_hctl", [
>      ("lun", uint32),
>      ])
>  
> +libxl_vscsi_pdev = Struct("vscsi_pdev", [
> +    ("p_devname",        string),
> +    ("u", KeyedUnion(None, libxl_vscsi_pdev_type, "type",
> +        [
> +         ("invalid", None),
> +         ("dev", libxl_vscsi_hctl),
> +         ("wwn", string),
> +         ("hctl", libxl_vscsi_hctl),

Following the other examples in libxl_types.idl this would need to be
something like:
         ("invalid", None),
         ("dev", Struct(None, [(libxl_vscsi_hctl, "dev")])),
         ("wwn", Struct(None, [(string, "wwn")])),
         ("hctl", Struct(None, [(libxl_vscsi_hctl, "hctl")])),
           ^(*)                                     ^(**)

Or you could teach the IDL machinery and code generators about
non-struct members of keyed unions but a) that will be a lot of faff and
b) it will make it hard to extend any one of the members in the future.

The name at (*) must be the enum member, which I've duplicated at (**)
but you might like to thing about whether (**) would have a better name
in the context of e.g. vscsi_dev->u.dev.dev or vscsi_dev->u.wwn.wwn.

Aside: What is the difference between dev and hctl in this context?

Ian.


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