Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Wei Liu]

On Tue, Mar 13, 2018 at 09:29:49AM +, Christian Lindig wrote:
> On 12/03/18 19:35, Michael Young wrote:
> 
> > Here is version 4 of the patch where I have replaced the uses of s with b
> > where the patch changes it from string to bytes. I have also removed the
> > two trailing spaces and changed stmp back to s.
> >  Michael Young
> > 0001-make-xen-ocaml-safe-strings-compliant.patch
> > 
> >  From da088e4eef2bbea4be262e12db4c36960ff5145a Mon Sep 17 00:00:00 2001
> > From: Michael Young
> > Date: Mon, 12 Mar 2018 18:49:29 +
> > Subject: [PATCH v4] make xen ocaml safe-strings compliant
> > 
> > Xen built with ocaml 4.06 gives errors such as
> > Error: This expression has type bytes but an expression was
> >  expected of type string
> > as Byte and safe-strings which were introduced in 4.02 are the
> > default in 4.06.
> > This patch which is partly by Richard W.M. Jones of Red Hat
> > fromhttps://bugzilla.redhat.com/show_bug.cgi?id=1526703
> > fixes these issues.
> > 
> > v4: Where string s is now bytes, rename it to b.
> > 
> > Signed-off-by: Michael Young
> > ---
> >   tools/ocaml/libs/xb/xb.ml| 34 ++
> >   tools/ocaml/libs/xb/xb.mli   | 10 +-
> >   tools/ocaml/xenstored/logging.ml | 22 +++---
> >   tools/ocaml/xenstored/stdext.ml  |  2 +-
> >   tools/ocaml/xenstored/utils.ml   | 20 ++--
> >   5 files changed, 45 insertions(+), 43 deletions(-)
> 
> Reviewed-by: Christian Lindig

Thanks. I will apply this patch soon.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Christian Lindig]

On 12/03/18 19:35, Michael Young wrote:


Here is version 4 of the patch where I have replaced the uses of s with b
where the patch changes it from string to bytes. I have also removed the
two trailing spaces and changed stmp back to s.
 Michael Young
0001-make-xen-ocaml-safe-strings-compliant.patch

 From da088e4eef2bbea4be262e12db4c36960ff5145a Mon Sep 17 00:00:00 2001
From: Michael Young
Date: Mon, 12 Mar 2018 18:49:29 +
Subject: [PATCH v4] make xen ocaml safe-strings compliant

Xen built with ocaml 4.06 gives errors such as
Error: This expression has type bytes but an expression was
 expected of type string
as Byte and safe-strings which were introduced in 4.02 are the
default in 4.06.
This patch which is partly by Richard W.M. Jones of Red Hat
fromhttps://bugzilla.redhat.com/show_bug.cgi?id=1526703
fixes these issues.

v4: Where string s is now bytes, rename it to b.

Signed-off-by: Michael Young
---
  tools/ocaml/libs/xb/xb.ml| 34 ++
  tools/ocaml/libs/xb/xb.mli   | 10 +-
  tools/ocaml/xenstored/logging.ml | 22 +++---
  tools/ocaml/xenstored/stdext.ml  |  2 +-
  tools/ocaml/xenstored/utils.ml   | 20 ++--
  5 files changed, 45 insertions(+), 43 deletions(-)


Reviewed-by: Christian Lindig

This patch is good to go. One caveat, though: the conversion between bytes and 
strings
is not just a cast but involves copying and thus has a performance cost which we
want to keep in mind. The OCaml low-level libraries like Unix.read return 
(mutable) bytes,
but most code expects (immutable) strings. I believe that it would be difficult 
to avoid
this in the long run and this patch is the right way to go.

-- Christian
 



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Michael Young]

On Mon, 12 Mar 2018, Christian Lindig wrote:


 The problem with the old patch is illustrated by the following section
 from the old patch for tools/ocaml/xenstored/utils.ml
 @@ -85,7 +85,7 @@ let create_unix_socket name =
  let read_file_single_integer filename =
     let fd = Unix.openfile filename [ Unix.O_RDONLY ] 0o640 in
     let buf = String.make 20 (char_of_int 0) in
 -   let sz = Unix.read fd buf 0 20 in
 +   let sz = Unix.read fd (Bytes.of_string buf) 0 20 in
     Unix.close fd;
     int_of_string (String.sub buf 0 sz)

 where the patch makes Unix.read write to a Bytes copy of buf and buf
 itself is unchanged, so int_of_string sees a string of null characters
 rather than a string to convert into a number.


