* [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
* [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 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
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 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 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
* 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-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.