* [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD
@ 2017-04-14 10:20 Wei Liu
2017-04-14 10:20 ` [PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension Wei Liu
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Wei Liu @ 2017-04-14 10:20 UTC (permalink / raw)
To: Xen-devel
Cc: Wei Liu, jonathan.ludlam, Ian Jackson, Julien Grall,
christian.lindig, dave, Roger Pau Monné
I managed to remove almost all Linux-ism in my previous work to make all paths
configurable.
The last bits missing are the two device node paths.
Unfortunately there is an easy way to determine system name in the standard
library so I wrote a wrapper for uname syscall.
I think this is a good candidate for inclusion in 4.9. I wouldn't be too
disappointed if they don't end up there though. I will let Julien decide.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: dave@recoil.org
Cc: christian.lindig@citrix.com
Cc: jonathan.ludlam@citrix.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Wei Liu (2):
oxenstored: add an Unix syscall C extension
oxenstored: make it work on FreeBSD
tools/ocaml/xenstored/Makefile | 9 ++++--
tools/ocaml/xenstored/define.ml | 16 ++++++++--
tools/ocaml/xenstored/unix_syscalls.ml | 29 ++++++++++++++++++
tools/ocaml/xenstored/unix_syscalls.mli | 29 ++++++++++++++++++
tools/ocaml/xenstored/unix_syscalls_stubs.c | 46 +++++++++++++++++++++++++++++
5 files changed, 124 insertions(+), 5 deletions(-)
create mode 100644 tools/ocaml/xenstored/unix_syscalls.ml
create mode 100644 tools/ocaml/xenstored/unix_syscalls.mli
create mode 100644 tools/ocaml/xenstored/unix_syscalls_stubs.c
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension 2017-04-14 10:20 [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD Wei Liu @ 2017-04-14 10:20 ` Wei Liu 2017-04-18 10:14 ` Ian Jackson 2017-04-14 10:20 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Wei Liu 2017-04-18 9:46 ` [PATCH for-4.9 0/2] " Christian Lindig 2 siblings, 1 reply; 13+ messages in thread From: Wei Liu @ 2017-04-14 10:20 UTC (permalink / raw) To: Xen-devel Cc: Wei Liu, jonathan.ludlam, Ian Jackson, christian.lindig, dave, Roger Pau Monné Currently there is only uname syscall in it. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: dave@recoil.org Cc: christian.lindig@citrix.com Cc: jonathan.ludlam@citrix.com Cc: Roger Pau Monné <roger.pau@citrix.com> --- tools/ocaml/xenstored/Makefile | 9 ++++-- tools/ocaml/xenstored/unix_syscalls.ml | 29 ++++++++++++++++++ tools/ocaml/xenstored/unix_syscalls.mli | 29 ++++++++++++++++++ tools/ocaml/xenstored/unix_syscalls_stubs.c | 46 +++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 tools/ocaml/xenstored/unix_syscalls.ml create mode 100644 tools/ocaml/xenstored/unix_syscalls.mli create mode 100644 tools/ocaml/xenstored/unix_syscalls_stubs.c diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile index d23883683d..cfd4d81b9e 100644 --- a/tools/ocaml/xenstored/Makefile +++ b/tools/ocaml/xenstored/Makefile @@ -18,12 +18,14 @@ OCAMLINCLUDE += \ -I $(OCAML_TOPLEVEL)/libs/xc \ -I $(OCAML_TOPLEVEL)/libs/eventchn -LIBS = syslog.cma syslog.cmxa select.cma select.cmxa +LIBS = syslog.cma syslog.cmxa select.cma select.cmxa unix_syscalls.cma unix_syscalls.cmxa syslog_OBJS = syslog syslog_C_OBJS = syslog_stubs select_OBJS = select select_C_OBJS = select_stubs -OCAML_LIBRARY = syslog select +unix_syscalls_C_OBJS = unix_syscalls_stubs +unix_syscalls_OBJS = unix_syscalls +OCAML_LIBRARY = syslog select unix_syscalls LIBS += systemd.cma systemd.cmxa systemd_OBJS = systemd @@ -58,13 +60,14 @@ OBJS = paths \ process \ xenstored -INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi select.cmi +INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi select.cmi unix_syscalls.cmi XENSTOREDLIBS = \ unix.cmxa \ -ccopt -L -ccopt . syslog.cmxa \ -ccopt -L -ccopt . systemd.cmxa \ -ccopt -L -ccopt . select.cmxa \ + -ccopt -L -ccopt . unix_syscalls.cmxa \ -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/mmap $(OCAML_TOPLEVEL)/libs/mmap/xenmmap.cmxa \ -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/eventchn $(OCAML_TOPLEVEL)/libs/eventchn/xeneventchn.cmxa \ -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/xc $(OCAML_TOPLEVEL)/libs/xc/xenctrl.cmxa \ diff --git a/tools/ocaml/xenstored/unix_syscalls.ml b/tools/ocaml/xenstored/unix_syscalls.ml new file mode 100644 index 0000000000..b52a07967d --- /dev/null +++ b/tools/ocaml/xenstored/unix_syscalls.ml @@ -0,0 +1,29 @@ +(* + * unix_syscalls.ml + * + * Stubs for unix syscalls + * + * Copyright (C) 2017 Wei Liu <wei.liu2@citrix.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License, version 2.1, as published by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see <http://www.gnu.org/licenses/>. + *) + +type utsname = { + sysname: string; + nodename: string; + release: string; + version: string; + machine: string; +} + +external uname : unit -> utsname = "unix_uname" diff --git a/tools/ocaml/xenstored/unix_syscalls.mli b/tools/ocaml/xenstored/unix_syscalls.mli new file mode 100644 index 0000000000..8e9adaf0a4 --- /dev/null +++ b/tools/ocaml/xenstored/unix_syscalls.mli @@ -0,0 +1,29 @@ +(* + * unix_syscalls.mli + * + * Stubs for unix syscalls + * + * Copyright (C) 2017 Wei Liu <wei.liu2@citrix.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License, version 2.1, as published by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see <http://www.gnu.org/licenses/>. + *) + +type utsname = { + sysname: string; + nodename: string; + release: string; + version: string; + machine: string; +} + +external uname : unit -> utsname = "unix_uname" diff --git a/tools/ocaml/xenstored/unix_syscalls_stubs.c b/tools/ocaml/xenstored/unix_syscalls_stubs.c new file mode 100644 index 0000000000..1cd46d0834 --- /dev/null +++ b/tools/ocaml/xenstored/unix_syscalls_stubs.c @@ -0,0 +1,46 @@ +/* + * unix_syscalls_stub.c + * + * Stubs for unix syscalls + * + * Copyright (C) 2017 Wei Liu <wei.liu2@citrix.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License, version 2.1, as published by the Free Software Foundation. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see <http://www.gnu.org/licenses/>. + */ + +#include <sys/utsname.h> +#include <caml/mlvalues.h> +#include <caml/memory.h> +#include <caml/fail.h> +#include <caml/alloc.h> +#include <caml/signals.h> +#include <caml/unixsupport.h> + +CAMLprim value unix_uname(value ignored) +{ + CAMLparam1(ignored); + CAMLlocal1(utsname); + struct utsname buf; + + if (uname(&buf)) + uerror("uname", Nothing); + + utsname = caml_alloc(5, 0); + Store_field(utsname, 0, caml_copy_string(buf.sysname)); + Store_field(utsname, 1, caml_copy_string(buf.nodename)); + Store_field(utsname, 2, caml_copy_string(buf.release)); + Store_field(utsname, 3, caml_copy_string(buf.version)); + Store_field(utsname, 4, caml_copy_string(buf.machine)); + + CAMLreturn(utsname); +} -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension 2017-04-14 10:20 ` [PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension Wei Liu @ 2017-04-18 10:14 ` Ian Jackson 0 siblings, 0 replies; 13+ messages in thread From: Ian Jackson @ 2017-04-18 10:14 UTC (permalink / raw) To: Wei Liu Cc: Xen-devel, jonathan.ludlam, Roger Pau Monné, christian.lindig, dave Wei Liu writes ("[PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension"): > Currently there is only uname syscall in it. I agree that this would be good to have in 4.9. The ocaml FFI has tricky memory management, so this needs review by an ocaml expert. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD 2017-04-14 10:20 [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD Wei Liu 2017-04-14 10:20 ` [PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension Wei Liu @ 2017-04-14 10:20 ` Wei Liu 2017-04-14 10:32 ` [PATCH for-4.9] oxenstored: remove "_proc" in names Wei Liu 2017-04-18 10:16 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Ian Jackson 2017-04-18 9:46 ` [PATCH for-4.9 0/2] " Christian Lindig 2 siblings, 2 replies; 13+ messages in thread From: Wei Liu @ 2017-04-14 10:20 UTC (permalink / raw) To: Xen-devel Cc: Wei Liu, jonathan.ludlam, Ian Jackson, christian.lindig, dave, Roger Pau Monné Call the uname syscall to determine sysname and return device names accordingly. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: dave@recoil.org Cc: christian.lindig@citrix.com Cc: jonathan.ludlam@citrix.com Cc: Roger Pau Monné <roger.pau@citrix.com> --- tools/ocaml/xenstored/define.ml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tools/ocaml/xenstored/define.ml b/tools/ocaml/xenstored/define.ml index 5a604d1bea..15b03e539f 100644 --- a/tools/ocaml/xenstored/define.ml +++ b/tools/ocaml/xenstored/define.ml @@ -14,11 +14,23 @@ * GNU Lesser General Public License for more details. *) +open Unix_syscalls + let xenstored_major = 1 let xenstored_minor = 0 -let xenstored_proc_kva = "/proc/xen/xsd_kva" -let xenstored_proc_port = "/proc/xen/xsd_port" +let xenstored_proc_kva = + let info = Unix_syscalls.uname () in + match info.sysname with + | "Linux" -> "/proc/xen/xsd_kva" + | "FreeBSD" -> "/dev/xen/xenstored" + | _ -> "nonexistent" +let xenstored_proc_port = + let info = Unix_syscalls.uname () in + match info.sysname with + | "Linux" -> "/proc/xen/xsd_port" + | "FreeBSD" -> "/dev/xen/xenstored" + | _ -> "nonexistent" let xs_daemon_socket = Paths.xen_run_stored ^ "/socket" let xs_daemon_socket_ro = Paths.xen_run_stored ^ "/socket_ro" -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH for-4.9] oxenstored: remove "_proc" in names 2017-04-14 10:20 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Wei Liu @ 2017-04-14 10:32 ` Wei Liu 2017-04-18 10:15 ` Ian Jackson 2017-04-18 10:16 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Ian Jackson 1 sibling, 1 reply; 13+ messages in thread From: Wei Liu @ 2017-04-14 10:32 UTC (permalink / raw) To: Xen-devel Cc: Wei Liu, jonathan.ludlam, Ian Jackson, christian.lindig, dave, Roger Pau Monné They aren't limited to proc file system anymore. Use more generic names. No functional change. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- This is a follow-up patch for "oxenstored: make it work on FreeBSD". Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: dave@recoil.org Cc: christian.lindig@citrix.com Cc: jonathan.ludlam@citrix.com Cc: Roger Pau Monné <roger.pau@citrix.com> --- tools/ocaml/xenstored/define.ml | 4 ++-- tools/ocaml/xenstored/domains.ml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/ocaml/xenstored/define.ml b/tools/ocaml/xenstored/define.ml index 15b03e539f..f3d8cd6ddc 100644 --- a/tools/ocaml/xenstored/define.ml +++ b/tools/ocaml/xenstored/define.ml @@ -19,13 +19,13 @@ open Unix_syscalls let xenstored_major = 1 let xenstored_minor = 0 -let xenstored_proc_kva = +let xenstored_kva = let info = Unix_syscalls.uname () in match info.sysname with | "Linux" -> "/proc/xen/xsd_kva" | "FreeBSD" -> "/dev/xen/xenstored" | _ -> "nonexistent" -let xenstored_proc_port = +let xenstored_port = let info = Unix_syscalls.uname () in match info.sysname with | "Linux" -> "/proc/xen/xsd_port" diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml index fdae298613..572c9fb091 100644 --- a/tools/ocaml/xenstored/domains.ml +++ b/tools/ocaml/xenstored/domains.ml @@ -130,8 +130,8 @@ let create xc doms domid mfn port = let create0 doms = let port, interface = ( - let port = Utils.read_file_single_integer Define.xenstored_proc_port - and fd = Unix.openfile Define.xenstored_proc_kva + let port = Utils.read_file_single_integer Define.xenstored_port + and fd = Unix.openfile Define.xenstored_kva [ Unix.O_RDWR ] 0o600 in let interface = Xenmmap.mmap fd Xenmmap.RDWR Xenmmap.SHARED (Xenmmap.getpagesize()) 0 in -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9] oxenstored: remove "_proc" in names 2017-04-14 10:32 ` [PATCH for-4.9] oxenstored: remove "_proc" in names Wei Liu @ 2017-04-18 10:15 ` Ian Jackson 0 siblings, 0 replies; 13+ messages in thread From: Ian Jackson @ 2017-04-18 10:15 UTC (permalink / raw) To: Wei Liu Cc: Xen-devel, jonathan.ludlam, Roger Pau Monné, christian.lindig, dave Wei Liu writes ("[PATCH for-4.9] oxenstored: remove "_proc" in names"): > They aren't limited to proc file system anymore. Use more generic names. > > No functional change. Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD 2017-04-14 10:20 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Wei Liu 2017-04-14 10:32 ` [PATCH for-4.9] oxenstored: remove "_proc" in names Wei Liu @ 2017-04-18 10:16 ` Ian Jackson 2017-04-18 10:22 ` Christian Lindig 1 sibling, 1 reply; 13+ messages in thread From: Ian Jackson @ 2017-04-18 10:16 UTC (permalink / raw) To: Wei Liu Cc: Xen-devel, jonathan.ludlam, Roger Pau Monné, christian.lindig, dave Wei Liu writes ("[PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD"): > Call the uname syscall to determine sysname and return device names > accordingly. ... > -let xenstored_proc_kva = "/proc/xen/xsd_kva" > -let xenstored_proc_port = "/proc/xen/xsd_port" > +let xenstored_proc_kva = > + let info = Unix_syscalls.uname () in > + match info.sysname with > + | "Linux" -> "/proc/xen/xsd_kva" > + | "FreeBSD" -> "/dev/xen/xenstored" > + | _ -> "nonexistent" This isn't very good. If this code wants to fail, it returns a string "nonexistent" which would then be used to construct pathnames, and actually accessed. In Haskell one could simply leave the default case off, which would generate a runtime failure. Is that possible in ocaml ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD 2017-04-18 10:16 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Ian Jackson @ 2017-04-18 10:22 ` Christian Lindig 0 siblings, 0 replies; 13+ messages in thread From: Christian Lindig @ 2017-04-18 10:22 UTC (permalink / raw) To: Ian Jackson; +Cc: Xen-devel, Jonathan Ludlam, Roger Pau Monne, Wei Liu, dave > On 18. Apr 2017, at 11:16, Ian Jackson <ian.jackson@eu.citrix.com> wrote: > > Wei Liu writes ("[PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD"): >> Call the uname syscall to determine sysname and return device names >> accordingly. > ... >> -let xenstored_proc_kva = "/proc/xen/xsd_kva" >> -let xenstored_proc_port = "/proc/xen/xsd_port" >> +let xenstored_proc_kva = >> + let info = Unix_syscalls.uname () in >> + match info.sysname with >> + | "Linux" -> "/proc/xen/xsd_kva" >> + | "FreeBSD" -> "/dev/xen/xenstored" >> + | _ -> "nonexistent" > > This isn't very good. If this code wants to fail, it returns a string > "nonexistent" which would then be used to construct pathnames, and > actually accessed. > > In Haskell one could simply leave the default case off, which would > generate a runtime failure. Is that possible in ocaml ? In Ocaml you would either use “assert false” or raise a dedicated exception for the catch-all case. You would get a run-time Match_failure exception without but this is nasty. The current solution has the problem you describe. — C _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD 2017-04-14 10:20 [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD Wei Liu 2017-04-14 10:20 ` [PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension Wei Liu 2017-04-14 10:20 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Wei Liu @ 2017-04-18 9:46 ` Christian Lindig 2017-04-18 9:59 ` Wei Liu 2 siblings, 1 reply; 13+ messages in thread From: Christian Lindig @ 2017-04-18 9:46 UTC (permalink / raw) To: Wei Liu Cc: Ian Jackson, Jonathan Ludlam, Julien Grall, dave, Xen-devel, Roger Pau Monne > On 14. Apr 2017, at 11:20, Wei Liu <wei.liu2@citrix.com> wrote: > > Unfortunately there is an easy way to determine system name in the standard > library so I wrote a wrapper for uname syscall. I think there are two solutions to using the correct path: (1) Determine the path at compile time and use the mechanism in config.ml to read the path at startup. https://github.com/mirage/xen/blob/master/tools/ocaml/xenstored/oxenstored.conf.in https://github.com/mirage/xen/blob/master/tools/ocaml/xenstored/xenstored.ml#L90 (2) Determine the path at runtime and this is what the patch does by using uname: -let xenstored_proc_kva = "/proc/xen/xsd_kva" -let xenstored_proc_port = "/proc/xen/xsd_port" +let xenstored_proc_kva = + let info = Unix_syscalls.uname () in + match info.sysname with + | "Linux" -> "/proc/xen/xsd_kva" + | "FreeBSD" -> "/dev/xen/xenstored" + | _ -> "nonexistent" +let xenstored_proc_port = + let info = Unix_syscalls.uname () in + match info.sysname with + | "Linux" -> "/proc/xen/xsd_port" + | "FreeBSD" -> "/dev/xen/xenstored" + | _ -> "nonexistent" I wonder whether this could be simplified without binding a system call in C by simply checking the presence of the two paths (using Sys.is_directory) and picking the one that does exist. In general I’m in favour of (1) but would also consider a simple run-time check. - Christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD 2017-04-18 9:46 ` [PATCH for-4.9 0/2] " Christian Lindig @ 2017-04-18 9:59 ` Wei Liu 2017-04-18 10:11 ` Christian Lindig 2017-04-18 10:11 ` Christian Lindig 0 siblings, 2 replies; 13+ messages in thread From: Wei Liu @ 2017-04-18 9:59 UTC (permalink / raw) To: Christian Lindig Cc: Ian Jackson, Wei Liu, Jonathan Ludlam, Julien Grall, dave, Xen-devel, Roger Pau Monne On Tue, Apr 18, 2017 at 10:46:15AM +0100, Christian Lindig wrote: > > > On 14. Apr 2017, at 11:20, Wei Liu <wei.liu2@citrix.com> wrote: > > > > Unfortunately there is an easy way to determine system name in the standard > > library so I wrote a wrapper for uname syscall. > > I think there are two solutions to using the correct path: > > (1) Determine the path at compile time and use the mechanism in config.ml to read the path at startup. > > https://github.com/mirage/xen/blob/master/tools/ocaml/xenstored/oxenstored.conf.in > https://github.com/mirage/xen/blob/master/tools/ocaml/xenstored/xenstored.ml#L90 > Forgive my ignorance for the Ocaml ecosystem, but I can't seem to find a way to conditionally compile ocaml source code easily. Presumably you mean "Determine the path at *configure* time"? Even if we make those configurable, there should still be two default values in define.ml -- what should they be? All in all I don't think it is nice to require extra configurations when all relevant information is already available during build phase and run time. > (2) Determine the path at runtime and this is what the patch does by using uname: > > -let xenstored_proc_kva = "/proc/xen/xsd_kva" > -let xenstored_proc_port = "/proc/xen/xsd_port" > +let xenstored_proc_kva = > + let info = Unix_syscalls.uname () in > + match info.sysname with > + | "Linux" -> "/proc/xen/xsd_kva" > + | "FreeBSD" -> "/dev/xen/xenstored" > + | _ -> "nonexistent" > +let xenstored_proc_port = > + let info = Unix_syscalls.uname () in > + match info.sysname with > + | "Linux" -> "/proc/xen/xsd_port" > + | "FreeBSD" -> "/dev/xen/xenstored" > + | _ -> "nonexistent" > > I wonder whether this could be simplified without binding a system > call in C by simply checking the presence of the two paths (using > Sys.is_directory) and picking the one that does exist. In general I’m > in favour of (1) but would also consider a simple run-time check. I am fine with checking the paths using Sys.is_directory. Wei. > > - Christian > > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD 2017-04-18 9:59 ` Wei Liu @ 2017-04-18 10:11 ` Christian Lindig 2017-04-18 10:11 ` Christian Lindig 1 sibling, 0 replies; 13+ messages in thread From: Christian Lindig @ 2017-04-18 10:11 UTC (permalink / raw) To: Wei Liu Cc: Ian Jackson, Jonathan Ludlam, Julien Grall, dave, Xen-devel, Roger Pau Monne > On 18. Apr 2017, at 10:59, Wei Liu <wei.liu2@citrix.com> wrote: > > Forgive my ignorance for the Ocaml ecosystem, but I can't seem to find a > way to conditionally compile ocaml source code easily. > > Presumably you mean "Determine the path at *configure* time"? Even if > we make those configurable, there should still be two default values in > define.ml -- what should they be? Sorry for the confusion. I was conflating configure time and compile times. The configure step could provide the correct paths in the config file that is read at startup. I would definitely avoid conditional compilation. — Christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD 2017-04-18 9:59 ` Wei Liu 2017-04-18 10:11 ` Christian Lindig @ 2017-04-18 10:11 ` Christian Lindig 2017-04-18 13:07 ` Wei Liu 1 sibling, 1 reply; 13+ messages in thread From: Christian Lindig @ 2017-04-18 10:11 UTC (permalink / raw) To: Wei Liu Cc: Ian Jackson, Jonathan Ludlam, Julien Grall, dave, Xen-devel, Roger Pau Monne > On 18. Apr 2017, at 10:59, Wei Liu <wei.liu2@citrix.com> wrote: > > Forgive my ignorance for the Ocaml ecosystem, but I can't seem to find a > way to conditionally compile ocaml source code easily. > > Presumably you mean "Determine the path at *configure* time"? Even if > we make those configurable, there should still be two default values in > define.ml -- what should they be? Sorry for the confusion. I was conflating configure time and compile times. The configure step could provide the correct paths in the config file that is read at startup. I would definitely avoid conditional compilation. — Christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD 2017-04-18 10:11 ` Christian Lindig @ 2017-04-18 13:07 ` Wei Liu 0 siblings, 0 replies; 13+ messages in thread From: Wei Liu @ 2017-04-18 13:07 UTC (permalink / raw) To: Christian Lindig Cc: Ian Jackson, Wei Liu, Jonathan Ludlam, Julien Grall, dave, Xen-devel, Roger Pau Monne On Tue, Apr 18, 2017 at 11:11:20AM +0100, Christian Lindig wrote: > > > On 18. Apr 2017, at 10:59, Wei Liu <wei.liu2@citrix.com> wrote: > > > > Forgive my ignorance for the Ocaml ecosystem, but I can't seem to find a > > way to conditionally compile ocaml source code easily. > > > > Presumably you mean "Determine the path at *configure* time"? Even if > > we make those configurable, there should still be two default values in > > define.ml -- what should they be? > > Sorry for the confusion. I was conflating configure time and compile > times. The configure step could provide the correct paths in the > config file that is read at startup. I would definitely avoid > conditional compilation. OK. I'm fine with this. Let me give it a shot. Wei. > > — Christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-04-18 13:07 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-14 10:20 [PATCH for-4.9 0/2] oxenstored: make it work on FreeBSD Wei Liu 2017-04-14 10:20 ` [PATCH for-4.9 1/2] oxenstored: add an Unix syscall C extension Wei Liu 2017-04-18 10:14 ` Ian Jackson 2017-04-14 10:20 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Wei Liu 2017-04-14 10:32 ` [PATCH for-4.9] oxenstored: remove "_proc" in names Wei Liu 2017-04-18 10:15 ` Ian Jackson 2017-04-18 10:16 ` [PATCH for-4.9 2/2] oxenstored: make it work on FreeBSD Ian Jackson 2017-04-18 10:22 ` Christian Lindig 2017-04-18 9:46 ` [PATCH for-4.9 0/2] " Christian Lindig 2017-04-18 9:59 ` Wei Liu 2017-04-18 10:11 ` Christian Lindig 2017-04-18 10:11 ` Christian Lindig 2017-04-18 13:07 ` Wei Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.