Good analysis. (Bytes.of_string buf) created a buffer for the result from 
read() for which we have no handle.


Reviewing the new patch I believe it is sound. The (new) signature of 
read_mmap is



 val read_mmap : backend_mmap -> 'a -> bytes -> int -> int


The new implementation is below - argument s used to be a string value and is 
now a bytes value. I would suggest to reflect this in the names (using b 
instead of s) as this is about conversion between strings and bytes.

   let read_mmap back con s len =
 -  let rd = Xs_ring.read back.mmap s len in
 +  let stmp = String.make len (char_of_int 0) in
 +  let rd = Xs_ring.read back.mmap stmp len in
 +  Bytes.blit_string stmp 0 s 0 rd;
back.work_again <- (rd > 0);
if rd > 0 then
 back.eventchn_notify ();


Below are the functions that changed their type and where this also should be 
considered:

 -val read_fd : backend_fd -> 'a -> string -> int -> int
 -val read_mmap : backend_mmap -> 'a -> string -> int -> int
 -val read : t -> string -> int -> int
 -val write_fd : backend_fd -> 'a -> string -> int -> int
 +val read_fd : backend_fd -> 'a -> bytes -> int -> int
 +val read_mmap : backend_mmap -> 'a -> bytes -> int -> int
 +val read : t -> bytes -> int -> int
 +val write_fd : backend_fd -> 'a -> bytes -> int -> int


-- Christian


Here is version 4 of the patch where I have replaced the uses of s with b 
where the patch changes it from string to bytes. I have also removed the 
two trailing spaces and changed stmp back to s.


Michael YoungFrom da088e4eef2bbea4be262e12db4c36960ff5145a Mon Sep 17 00:00:00 2001
From: Michael Young 
Date: Mon, 12 Mar 2018 18:49:29 +
Subject: [PATCH v4] make xen ocaml safe-strings compliant

Xen built with ocaml 4.06 gives errors such as
Error: This expression has type bytes but an expression was
expected of type string
as Byte and safe-strings which were introduced in 4.02 are the
default in 4.06.
This patch which is partly by Richard W.M. Jones of Red Hat
from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
fixes these issues.

v4: Where string s is now bytes, rename it to b.

Signed-off-by: Michael Young 
---
 tools/ocaml/libs/xb/xb.ml| 34 ++
 tools/ocaml/libs/xb/xb.mli   | 10 +-
 tools/ocaml/xenstored/logging.ml | 22 +++---
 tools/ocaml/xenstored/stdext.ml  |  2 +-
 tools/ocaml/xenstored/utils.ml   | 20 ++--
 5 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml
index 50944b5fd6..519842723b 100644
--- a/tools/ocaml/libs/xb/xb.ml
+++ b/tools/ocaml/libs/xb/xb.ml
@@ -40,7 +40,7 @@ type backend_fd =
 
 type backend = Fd of backend_fd | Xenmmap of backend_mmap
 
-type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * string
+type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * bytes
 
 type t =
 {
@@ -52,7 +52,7 @@ type t =
 }
 
 let init_partial_in () = NoHdr
-   (Partial.header_size (), String.make (Partial.header_size()) '\000')
+   (Partial.header_size (), Bytes.make (Partial.header_size()) '\000')
 
 let reconnect t = match t.backend with
| Fd _ ->
@@ -69,26 +69,28 @@ let reconnect t = match t.backend with
 
 let queue con pkt = Queue.push pkt con.pkt_out
 
-let read_fd back con s len =
-   let rd = Unix.read back.fd s 0 len in
+let read_fd back con b len =
+   let rd = Unix.read back.fd b 0 len in
if rd = 0 then
raise End_of_file;
rd
 
-let read_mmap back con s len =
+let read_mmap back con b len =
+   let s = String.make len (char_of_int 0) in
let rd = Xs_ring.read back.mmap s len in
+   Bytes.blit_string s 0 b 0 rd;
back.work_again <- (rd > 0);
if rd > 0 then
back.eventchn_notify ();
rd
 
