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

Re: [Xen-devel] [PATCH 3/7] oxenstored: refactor request processing



On 03/29/2016 05:08 AM, Jonathan Davies wrote:
On Thu, Mar 24, 2016 at 07:57:30PM -0400, Boris Ostrovsky wrote:
On 03/24/2016 06:49 PM, Andrew Cooper wrote:
On 24/03/2016 22:22, Boris Ostrovsky wrote:
On 03/17/2016 01:51 PM, Jonathan Davies wrote:
Encapsulate the request in a record that is passed from do_input to
process_packet and input_handle_error.

This will be helpful when keeping track of the requests made as part
of a
transaction.

Signed-off-by: Jonathan Davies <jonathan.davies@xxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Reviewed-by: Jon Ludlam <jonathan.ludlam@xxxxxxxxxx>
Reviewed-by: Euan Harris <euan.harris@xxxxxxxxxx>
---
   tools/ocaml/xenstored/process.ml |   15 ++++++++++-----
   1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/tools/ocaml/xenstored/process.ml
b/tools/ocaml/xenstored/process.ml
index 7a73669..c92bec7 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -344,11 +344,11 @@ let function_of_type ty =
       | Xenbus.Xb.Op.Invalid           -> reply_ack do_error
       | _                              -> reply_ack do_error
   -let input_handle_error ~cons ~doms ~fct ~ty ~con ~t ~rid ~data =
+let input_handle_error ~cons ~doms ~fct ~con ~t ~req =
       let reply_error e =
           Packet.Error e in
       try
-        fct con t doms cons data
+        fct con t doms cons req.Packet.data
       with
       | Define.Invalid_path          -> reply_error "EINVAL"
       | Define.Already_exist         -> reply_error "EEXIST"
@@ -370,7 +370,10 @@ let input_handle_error ~cons ~doms ~fct ~ty ~con
~t ~rid ~data =
   (**
    * Nothrow guarantee.
    *)
-let process_packet ~store ~cons ~doms ~con ~tid ~rid ~ty ~data =
+let process_packet ~store ~cons ~doms ~con ~req =
+    let ty = req.Packet.ty in
+    let tid = req.Packet.tid in
+    let rid = req.Packet.rid in
       try
           let fct = function_of_type ty in
           let t =
@@ -379,7 +382,7 @@ let process_packet ~store ~cons ~doms ~con ~tid
~rid ~ty ~data =
               else
                   Connection.get_transaction con tid
               in
-        let response = input_handle_error ~cons ~doms ~fct ~ty ~con
~t ~rid ~data in
+        let response = input_handle_error ~cons ~doms ~fct ~con ~t
~req in
             (* Put the response on the wire *)
           send_response ty con t rid response
@@ -412,11 +415,13 @@ let do_input store cons doms con =
       if newpacket then (
           let packet = Connection.pop_in con in
           let tid, rid, ty, data = Xenbus.Xb.Packet.unpack packet in
+        let req = {Packet.tid; Packet.rid; Packet.ty; Packet.data} in
+
I think this change breaks the build with older ocaml versions:

root@haswell> ocamlopt -v
The OCaml native-code compiler, version 4.00.1
Standard library directory: /usr/lib64/ocaml
root@haswell> ocamlopt -g -ccopt "    " -dtypes -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/xb -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/mmap -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/xc -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/eventchn -cc gcc -w F
-warn-error F -c -o process.cmx process.ml
root@haswell>


root@ovs104> ocamlopt -v
The Objective Caml native-code compiler, version 3.11.2
Standard library directory: /usr/lib64/ocaml
root@ovs104> ocamlopt -g -ccopt "    " -dtypes -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/xb -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/mmap -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/xc -I
/root/tmp/xen/tools/ocaml/xenstored/../libs/eventchn -cc gcc -w F
-warn-error F -c -o process.cmx process.ml
File "process.ml", line 487, characters 23-24:
Error: Syntax error
root@ovs104>

I don't know much about ocaml (OK, I know *nothing* about ocaml) so I
can't say what exactly might be wrong.
Could you perhaps try this instead?

let req = {tid = Packet.tid; rid = Packet.rid; ty = Packet.ty; data =
Packet.data} in

It is most likely that the older version of Ocaml can't infer the order
of fields.

~Andrew

No, it now gives me "Error: Unbound record field label tid"

-boris
Andrew's guess was close, but the wrong way around -- please could you try the
following with the older compiler?

let req = {Packet.tid=tid; Packet.rid=rid; Packet.ty=ty; Packet.data=data} in

I was using a syntactic feature of OCaml called 'field punning' which is
generally considered good practice and makes for more readable code. It looks
like this feature was introduced in OCaml 3.12.0 (dating from 2010), which is
consistent with Boris' findings.

Yes, this fixes it, thanks.

What's the policy here -- is there a defined version of the OCaml compiler which
tools/ocaml needs to be able to compile with?

Not to answer this question directly but more as an FYI --- this version of ocaml is found on Oracle Linux 6.2, which AFAIK is actively supported.

-boris


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

 


Rackspace

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