Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant
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
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
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 = - Unix.write back.fd s 0 len +let write_fd ba
Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant
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
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
> 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
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 : Packet.t Queue.t; @@ -76,10 +76,10 @@ type t = { val init_partial_in :
Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant
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
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
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
> 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
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
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
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
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
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 ?(data="") ~level access_type = let date = string_of_date() in let tid = s
Re: [Xen-devel] [PATCH 1/2] make xen ocaml safe-strings compliant
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