-let read con s len =
+let read con b len =
match con.backend with
-   | Fd backfd -> read_fd backfd con s len
-   | Xenmmap backmmap -> read_mmap backmmap con s len
+   | Fd backfd -> read_fd backfd con b len
+   | Xenmmap backmmap -> read_mmap backmmap con b len
 
-let write_fd back con s len =
-  

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Christian Lindig]

The problem with the old patch is illustrated by the following section 
from the old patch for tools/ocaml/xenstored/utils.ml

@@ -85,7 +85,7 @@ let create_unix_socket name =
 let read_file_single_integer filename =
    let fd = Unix.openfile filename [ Unix.O_RDONLY ] 0o640 in
    let buf = String.make 20 (char_of_int 0) in
-   let sz = Unix.read fd buf 0 20 in
+   let sz = Unix.read fd (Bytes.of_string buf) 0 20 in
    Unix.close fd;
    int_of_string (String.sub buf 0 sz)

where the patch makes Unix.read write to a Bytes copy of buf and buf 
itself is unchanged, so int_of_string sees a string of null characters 
rather than a string to convert into a number.


Good analysis. (Bytes.of_string buf) created a buffer for the result 
from read() for which we have no handle.


Reviewing the new patch I believe it is sound. The (new) signature of 
read_mmap is



val read_mmap : backend_mmap -> 'a -> bytes -> int -> int


The new implementation is below - argument s used to be a string value 
and is now a bytes value. I would suggest to reflect this in the names 
(using b instead of s) as this is about conversion between strings and 
bytes.

  let read_mmap back con s len =
-   let rd = Xs_ring.read back.mmap s len in
+   let stmp = String.make len (char_of_int 0) in
+   let rd = Xs_ring.read back.mmap stmp len in
+   Bytes.blit_string stmp 0 s 0 rd;
back.work_again <- (rd > 0);
if rd > 0 then
back.eventchn_notify ();


Below are the functions that changed their type and where this also 
should be considered:

-val read_fd : backend_fd -> 'a -> string -> int -> int
-val read_mmap : backend_mmap -> 'a -> string -> int -> int
-val read : t -> string -> int -> int
-val write_fd : backend_fd -> 'a -> string -> int -> int
+val read_fd : backend_fd -> 'a -> bytes -> int -> int
+val read_mmap : backend_mmap -> 'a -> bytes -> int -> int
+val read : t -> bytes -> int -> int
+val write_fd : backend_fd -> 'a -> bytes -> int -> int


-- Christian


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Michael Young]

On Fri, 9 Mar 2018, Christian Lindig wrote:





On 9. Mar 2018, at 22:57, Michael Young  wrote:

I have had a go at fixing the patch and my revised attempt is attached. I 
suspect it could be tidied up, but it works for me.


Thank you for giving this another go. What is the key difference to the 
previous patch which compiled but lead to lock up at run time? I briefly tried 
your patch and can confirm that it compiles and looks clean except for two 
trailing spaces.

— Christian

Acked-by: Christian Lindig 


The problem with the old patch is illustrated by the following section 
from the old patch for tools/ocaml/xenstored/utils.ml

@@ -85,7 +85,7 @@ let create_unix_socket name =
 let read_file_single_integer filename =
let fd = Unix.openfile filename [ Unix.O_RDONLY ] 0o640 in
let buf = String.make 20 (char_of_int 0) in
-   let sz = Unix.read fd buf 0 20 in
+   let sz = Unix.read fd (Bytes.of_string buf) 0 20 in
Unix.close fd;
int_of_string (String.sub buf 0 sz)

where the patch makes Unix.read write to a Bytes copy of buf and buf 
itself is unchanged, so int_of_string sees a string of null characters 
rather than a string to convert into a number. The net result is that 
information being read by oxenstored from the hypervisor is corrupted or 
lost.
The same basic problem also occurred in a couple of places in 
the old patch of tools/ocaml/libs/xb/xb.ml.
My fix for this is to switch to Bytes at an earlier stage, so for example 
the corresponding section in the new patch becomes

@@ -84,10 +84,10 @@ let create_unix_socket name =

 let read_file_single_integer filename =
