CP-26352 Port xcp-idl/network interface from Camlp4 to PPX #189

krizex · 2018-01-17T07:57:27Z

Signed-off-by: Yang Qian yang.qian@citrix.com

Prototype of porting network interface to ppx.

Because of below cases, we have to update client and server part accordingly:

type dhcp_options should change from polymorphic variant to variant
variant Basic of type port_kind should change to Basic_port because the Basic definition conflicts with that in rpc.ml
field ipv4_routes of type interface_config_t should change to record.
the optional arguments of interfaces should move to use option so all client and server part code should update at same time.
rpc_of_x and x_of_rpc function not exists yet, should replace them with Rpcmarshal.marshal and Rpcmarshal.unmarshal
Move the definition of exceptions to xcp-networkd and update the code of processing exceptions in client part

This PR should goes in with xapi-project/xcp-networkd#128 and xapi-project/xen-api#3439

krizex · 2018-01-17T08:03:02Z

@robhoes @jonludlam Please have a look and help confirm whether I am on the right direction.

coveralls · 2018-01-17T10:40:17Z

Coverage remained the same at 54.268% when pulling 6cdd636 on krizex:private/yangqi/CP-26352 into 144fa27 on xapi-project:ppx_network.

robhoes · 2018-01-17T11:03:57Z

@minishrink, could you please review this, since you have the most experience with this stuff now?

jonludlam · 2018-01-17T11:15:01Z

network/network_interface.ml

+  interface_config = [];
+  bridge_config = [];
+  gateway_interface = None;
+  dns_interface = None
 }


I don't think you need these default_* any more because you've got the in-line defaults defined above.

Yes, idl do not need this anymore, but xcp-networkd need them, so I will move them to xcp-networkd repo.

krizex · 2018-01-17T11:56:56Z

xcp-networkd need them, or I can move them to xcp-networkd repo