let fd = Unix.openfile filename [ Unix.O_RDONLY ] 0o640 in
-   let buf = String.make 20 (char_of_int 0) in
+   let buf = Bytes.make 20 (char_of_int 0) in
let sz = Unix.read fd buf 0 20 in
Unix.close fd;
-   int_of_string (String.sub buf 0 sz)
+   int_of_string (Bytes.to_string (Bytes.sub buf 0 sz))

 let path_complete path connection_path =
if String.get path 0 <> '/' then

Michael Young___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Christian Lindig]

> On 9. Mar 2018, at 22:57, Michael Young  wrote:
> 
> I have had a go at fixing the patch and my revised attempt is attached. I 
> suspect it could be tidied up, but it works for me.

Thank you for giving this another go. What is the key difference to the 
previous patch which compiled but lead to lock up at run time? I briefly tried 
your patch and can confirm that it compiles and looks clean except for two 
trailing spaces. 

— Christian

Acked-by: Christian Lindig 



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Michael Young]

On Mon, 12 Feb 2018, Wei Liu wrote:


On Fri, Feb 09, 2018 at 09:20:33AM +, Christian Lindig wrote:




On 8. Feb 2018, at 18:24, Wei Liu  wrote:

Christian, do you have any idea when you can look into fixing the
safe-string patch?


Sorry, I can’t make a promise because of my other obligations. I do wonder, 
though: this patch did not come out of nowhere but supposedly was working - 
what is different here?



No worries. I have reverted some patches in xen.git to get things going
again.


I have had a go at fixing the patch and my revised attempt is attached. 
I suspect it could be tidied up, but it works for me.


Michael YoungFrom 550ffe177842e3fd9f38c78e07072fa7c7b591a5 Mon Sep 17 00:00:00 2001
From: Michael Young 
Date: Fri, 9 Mar 2018 22:31:41 +
Subject: [PATCH v3] make xen ocaml safe-strings compliant

Xen built with ocaml 4.06 gives errors such as
Error: This expression has type bytes but an expression was
expected of type string
as Byte and safe-strings which were introduced in 4.02 are the
default in 4.06.
This patch which is partly by Richard W.M. Jones of Red Hat
from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
fixes these issues.

v3: rework patches for xb.ml and /utils.ml to fix broken code relating
to Unix.read.  Update xb.mli to match changes in xb.ml.

Signed-off-by: Michael Young 

---
 tools/ocaml/libs/xb/xb.ml| 18 ++
 tools/ocaml/libs/xb/xb.mli   | 10 +-
 tools/ocaml/xenstored/logging.ml | 22 +++---
 tools/ocaml/xenstored/stdext.ml  |  2 +-
 tools/ocaml/xenstored/utils.ml   | 20 ++--
 5 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml
index 50944b5fd6..42ae8d2bd8 100644
--- a/tools/ocaml/libs/xb/xb.ml
+++ b/tools/ocaml/libs/xb/xb.ml
@@ -40,7 +40,7 @@ type backend_fd =
 
 type backend = Fd of backend_fd | Xenmmap of backend_mmap
 
-type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * string
+type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * bytes
 
 type t =
 {
@@ -52,7 +52,7 @@ type t =
 }
 
 let init_partial_in () = NoHdr
-   (Partial.header_size (), String.make (Partial.header_size()) '\000')
+   (Partial.header_size (), Bytes.make (Partial.header_size()) '\000')
 
 let reconnect t = match t.backend with
| Fd _ ->
@@ -76,7 +76,9 @@ let read_fd back con s len =
rd
 
 let read_mmap back con s len =
-   let rd = Xs_ring.read back.mmap s len in
+   let stmp = String.make len (char_of_int 0) in
+   let rd = Xs_ring.read back.mmap stmp len in
+   Bytes.blit_string stmp 0 s 0 rd;
back.work_again <- (rd > 0);
if rd > 0 then
back.eventchn_notify ();
@@ -98,7 +100,7 @@ let write_mmap back con s len =
 
 let write con s len =
match con.backend with
-   | Fd backfd -> write_fd backfd con s len
+   | Fd backfd -> write_fd backfd con (Bytes.of_string s) len
| Xenmmap backmmap -> write_mmap backmmap con s len
 
 (* NB: can throw Reconnect *)
@@ -129,7 +131,7 @@ let input con =
| NoHdr   (i, buf)-> i in
 