________________________________ From: Jon Ludlam <notifications@github.com> Sent: Wednesday, January 17, 2018 7:15:02 PM To: xapi-project/xcp-idl Cc: Yang Qian; Author Subject: Re: [xapi-project/xcp-idl] CP-26352 Port xcp-idl/network interface from Camlp4 to PPX (#189) @jonludlam commented on this pull request.

________________________________ In network/network_interface.ml<#189 (comment)>:

let default_config = {

- interface_config = []; - bridge_config = []; - gateway_interface = None; - dns_interface = None + interface_config = []; + bridge_config = []; + gateway_interface = None; + dns_interface = None } I don't think you need these default_* any more because you've got the in-line defaults defined above. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#189 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ADNcadL5SDc0gCSatFLtKM8oatyXLKHjks5tLdY2gaJpZM4Rg8FD>.

mseri · 2018-01-17T16:01:01Z

network/jbuild

@@ -32,7 +32,7 @@ let () = Printf.ksprintf Jbuild_plugin.V1.send {|
 ((name xcp_network_interface)
  (public_name xcp.network.interface)
  (modules (network_interface))
-  (flags (:standard -w -39 %s))
+  (flags (:standard -w -39 -w -34 -w -27 %s))


Please do not silence warnings 27 and 34. In general they can be fixed prefixing variables or types with _ or deleting them if they are unused or unexposed. They can also be silenced locally if really needed. But I'd rather see them than ignore them

-w -34 has been deleted, but I cannot delete -w -27 because if delete it the compiler will complains

File "network/network_interface.ml", line 116, characters 0-611: Warning 27: unused variable x.

The corresponding code is:

type interface_config_t = { (* Line 116 *) ipv4_conf: ipv4 [@default None4]; ipv4_gateway: Unix.inet_addr option [@default None]; ipv6_conf: ipv6 [@default None6]; ipv6_gateway: Unix.inet_addr option [@default None]; (* Change from ipv4_routes: (Unix.inet_addr * int * Unix.inet_addr) list; *) ipv4_routes: ipv4_route_t list [@default []]; dns: Unix.inet_addr list * string list [@default [], []]; mtu: int [@default 1500]; ethtool_settings: (string * string) list [@default []]; ethtool_offload: (string * string) list [@default ["lro", "off"]]; persistent_i: bool [@default false]; } [@@deriving rpcty]

Any idea about the warning? I think it comes from ppx_core

The warning above should not break the compilation though, we could live with the warning. I'll have a look at the generated file

That is a bug of ocaml-rpc that does generate some code like:

typ_of_interface_config_t = let open! Rpc.Types in Struct ({ fields = [BoxedField interface_config_t_ipv4_conf; BoxedField interface_config_t_ipv4_gateway; BoxedField interface_config_t_ipv6_conf; BoxedField interface_config_t_ipv6_gateway; BoxedField interface_config_t_ipv4_routes; BoxedField interface_config_t_dns; BoxedField interface_config_t_mtu; BoxedField interface_config_t_ethtool_settings; BoxedField interface_config_t_ethtool_offload; BoxedField interface_config_t_persistent_i]; sname = "interface_config_t"; version = None; constructor = (fun getter -> let open Rresult.R in (match getter.Rpc.Types.fget "persistent_i" (let open! Rpc.Types in Basic Bool) with | Result.Ok x as y -> y | Result.Error _ -> Result.Ok false) >>= (fun interface_config_t_persistent_i -> (match getter.Rpc.Types.fget "ethtool_offload" (let open! Rpc.Types in Dict (String, (let open! Rpc.Types in Basic String))) with | Result.Ok x as y -> y | Result.Error _ -> Result.Ok [("lro", "off")]) >>= (fun interface_config_t_ethtool_offload -> (match getter.Rpc.Types.fget "ethtool_settings" (let open! Rpc.Types in Dict (String, (let open! Rpc.Types in Basic String))) with | Result.Ok x as y -> y | Result.Error _ -> Result.Ok []) >>= (fun interface_config_t_ethtool_settings -> (match getter.Rpc.Types.fget "mtu" (let open! Rpc.Types in Basic Int) [...]

We'll fix it there, please leave the warning on and instead modify the Makefile as follows:

Change

build: jbuilder build @install --dev -j $$(getconf _NPROCESSORS_ONLN) release: jbuilder build @install

into

release: jbuilder build @install -j $$(getconf _NPROCESSORS_ONLN) build: jbuilder build @install --dev

I have made a PR to fix the issue upstream: mirage/ocaml-rpc#77
We will get it in xenserver after the opam-next merge

Thanks @mseri ! BTW, can you show me how to get the code that generated by ocaml-rpc that as you showed above so I can check them when I get a compilation error?

After jbuilder build, if you enter into _build/default, you can run the ppx rewriters generated by jbuilder as follows

$ .ppx/ppx_deriving_rpc/ppx.exe --impl network/network_interface.ml > network_interface.generated.ml

Depending on the file, you may need to use ppx_deriving_rpc+ppx_sexp_conv instead of ppx_deriving_rpc, or some other combination or rewriters, but you get the idea

@mseri However I cannot find .ppx/ppx_deriving_rpc/ppx.exe after executing jbuilder build in xcp-idl repo.
I have tried to build the executable file by

ocamlfind ocamlopt -predicates ppx_driver -o ppx -linkpkg -package ppx_deriving_rpc -package ppx_bin_prot -package ppx_driver.runner

but when executing it complains

[planex xcp-idl]$ ./ppx --impl network/network_interface.ml File "network/network_interface.ml", line 67, characters 37-42: Error: ppx_type_conv: 'rpcty' is not a supported type type-conv generator

mseri · 2018-01-17T16:02:01Z

network/jbuild

@@ -23,7 +23,7 @@ let coverage_rewriter =
    ""

 let rewriters_camlp4 = ["rpclib.idl -syntax camlp4o"]
-let rewriters_ppx = ["ppx_deriving_rpc"; "ppx_sexp_conv"]
+let rewriters_ppx = ["ppx_deriving_rpc"; "ppx_sexp_conv";]


Last semicolon should not be there

coveralls · 2018-01-18T05:39:34Z

Coverage remained the same at 54.268% when pulling 2442376 on krizex:private/yangqi/CP-26352 into 144fa27 on xapi-project:ppx_network.

coveralls · 2018-01-18T05:42:29Z

Coverage remained the same at 54.268% when pulling 2442376 on krizex:private/yangqi/CP-26352 into 144fa27 on xapi-project:ppx_network.

coveralls · 2018-01-18T05:45:46Z

Coverage remained the same at 61.934% when pulling 47a25db on krizex:private/yangqi/CP-26352 into 236beab on xapi-project:ppx_network.

mseri · 2018-01-19T14:46:05Z

network/network_interface.ml

@@ -11,8 +11,8 @@
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU Lesser General Public License for more details.
 *)
-
-(** {2 Helper functions} *)


Leave this header below the module openings

mseri · 2018-01-19T14:47:13Z

network/network_interface.ml

-		let masks = List.map ((-) 255) [a; b; c; d] in
-		32 - (List.fold_left length 0 masks)
-	)
+  Scanf.sscanf netmask "%d.%d.%d.%d" (fun a b c d ->


Can you separate the white space changes from the actual code changes in two different commits?

In the meanwhile I have found that adding &w=true to the url, github ignores the whitespace. You cannot add review comment on the ignore-whitespace versions but it made reviewing much easier

Anyway, it could be better to separate the whitespace changes into an individual commit.

mseri · 2018-01-19T14:51:37Z

network/network_interface.ml

+  include Unix
+  let typ_of_inet_addr = Rpc.Types.Abstract ({
+      rpc_of = (fun t -> Rpc.String (Unix.string_of_inet_addr t));
+      of_rpc = (function | Rpc.String s -> Ok (Unix.inet_addr_of_string s) | _ -> Error (`Msg "Expecting value of string type when unmarshalling inet_addr"));


Please use multiple lines

function | Rpc.String s -> Ok (Unix.inet_addr_of_string s) | _ -> Error (`Msg "Expecting value of string type when unmarshalling inet_addr")

mseri · 2018-01-19T14:51:52Z

network/network_interface.ml

-	let masks = List.map (string_of_int ++ mask) lens in
-	String.concat "." masks
-
-(** {2 Exceptions} *)


Please keep this header in the appropriate place

mseri · 2018-01-19T14:52:05Z

network/network_interface.ml

-exception Not_implemented
-exception Vlan_in_use of (string * int)
-
-(** {2 Types} *)


Please keep this header in the appropriate place

mseri · 2018-01-19T15:02:38Z

network/network_interface.ml

+        "The default variant for forward compatibility."
+      ]]
+[@@default Unknown_error]
+[@@deriving rpcty]


These are fine, when you have such long types it make sense to append them to the bottom.

Not that you can replace the decorated strings with actual docstrings:

[@doc [ "[PVS_proxy_connection_error] is reported "; "when unable to connect PVS proxy"]]

into

(** [PVS_proxy_connection_error] is reported "when unable to connect PVS proxy *)

and in this way they will appear both in the rpc documentation and in the ocaml one.

That should be more readable, but where to put the docstring? before the match pattern or after?
e.g.

| PVS_proxy_connection_error (** [PVS_proxy_connection_error] is reported "when unable to connect PVS proxy *)

or

| (** [PVS_proxy_connection_error] is reported "when unable to connect PVS proxy *) PVS_proxy_connection_error

They go after, you can use https://caml.inria.fr/pub/docs/manual-ocaml/ocamldoc.html#sec349 as a reference

mseri · 2018-01-19T15:05:12Z

network/network_interface.ml

+
+  let unit_p = Param.mk Types.unit
+
+  (* external clear_state: unit -> unit = "" *)


You don't need any of these comments, the type is explicit in the definitions and the old type can be recovered using git

Yes, that used to help you review the code changes.
I will remove them now.

mseri · 2018-01-19T15:13:12Z

network/network_interface.ml

+    (* 	external get_all : debug_info -> unit -> iface list = "" *)
+    let get_all =
+      let module T = struct
+        type _iface_list_t = iface list [@@deriving rpcty]


@jonludlam why do we need this kind of type juggling for polymorphic types like lists or options?

There should be a convenience approach to handle this stuff, but I cannot find it, need Jon's help.

I am not sure there is, that's why I wanted to double check

mseri · 2018-01-19T15:27:01Z

network/network_interface.ml


-exception PVS_proxy_connection_error


@jonludlam can we have multiple kind or errors err, pvs_err, ... so that specific calls are bound to return specific errors? @robhoes do you thin would be worth doing it?

mseri · 2018-01-19T15:28:48Z

network/network_stats.ml

@@ -119,7 +119,9 @@ let read_stats () =
 			if payload |> Digest.string |> Digest.to_hex <> checksum then
 				raise Invalid_checksum
 			else
-				payload |> Jsonrpc.of_string |> stats_t_of_rpc


Why do we need to change this instead of keeping the deriving rpc annotation and using the generated stats_t_of_rpc?

By deriving rpcty, there are no stats_t_of_rpc again.

Why do we need to derive rpcty instead of rpc? Can we do [@@deriving rpc, rpcty], does it work?

John requests using rpcty, I think derive both from rpc and rpcty works, but it's redundant because they handle the same stuffs.

In this case I don't see the benefit. @jonludlam why rpcty for this instead of rpc?

krizex · 2018-01-22T10:24:06Z

Updates:

Separate the white spaces changes into an individual commit
Update per comments from @mseri

mseri · 2018-01-22T10:26:02Z

network/jbuild

@@ -32,7 +32,7 @@ let () = Printf.ksprintf Jbuild_plugin.V1.send {|
 ((name xcp_network_interface)
  (public_name xcp.network.interface)
  (modules (network_interface))
-  (flags (:standard -w -39 %s))
+  (flags (:standard -w -39 -w -27 %s))


Remove -w -27. If you rebase over xcp-idl master we merged changes to the Makefile to avoid failing in case of warning. The warning will be then fixed in the next release of ocaml-rpc

krizex · 2018-01-23T08:20:17Z

Rebase on master and delete the -w -27 in network/jbuild.

mseri · 2018-01-23T10:24:37Z

This looks good but should be first tested separately after building on a feature branch (with extreme attention to the update/upgrade tests) and will not be merged until that has happened

krizex · 2018-01-24T02:01:04Z

Another thing is regarding to the optional parameters in the interface, Param.mk cannot handle this case, so I replace them by adding suffix option and convert them to named parameters, but this will bring extra effort to convert them in the implementation, so @robhoes suggested to just convert them to named parameter and change client part to explicitly pass the parameter.
I will make the change with a new commit.

minishrink

This is a good PR. I would suggest you add a network CLI for debugging (if you do this, you'll have to update the jbuild file again), and prune the jbuild file of unnecessary bits.

minishrink · 2018-01-30T16:26:58Z

network/jbuild

@@ -53,4 +53,4 @@ let () = Printf.ksprintf Jbuild_plugin.V1.send {|
  (wrapped false)
  %s))

-|} (flags rewriters_camlp4) coverage_rewriter (flags rewriters_ppx) coverage_rewriter
+|} (flags rewriters_ppx) coverage_rewriter (flags rewriters_ppx) coverage_rewriter


You can trim the jbuild file, as some of it is now redundant:

Line 25 is not needed anymore, as you're removing Camlp4 as a dependency

Line 26: similarly, ppx_deriving_sexp is no longer necessary

Lines 31-41, 46, 52: as in the jbuild files for Squeezed and V6d, you can remove explicit references to the interface without affecting the build

if you do this, line 56 can be changed to just
|} (flags rewriters_ppx) coverage_rewriter
or you can follow the squeezed jbuild file and write

let flags = flags rewriters_ppx [...] |} flags coverage_rewriter

minishrink · 2018-01-30T16:44:04Z

network/network_interface.ml

+  if List.mem_assoc name config then
+    List.remove_assoc name config
+  else
+    config


A thought: List.remove_assoc doesn't throw any exceptions, but performs this check anyway, could just write this as List.remove_assoc name config?

Yes, will update.

minishrink · 2018-01-30T16:50:26Z

network/network_interface.ml

+    def = errors;
+    raiser = (function | e -> raise (NetworkError e));
+    matcher = function | NetworkError e -> Some e | _ -> None
+  }



If you like, you can shorten this by using the error functor defined here:

module NetworkErrorHandlerOrSomething = Error.Make(struct type t = errors val t = errors end) let err = NetworkErrorHandlerOrSomething.error

Good suggestion.

minishrink · 2018-01-30T17:11:57Z

network/network_interface.ml

+        "get_all"
+        ["Get all bridges"]
+        (debug_info_p @-> unit_p @-> returning result err)
+


Once you add a network debugging CLI (as seen in squeezed), the CLI will be confused by two functions both called get_all, even though they're part of different modules. I'm not sure how to get around this; perhaps by creating submodules with different Interface.namespace values? Though it isn't worth it for two different API calls. @jonludlam is there a way around this without renaming API calls?

minishrink · 2018-01-30T17:54:51Z

network/network_interface.ml

+      (debug_info_p @-> name_p @-> returning unit_p err)
+
+  let set_dns_interface =
+    let name_p = Param.mk ~name:"name" ~description:["gateway name"] iface in


set_gateway_interface and set_dns_interface use the same parameter name_p, why not just declare it as a common parameter like debug_info_p, but name it gateway_name_p? You could also do the same elsewhere, e.g. bridge_name_p

debug_info_p and unit_p is in common use (nearly in every interface), but not for the gateway_name_p and bridge_name_p etc. I'd like to keep the interface definition clear to read and easy for maintenance so keep them local definition is better than move the definition to module level.

minishrink · 2018-01-30T18:10:56Z

network/network_interface.ml

+        (debug_info_p @-> unit_p @-> returning iface_list_p err)
+
+    let exists =
+      let result = Param.mk ~description:["existance"] Types.bool in


Typo for "existence"

minishrink · 2018-01-30T18:11:47Z

network/network_interface.ml

+      let result = Param.mk ~description:["interface is up"] Types.bool in
+      declare
+        "is_up"
+        ["Interface is up"]


This could be more explicit: "Checks whether the interface is up".

minishrink · 2018-01-30T18:25:13Z

network/network_interface.ml

+      let module T = struct
+        type _dns_info_t = Unix.inet_addr list * string list [@@deriving rpcty]
+      end in
+      let result = Param.mk ~description:["DNS servers infomation"] T._dns_info_t in


Typo on information

minishrink · 2018-01-30T18:43:14Z

network/network_interface.ml

+  if List.mem_assoc name config = false then
+    default
+  else
+    List.assoc name config



This could be simplified like so, though I'm not sure which style is preferred in this repo:

try List.assoc name config with _ -> default

I prefer keep it as is.

If you want to keep it as it is, please do

if not List.mem_assoc name config then default else List.assoc name config

I personally also prefer the @minishrink one that avoids a lookup.

Signed-off-by: Yang Qian <yang.qian@citrix.com>

1. all types deriving from rpcty 2. remove redundent rpc functions, replace them with `@@default` rewriter 3. rename `Basic` of `string_of_port_kind` to `Basic_port` 4. change type `ipv4_route_t` from list to a struct type 5. move exception definition to xcp-networkd 6. declare interface with new style code by using `Idl.declare` 7. replace optional parameter with position parameter or position option parameter, remove the redundent suffix unit parameter accordingly 8. replace `rpc_of_x` and `x_of_rpc` with `Rpcmarshal.marshal` and `Rpamarshal.unmarshal` 9. update jbuilder file, remove redundant contents Signed-off-by: Yang Qian <yang.qian@citrix.com>

krizex · 2018-02-09T08:12:28Z

Updated with changes of xapi and xcp-networkd

mseri

LGTM but I have a couple of minor comments.

mseri · 2018-02-09T11:15:45Z

network/network_client.ml

-	  call
-      )
-end)
+let rpc call =


In the other modules we are moving away from global openings. Please remove open Network_interface and open Xcp_client, see e.g. https://github.com/xapi-project/xcp-idl/blob/master/gpumon/gpumon_client.ml

mseri · 2018-02-09T11:18:50Z

network/network_interface.ml

+      rpc_of = (fun t -> Rpc.String (Unix.string_of_inet_addr t));
+      of_rpc = (function
+          | Rpc.String s -> Ok (Unix.inet_addr_of_string s)
+          | _ -> Error (`Msg "Expecting value of string type when unmarshalling inet_addr"));


Be more explicit:

| r -> Error(`Msg (Printf.printf "typ_of_inet_addr: expected rpc string, got %s" (Rpc.to_string r))

mseri · 2018-02-09T11:22:29Z

Sorry, I meant to press the Comment button. With the two changes above I think it's good. But don't merge it unti testing is complete

minishrink · 2018-02-09T11:34:22Z

I have a question regarding your xcp-networkd PR: I noticed that network_server includes some API calls that were never in xcp-idl/network_interface.ml to begin with, such as set_ipv6_conf, get/set_ipv6_gateway, set_dns, set_ipv4_routes, etc. Just wondering: should these be added?

Disclaimer: I don't know much about networkd, so this might be an unnecessary question.

robhoes · 2018-02-09T13:42:10Z

@minishrink Those functions were actually part of the interface, but I have recently removed them because they were unused, and I wanted to simplify the interface. The functions are, however, used internally in the Interface module inside the server (especially by Interface.make_config).

mseri

Look at #204 for reference

mseri · 2018-02-19T11:39:42Z

network/network_interface.ml

+  | Not_implemented (** [Not_implemented] is reported if the interface is not implemented *)
+  | Vlan_in_use of (string * int) (** [Vlan_in_use (bridge, vlan_id)] is reported when [vlan_id] on [bridge] is inuse *)
+  | PVS_proxy_connection_error (** [PVS_proxy_connection_error] is reported when unable to connect PVS proxy *)
+  | Unknown_error (** The default variant for forward compatibility. *)


Please add Internal_error of string

Sure, thanks.

mseri · 2018-02-19T11:41:50Z

network/network_interface.ml

+let err = Error.{
+    def = errors;
+    raiser = (function | e -> raise (Network_error e));
+    matcher = function | Network_error e -> Some e | _ -> None


Change the last match with

| e -> Internal_error (Printexc.to_string e)

Sure, thanks.

Signed-off-by: Yang Qian <yang.qian@citrix.com>

mseri · 2018-02-22T09:53:50Z

@krizex has this been built in Jenkins in a private branch and tested (BVT + BST)?

krizex · 2018-02-23T03:13:30Z

@mseri I have executed network regression and some bond cases need to triage. BVT is on the way.

mseri · 2018-02-23T09:29:16Z

Good, please run also the ring3 BST

krizex · 2018-02-26T02:06:06Z

@mseri
Ring3 BVT passed in job 79965
Ring3 BST passed job 79962
Network regression passed job 80071 except infrastructure issues of bond cases on BOURNE04, I have spoke it to Rama and got confirmed, the ticket has been assigned to him.

krizex · 2018-02-27T06:07:36Z

Remove label of don't merge yet

minishrink

Looks ready for merging, unless @jonludlam can think of any PPX/ocaml-rpc changes to wait for.

mseri · 2018-02-27T12:36:50Z

I am going to do a manual merge to master and close the PR to ppx_network

krizex requested review from jonludlam and robhoes January 17, 2018 07:57

jonludlam reviewed Jan 17, 2018

View reviewed changes

mseri reviewed Jan 17, 2018

View reviewed changes

krizex force-pushed the private/yangqi/CP-26352 branch from 728ab6b to 2442376 Compare January 18, 2018 05:27

mseri requested changes Jan 19, 2018

View reviewed changes

krizex force-pushed the private/yangqi/CP-26352 branch from 2442376 to 7965aba Compare January 22, 2018 10:21

mseri reviewed Jan 22, 2018

View reviewed changes

krizex force-pushed the private/yangqi/CP-26352 branch from 7965aba to 13b4b07 Compare January 23, 2018 08:19

mseri added the don't merge yet label Jan 23, 2018

minishrink suggested changes Jan 30, 2018

View reviewed changes

minishrink reviewed Jan 30, 2018

View reviewed changes

krizex force-pushed the private/yangqi/CP-26352 branch from e300961 to 38f4d1a Compare February 1, 2018 09:57

krizex added 2 commits February 9, 2018 11:22

ocp-indent network interface and related stuffs

c437400

Signed-off-by: Yang Qian <yang.qian@citrix.com>

krizex force-pushed the private/yangqi/CP-26352 branch from 7fe398a to 9078bd4 Compare February 9, 2018 05:55

This was referenced Feb 9, 2018

CP-26352 Port xcp-networkd from Camlp4 to PPX xapi-project/xcp-networkd#128

Closed

CP-26685 Adapt network interface when porting to ppx xapi-project/xen-api#3439

Closed

mseri approved these changes Feb 9, 2018

View reviewed changes

mseri requested changes Feb 20, 2018

View reviewed changes

fixup! CP-26352 Port xcp-idl/network interface from Camlp4 to PPX

47a25db

Signed-off-by: Yang Qian <yang.qian@citrix.com>

krizex removed the don't merge yet label Feb 27, 2018

minishrink approved these changes Feb 27, 2018

View reviewed changes

mseri closed this Feb 27, 2018


		let unit_p = Param.mk Types.unit

		(* external clear_state: unit -> unit = "" *)

CP-26352 Port xcp-idl/network interface from Camlp4 to PPX #189

CP-26352 Port xcp-idl/network interface from Camlp4 to PPX #189

Conversation

krizex commented Jan 17, 2018 • edited Loading

krizex commented Jan 17, 2018

coveralls commented Jan 17, 2018 • edited Loading

robhoes commented Jan 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krizex commented Jan 17, 2018 via email

mseri Jan 17, 2018 • edited Loading

Choose a reason for hiding this comment

krizex Jan 18, 2018 • edited Loading

Choose a reason for hiding this comment

mseri Jan 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krizex Feb 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 18, 2018 • edited Loading

coveralls commented Jan 18, 2018 • edited Loading

coveralls commented Jan 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mseri Jan 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mseri Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krizex commented Jan 22, 2018

mseri Jan 22, 2018 • edited Loading

Choose a reason for hiding this comment

krizex commented Jan 23, 2018

mseri commented Jan 23, 2018 • edited Loading

krizex commented Jan 24, 2018 • edited Loading

minishrink left a comment

Choose a reason for hiding this comment

minishrink Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krizex Jan 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krizex Jan 31, 2018 • edited Loading

Choose a reason for hiding this comment

minishrink Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krizex commented Jan 17, 2018 •

edited

Loading

coveralls commented Jan 17, 2018 •

edited

Loading

mseri Jan 17, 2018 •

edited

Loading

krizex Jan 18, 2018 •

edited

Loading

mseri Jan 18, 2018 •

edited

Loading

krizex Feb 1, 2018 •

edited

Loading

coveralls commented Jan 18, 2018 •

edited

Loading

coveralls commented Jan 18, 2018 •

edited

Loading

coveralls commented Jan 18, 2018 •

edited

Loading

mseri Jan 19, 2018 •

edited

Loading

mseri Jan 22, 2018 •

edited

Loading

mseri Jan 22, 2018 •

edited

Loading

mseri commented Jan 23, 2018 •

edited

Loading

krizex commented Jan 24, 2018 •

edited

Loading

minishrink Jan 30, 2018 •

edited

Loading

krizex Jan 31, 2018 •

edited

Loading

krizex Jan 31, 2018 •

edited

Loading

minishrink Jan 30, 2018 •

edited

Loading

mseri left a comment •

edited

Loading

minishrink commented Feb 9, 2018 •

edited

Loading