(* try to get more data from input stream *)
-   let s = String.make to_read '\000' in
+   let s = Bytes.make to_read '\000' in
let sz = if to_read > 0 then read con s to_read else 0 in
 
(
@@ -137,7 +139,7 @@ let input con =
| HaveHdr partial_pkt ->
(* we complete the data *)
if sz > 0 then
-   Partial.append partial_pkt s sz;
+   Partial.append partial_pkt (Bytes.to_string s) sz;
if Partial.to_complete partial_pkt = 0 then (
let pkt = Packet.of_partialpkt partial_pkt in
con.partial_in <- init_partial_in ();
@@ -147,9 +149,9 @@ let input con =
| NoHdr (i, buf)  ->
(* we complete the partial header *)
if sz > 0 then
-   String.blit s 0 buf (Partial.header_size () - i) sz;
+   Bytes.blit s 0 buf (Partial.header_size () - i) sz;
con.partial_in <- if sz = i then
-   HaveHdr (Partial.of_string buf) else NoHdr (i - sz, buf)
+   HaveHdr (Partial.of_string (Bytes.to_string buf)) else 
NoHdr (i - sz, buf)
);
!newpacket
 
diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli
index b4d705201f..d566011fc7 100644
--- a/tools/ocaml/libs/xb/xb.mli
+++ b/tools/ocaml/libs/xb/xb.mli
@@ -65,7 +65,7 @@ type backend_mmap = {
 }
 type backend_fd = { fd : Unix.file_descr; }
 type backend = Fd of backend_fd | Xenmmap of backend_mmap
-type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * string
+type partial_buf = HaveHdr of Partial.pkt | NoHdr of int * bytes
 type t = {
   backend : backend;
   pkt_in : 

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Michael Young]

On Fri, 9 Feb 2018, Christian Lindig wrote:

Sorry, I can’t make a promise because of my other obligations. I do 
wonder, though: this patch did not come out of nowhere but supposedly 
was working - what is different here?


The patch was from Fedora and is broken there too! It fixes the build 
issues but it doesn't look like anyone tested it (I use xenstored).


I have been trying to narrow down where the problem is and I think there 
are actually two issues;
one in tools/ocaml/xenstored/utils.ml which I think causes an error like 
Fatal error: exception Failure("int_of_string")

and one in tools/ocaml/libs/xb/xb.ml which I think causes an error like
Fatal error: exception Failure("evtchn bind_interdomain failed")

Michael Young___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Wei Liu]

On Fri, Feb 09, 2018 at 09:20:33AM +, Christian Lindig wrote:
> 
> 
> > On 8. Feb 2018, at 18:24, Wei Liu  wrote:
> > 
> > Christian, do you have any idea when you can look into fixing the
> > safe-string patch?
> 
> Sorry, I can’t make a promise because of my other obligations. I do wonder, 
> though: this patch did not come out of nowhere but supposedly was working - 
> what is different here?
> 

No worries. I have reverted some patches in xen.git to get things going
again.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Dario Faggioli]

On Fri, 2018-02-09 at 09:20 +, Christian Lindig wrote:
> > On 8. Feb 2018, at 18:24, Wei Liu  wrote:
> > 
> > Christian, do you have any idea when you can look into fixing the
> > safe-string patch?
> 
> Sorry, I can’t make a promise because of my other obligations. I do
> wonder, though: this patch did not come out of nowhere but supposedly
> was working - what is different here?
> 
Well, the only thing that I know about OCAML is that it is, for me as
an Italian, a word a little bit difficult to pronounce. :-D

But I'm happy to try to give some more details on what fails and how,
if you direct me. :-)

The testbox where this happens is a Debian unstable which has ocaml
4.05.0-10.

> In any case, I will reproduce the problem and take a look.
> 
Andrew said on IRC that he's seen something similar happening on some
of yours XenRT runs... but really, I'm happy to provide more info.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Christian Lindig]

> On 8. Feb 2018, at 18:24, Wei Liu  wrote:
> 
> Christian, do you have any idea when you can look into fixing the
> safe-string patch?

Sorry, I can’t make a promise because of my other obligations. I do wonder, 
though: this patch did not come out of nowhere but supposedly was working - 
what is different here?

In any case, I will reproduce the problem and take a look.

The patch was doing the right thing for the future but there is no harm not 
having it right away. The entire XenServer toolstack is facing the same problem 
of moving to immutable strings and so far we have done it where it was easy and 
moved to use upstream libraries that use immutable string. In our own code, 
this is still a massive task.

— Christian
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Wei Liu]

On Thu, Feb 08, 2018 at 06:03:48PM +, Wei Liu wrote:
> On Thu, Feb 08, 2018 at 06:49:58PM +0100, Dario Faggioli wrote:
> > On Tue, 2018-01-30 at 22:55 +, Michael Young wrote:
> > > Xen built with ocaml 4.06 gives errors such as
> > > Error: This expression has type bytes but an expression was
> > >   expected of type string
> > > as Byte and safe-strings which were introduced in 4.02 are the
> > > default in 4.06.
> > > This patch which is partly by Richard W.M. Jones of Red Hat
> > > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
> > > fixes these issues.
> > > 
> > > Signed-off-by: Michael Young 
> > > Reviewed-by: Christian Lindig 
> > >
> > So, with this patch, oxenstord does not start for me.
> > 
> > Systemd says this (sorry, it's not the full output.. I don't have it
> > right now, but can produce it):
> > 
> > systemctl status xenstored
> >  ...
> >  Active: failed (Result: protocol) since Thu 2018-02-08 17:47:56 CET; 12min 
> > ago
> >  ...
> >  Feb 08 17:47:56 Zhaman systemd[1]: xenstored.service: Failed with result 
> > 'protocol'.
> > 
> > Just running oxenstored from command line seems to exits with 0, but
> > there's not an oxenstored process running.
> > 
> > Getting rid of what is now commit "make xen ocaml safe-strings
> > compliant" (df1e4c6e7f8892e950433ff33c215df0cd7b30f7), things work
> > again for me.
> > 
> 
> OK. I will revert the relevant commits in staging. I'm sure this will
> block osstest flights.
> 

Correction: osstest currently still runs Debian jessie, which has ocaml
4.01, which means the bump to 4.02 in staging is likely cause oxenstored
to be disabled. New flights could still pass but that's due to
oxenstored not getting tested. It is convoluted, I know. :-/

I need to at least revert the safe-string patches in staging. As for the
version bump, I'm not so sure. On one hand it is useless in its own and
leaving the bump in tree actually stops the testing of oxenstored, on
the other hand it is a must-have for safe-string patch, assuming we will
have a proper safe-string fix soon-ish we can leave them in staging.

Christian, do you have any idea when you can look into fixing the
safe-string patch?

And osstest should really be upgraded to strech (which has 4.02).

Wei.


> Wei.
> 
> > Regards,
> > Dario
> > 
> > PS. There has been a v2 of this patch, I think, but I don't have in my
> > emails any longer, so I'm replying to this one.
> > 
> > -- 
> > <> (Raistlin Majere)
> > -
> > Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> > Software Engineer @ SUSE https://www.suse.com/
> 
> 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Wei Liu]

On Thu, Feb 08, 2018 at 06:49:58PM +0100, Dario Faggioli wrote:
> On Tue, 2018-01-30 at 22:55 +, Michael Young wrote:
> > Xen built with ocaml 4.06 gives errors such as
> > Error: This expression has type bytes but an expression was
> > expected of type string
> > as Byte and safe-strings which were introduced in 4.02 are the
> > default in 4.06.
> > This patch which is partly by Richard W.M. Jones of Red Hat
> > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
> > fixes these issues.
> > 
> > Signed-off-by: Michael Young 
> > Reviewed-by: Christian Lindig 
> >
> So, with this patch, oxenstord does not start for me.
> 
> Systemd says this (sorry, it's not the full output.. I don't have it
> right now, but can produce it):
> 
> systemctl status xenstored
>  ...
>  Active: failed (Result: protocol) since Thu 2018-02-08 17:47:56 CET; 12min 
> ago
>  ...
>  Feb 08 17:47:56 Zhaman systemd[1]: xenstored.service: Failed with result 
> 'protocol'.
> 
> Just running oxenstored from command line seems to exits with 0, but
> there's not an oxenstored process running.
> 
> Getting rid of what is now commit "make xen ocaml safe-strings
> compliant" (df1e4c6e7f8892e950433ff33c215df0cd7b30f7), things work
> again for me.
> 

OK. I will revert the relevant commits in staging. I'm sure this will
block osstest flights.

Wei.

> Regards,
> Dario
> 
> PS. There has been a v2 of this patch, I think, but I don't have in my
> emails any longer, so I'm replying to this one.
> 
> -- 
> <> (Raistlin Majere)
> -
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Software Engineer @ SUSE https://www.suse.com/



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Dario Faggioli]

On Tue, 2018-01-30 at 22:55 +, Michael Young wrote:
> Xen built with ocaml 4.06 gives errors such as
> Error: This expression has type bytes but an expression was
>   expected of type string
> as Byte and safe-strings which were introduced in 4.02 are the
> default in 4.06.
> This patch which is partly by Richard W.M. Jones of Red Hat
> from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
> fixes these issues.
> 
> Signed-off-by: Michael Young 
> Reviewed-by: Christian Lindig 
>
So, with this patch, oxenstord does not start for me.

Systemd says this (sorry, it's not the full output.. I don't have it
right now, but can produce it):

systemctl status xenstored
 ...
 Active: failed (Result: protocol) since Thu 2018-02-08 17:47:56 CET; 12min ago
 ...
 Feb 08 17:47:56 Zhaman systemd[1]: xenstored.service: Failed with result 
'protocol'.

Just running oxenstored from command line seems to exits with 0, but
there's not an oxenstored process running.

Getting rid of what is now commit "make xen ocaml safe-strings
compliant" (df1e4c6e7f8892e950433ff33c215df0cd7b30f7), things work
again for me.

Regards,
Dario

PS. There has been a v2 of this patch, I think, but I don't have in my
emails any longer, so I'm replying to this one.

-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/

signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Wei Liu]

On Tue, Feb 06, 2018 at 09:56:14PM +, Michael Young wrote:
> On Tue, 6 Feb 2018, Wei Liu wrote:
> 
> > On Tue, Jan 30, 2018 at 10:55:47PM +, Michael Young wrote:
> > > Xen built with ocaml 4.06 gives errors such as
> > > Error: This expression has type bytes but an expression was
> > >   expected of type string
> > > as Byte and safe-strings which were introduced in 4.02 are the
> > > default in 4.06.
> > > This patch which is partly by Richard W.M. Jones of Red Hat
> > > from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
> > > fixes these issues.
> > > 
> > > Signed-off-by: Michael Young 
> > > Reviewed-by: Christian Lindig 
> > 
> > Strangely this doesn't apply to staging. And after examining the
> > downloaded patch I'm not sure if my mail client is acting up. Do you
> > have a git branch that I can pull from?
> 
> The patch needed to be reduced as one of the sections being patched was
> removed by a recent patch. I am attaching the revised patch as a file in
> case there was also an email formatting issue.
> 

Thanks, I will try this today.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Michael Young]

On Tue, 6 Feb 2018, Wei Liu wrote:


On Tue, Jan 30, 2018 at 10:55:47PM +, Michael Young wrote:

Xen built with ocaml 4.06 gives errors such as
Error: This expression has type bytes but an expression was
expected of type string
as Byte and safe-strings which were introduced in 4.02 are the
default in 4.06.
This patch which is partly by Richard W.M. Jones of Red Hat
from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
fixes these issues.

Signed-off-by: Michael Young 
Reviewed-by: Christian Lindig 


Strangely this doesn't apply to staging. And after examining the
downloaded patch I'm not sure if my mail client is acting up. Do you
have a git branch that I can pull from?


The patch needed to be reduced as one of the sections being patched was 
removed by a recent patch. I am attaching the revised patch as a file 
in case there was also an email formatting issue.


Michael YoungFrom 247cea9b587baafaf80bcc5b44bc68defb4efa26 Mon Sep 17 00:00:00 2001
From: Michael Young 
Date: Tue, 6 Feb 2018 21:27:23 +
Subject: [PATCH 1/2 v2] make xen ocaml safe-strings compliant

Xen built with ocaml 4.06 gives errors such as
Error: This expression has type bytes but an expression was
expected of type string
as Byte and safe-strings which were introduced in 4.02 are the
default in 4.06.
This patch which is mostly by Richard W.M. Jones of Red Hat
from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
fixes these issues.

v2: drop tools/ocaml/libs/xc/xenctrl.ml from the patch as the
affected code was removed by commit d933f1a53c06002351c1e36d40615e40bd4bf6af
tools/ocaml: Drop coredump infrastructure

Signed-off-by: Michael Young 
Reviewed-by: Christian Lindig 
---
 tools/ocaml/libs/xb/xb.ml|  6 +++---
 tools/ocaml/xenstored/logging.ml | 22 +++---
 tools/ocaml/xenstored/stdext.ml  |  2 +-
 tools/ocaml/xenstored/utils.ml   | 18 +-
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml
index 50944b5fd6..aa2cf98223 100644
--- a/tools/ocaml/libs/xb/xb.ml
+++ b/tools/ocaml/libs/xb/xb.ml
@@ -84,7 +84,7 @@ let read_mmap back con s len =
 
 let read con s len =
match con.backend with
-   | Fd backfd -> read_fd backfd con s len
+   | Fd backfd -> read_fd backfd con (Bytes.of_string s) len
| Xenmmap backmmap -> read_mmap backmmap con s len
 
 let write_fd back con s len =
@@ -98,7 +98,7 @@ let write_mmap back con s len =
 
 let write con s len =
match con.backend with
-   | Fd backfd -> write_fd backfd con s len
+   | Fd backfd -> write_fd backfd con (Bytes.of_string s) len
| Xenmmap backmmap -> write_mmap backmmap con s len
 
 (* NB: can throw Reconnect *)
@@ -147,7 +147,7 @@ let input con =
| NoHdr (i, buf)  ->
(* we complete the partial header *)
if sz > 0 then
-   String.blit s 0 buf (Partial.header_size () - i) sz;
+   String.blit s 0 (Bytes.of_string buf) 
(Partial.header_size () - i) sz;
con.partial_in <- if sz = i then
HaveHdr (Partial.of_string buf) else NoHdr (i - sz, buf)
);
diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml
index 0c0d03d0c4..d24abf8a3a 100644
--- a/tools/ocaml/xenstored/logging.ml
+++ b/tools/ocaml/xenstored/logging.ml
@@ -60,11 +60,11 @@ type logger =
 let truncate_line nb_chars line = 
if String.length line > nb_chars - 1 then
let len = max (nb_chars - 1) 2 in
-   let dst_line = String.create len in
-   String.blit line 0 dst_line 0 (len - 2);
-   dst_line.[len-2] <- '.'; 
-   dst_line.[len-1] <- '.';
-   dst_line
+   let dst_line = Bytes.create len in
+   Bytes.blit_string line 0 dst_line 0 (len - 2);
+   Bytes.set dst_line (len-2) '.'; 
+   Bytes.set dst_line (len-1) '.';
+   Bytes.to_string dst_line
else line
 
 let log_rotate ref_ch log_file log_nb_files =
@@ -252,13 +252,13 @@ let string_of_access_type = function
*)
 
 let sanitize_data data =
-   let data = String.copy data in
-   for i = 0 to String.length data - 1
+   let data = Bytes.copy data in
+   for i = 0 to Bytes.length data - 1
do
-   if data.[i] = '\000' then
-   data.[i] <- ' '
+   if Bytes.get data i = '\000' then
+   Bytes.set data i ' '
done;
-   String.escaped data
+   String.escaped (Bytes.to_string data)
 
 let activate_access_log = ref true
 let access_log_destination = ref (File (Paths.xen_log_dir ^ 
"/xenstored-access.log"))
@@ -291,7 +291,7 @@ let access_logging ~con ~tid 

Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant

[Wei Liu]

On Tue, Jan 30, 2018 at 10:55:47PM +, Michael Young wrote:
> Xen built with ocaml 4.06 gives errors such as
> Error: This expression has type bytes but an expression was
>   expected of type string
> as Byte and safe-strings which were introduced in 4.02 are the
> default in 4.06.
> This patch which is partly by Richard W.M. Jones of Red Hat
> from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
> fixes these issues.
> 
> Signed-off-by: Michael Young 
> Reviewed-by: Christian Lindig 

Strangely this doesn't apply to staging. And after examining the
downloaded patch I'm not sure if my mail client is acting up. Do you
have a git branch that I can pull from?

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel