[PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD

[PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD

[Wei Liu]

Wei Liu (3):
  paths.m4: provide XENSTORED_{KVA,PORT}
  oxenstored: provide options to define xenstored devices
  hotplug/FreeBSD: configure xenstored

 m4/paths.m4                              | 12 ++++++++++++
 tools/hotplug/FreeBSD/rc.d/xencommons.in |  8 +++++---
 tools/ocaml/xenstored/define.ml          |  3 ---
 tools/ocaml/xenstored/domains.ml         |  7 +++++--
 tools/ocaml/xenstored/oxenstored.conf.in |  3 +++
 tools/ocaml/xenstored/xenstored.ml       |  4 +++-
 6 files changed, 28 insertions(+), 9 deletions(-)

-- 
2.11.0


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

[PATCH for-4.9 v2 1/3] paths.m4: provide XENSTORED_{KVA, PORT}

[Wei Liu]

The default values are Linux device names. No users yet.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Rerun autogen.sh
---
 m4/paths.m4 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/m4/paths.m4 b/m4/paths.m4
index 93ce89ab40..f208b7e39f 100644
--- a/m4/paths.m4
+++ b/m4/paths.m4
@@ -147,3 +147,15 @@ AC_SUBST(XEN_PAGING_DIR)
 XEN_DUMP_DIR=$xen_dumpdir_path
 AC_SUBST(XEN_DUMP_DIR)
 ])
+
+case "$host_os" in
+*freebsd*) XENSTORED_KVA=/dev/xen/xenstored ;;
+*) XENSTORED_KVA=/proc/xen/xsd_kva ;;
+esac
+AC_SUBST(XENSTORED_KVA)
+
+case "$host_os" in
+*freebsd*) XENSTORED_PORT=/dev/xen/xenstored ;;
+*) XENSTORED_PORT=/proc/xen/xsd_port ;;
+esac
+AC_SUBST(XENSTORED_PORT)
-- 
2.11.0


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

[PATCH for-4.9 v2 2/3] oxenstored: provide options to define xenstored devices

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/ocaml/xenstored/define.ml          | 3 ---
 tools/ocaml/xenstored/domains.ml         | 7 +++++--
 tools/ocaml/xenstored/oxenstored.conf.in | 3 +++
 tools/ocaml/xenstored/xenstored.ml       | 4 +++-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/ocaml/xenstored/define.ml b/tools/ocaml/xenstored/define.ml
index 5a604d1bea..2965c08534 100644
--- a/tools/ocaml/xenstored/define.ml
+++ b/tools/ocaml/xenstored/define.ml
@@ -17,9 +17,6 @@
 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 xs_daemon_socket = Paths.xen_run_stored ^ "/socket"
 let xs_daemon_socket_ro = Paths.xen_run_stored ^ "/socket_ro"
 
diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml
index fdae298613..4134630c58 100644
--- a/tools/ocaml/xenstored/domains.ml
+++ b/tools/ocaml/xenstored/domains.ml
@@ -127,11 +127,14 @@ let create xc doms domid mfn port =
 	Domain.bind_interdomain dom;
 	dom
 
+let xenstored_kva = ref ""
+let xenstored_port = ref ""
+
 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 !xenstored_port
+			and fd = Unix.openfile !xenstored_kva
 					       [ Unix.O_RDWR ] 0o600 in
 			let interface = Xenmmap.mmap fd Xenmmap.RDWR Xenmmap.SHARED
 						  (Xenmmap.getpagesize()) 0 in
diff --git a/tools/ocaml/xenstored/oxenstored.conf.in b/tools/ocaml/xenstored/oxenstored.conf.in
index 536611e8d0..6579b84448 100644
--- a/tools/ocaml/xenstored/oxenstored.conf.in
+++ b/tools/ocaml/xenstored/oxenstored.conf.in
@@ -69,3 +69,6 @@ persistent = false
 # Perodically scanning all the rings as a safenet for lazy clients.
 # Define the interval in seconds, set to negative to disable.
 # ring-scan-interval = 20
+
+xenstored-kva = @XENSTORED_KVA@
+xenstored-port = @XENSTORED_PORT@
diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index bb780d0459..33cff5f446 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -118,7 +118,9 @@ let parse_config filename =
 		("access-log-special-ops", Config.Set_bool Logging.access_log_special_ops);
 		("allow-debug", Config.Set_bool Process.allow_debug);
 		("ring-scan-interval", Config.Set_int ring_scan_interval);
-		("pid-file", Config.Set_string pidfile); ] in
+		("pid-file", Config.Set_string pidfile);
+		("xenstored-kva", Config.Set_string Domains.xenstored_kva);
+		("xenstored-port", Config.Set_string Domains.xenstored_port);] in
 	begin try Config.read filename options (fun _ _ -> raise Not_found)
 	with
 	| Config.Error err -> List.iter (fun (k, e) ->
-- 
2.11.0


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

[PATCH for-4.9 v2 3/3] hotplug/FreeBSD: configure xenstored

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/hotplug/FreeBSD/rc.d/xencommons.in | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/hotplug/FreeBSD/rc.d/xencommons.in b/tools/hotplug/FreeBSD/rc.d/xencommons.in
index 2fcd84ab6d..ccd5a9b055 100644
--- a/tools/hotplug/FreeBSD/rc.d/xencommons.in
+++ b/tools/hotplug/FreeBSD/rc.d/xencommons.in
@@ -3,6 +3,8 @@
 # PROVIDE: xencommons
 # REQUIRE: DAEMON
 
+XENSTORED=@XENSTORED@
+
 . /etc/rc.subr
 
 . @XEN_SCRIPT_DIR@/hotplugpath.sh
@@ -37,7 +39,7 @@ xen_startcmd()
 	local time=0
 	local timeout=30
 
-	xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${sbindir}/xenstored)
+	xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${XENSTORED})
 	if test -z "$xenstored_pid"; then
 		printf "Cleaning xenstore database.\n"
 		if [ -z "${XENSTORED_ROOTDIR}" ]; then
@@ -49,7 +51,7 @@ xen_startcmd()
 		if [ -n "${XENSTORED_TRACE}" ]; then
 			XENSTORED_ARGS="${XENSTORED_ARGS} -T @XEN_LOG_DIR@/xenstored-trace.log"
 		fi
-		${sbindir}/xenstored ${XENSTORED_ARGS}
+		${XENSTORED} ${XENSTORED_ARGS}
 		while [ $time -lt $timeout ] && ! `${bindir}/xenstore-read -s / >/dev/null 2>&1` ; do
 			printf "."
 			time=$(($time+1))
@@ -87,7 +89,7 @@ xen_stop()
 
 xen_status()
 {
-	xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${sbindir}/xenstored)
+	xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${XENSTORED})
 	if test -n ${xenstored_pid}; then
 		pids="$pids $xenstored_pid"
 	fi
-- 
2.11.0


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

Re: [PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD

[Christian Lindig]

> On 18. Apr 2017, at 16:31, Wei Liu <wei.liu2@citrix.com> wrote:
> 
> Wei Liu (3):
>  paths.m4: provide XENSTORED_{KVA,PORT}
>  oxenstored: provide options to define xenstored devices
>  hotplug/FreeBSD: configure xenstored
> 
> m4/paths.m4                              | 12 ++++++++++++
> tools/hotplug/FreeBSD/rc.d/xencommons.in |  8 +++++---
> tools/ocaml/xenstored/define.ml          |  3 ---
> tools/ocaml/xenstored/domains.ml         |  7 +++++--
> tools/ocaml/xenstored/oxenstored.conf.in |  3 +++
> tools/ocaml/xenstored/xenstored.ml       |  4 +++-
> 6 files changed, 28 insertions(+), 9 deletions(-)
> 
> -- 
> 2.11.

This approach adds two new entries into oxenstored.conf that are determined by the configure script. I prefer it over the previous design not the least because it results in a much smaller change and doesn’t require new bindings for C libraries. The code looks good and I believe it doesn’t change in significant ways in its failure modes as port and fd are still read from files - just the names of these files are now coming from oxenstored.conf. I’m not sure the paths.m4 is taking the best approach by relying on $host_os rather than testing the paths but I would leave that to people with more autoconf experience to comment on.

---
Rerun autogen.sh
---
m4/paths.m4 | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/m4/paths.m4 b/m4/paths.m4
index 93ce89ab40..f208b7e39f 100644
--- a/m4/paths.m4
+++ b/m4/paths.m4
@@ -147,3 +147,15 @@ AC_SUBST(XEN_PAGING_DIR)
XEN_DUMP_DIR=$xen_dumpdir_path
AC_SUBST(XEN_DUMP_DIR)
])
+
+case "$host_os" in
+*freebsd*) XENSTORED_KVA=/dev/xen/xenstored ;;
+*) XENSTORED_KVA=/proc/xen/xsd_kva ;;
+esac
+AC_SUBST(XENSTORED_KVA)
+
+case "$host_os" in
+*freebsd*) XENSTORED_PORT=/dev/xen/xenstored ;;
+*) XENSTORED_PORT=/proc/xen/xsd_port ;;
+esac
+AC_SUBST(XENSTORED_PORT)
-- 
2.11.0

I’d be fine with taking this patch.

— Christian

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

Re: [PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD

[Ian Jackson]

Christian Lindig writes ("Re: [PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD"):
> This approach adds two new entries into oxenstored.conf that are
> determined by the configure script. I prefer it over the previous
> design not the least because it results in a much smaller change and
> doesn’t require new bindings for C libraries. The code looks good
> and I believe it doesn’t change in significant ways in its failure
> modes as port and fd are still read from files - just the names of
> these files are now coming from oxenstored.conf. I’m not sure the
> paths.m4 is taking the best approach by relying on $host_os rather
> than testing the paths but I would leave that to people with more
> autoconf experience to comment on.

Let me repeat something I said in the corridor: we mustn't test for
the existence of these paths at build-time, because the build host
might not be running Xen.

Personally I would have just tested both paths at runtime, but this
configure-based approach is fine too.

> +case "$host_os" in
> +*freebsd*) XENSTORED_KVA=/dev/xen/xenstored ;;
> +*) XENSTORED_KVA=/proc/xen/xsd_kva ;;
> +esac
> +AC_SUBST(XENSTORED_KVA)
> +
> +case "$host_os" in
> +*freebsd*) XENSTORED_PORT=/dev/xen/xenstored ;;
> +*) XENSTORED_PORT=/proc/xen/xsd_port ;;
> +esac
> +AC_SUBST(XENSTORED_PORT)

I'm not sure why one wouldn't combine these two case statements, but I
don't care enough to suggest reworking it.

All three patches:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks,
Ian.

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

Re: [PATCH for-4.9 v2 1/3] paths.m4: provide XENSTORED_{KVA, PORT}

[Roger Pau Monné]

On Tue, Apr 18, 2017 at 04:31:42PM +0100, Wei Liu wrote:
> The default values are Linux device names. No users yet.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Since it's FreeBSD related stuff:

Acked-by: Roger Pau Monné <roger.pau@citrix.com>


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

Re: [PATCH for-4.9 v2 3/3] hotplug/FreeBSD: configure xenstored

[Roger Pau Monné]

On Tue, Apr 18, 2017 at 04:31:44PM +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

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

Re: [PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD

[Julien Grall]

Hi,

On 20/04/17 11:22, Ian Jackson wrote:
> Christian Lindig writes ("Re: [PATCH for-4.9 v2 0/3] oxenstored: make it work on FreeBSD"):
>> This approach adds two new entries into oxenstored.conf that are
>> determined by the configure script. I prefer it over the previous
>> design not the least because it results in a much smaller change and
>> doesn’t require new bindings for C libraries. The code looks good
>> and I believe it doesn’t change in significant ways in its failure
>> modes as port and fd are still read from files - just the names of
>> these files are now coming from oxenstored.conf. I’m not sure the
>> paths.m4 is taking the best approach by relying on $host_os rather
>> than testing the paths but I would leave that to people with more
>> autoconf experience to comment on.
>
> Let me repeat something I said in the corridor: we mustn't test for
> the existence of these paths at build-time, because the build host
> might not be running Xen.
>
> Personally I would have just tested both paths at runtime, but this
> configure-based approach is fine too.
>
>> +case "$host_os" in
>> +*freebsd*) XENSTORED_KVA=/dev/xen/xenstored ;;
>> +*) XENSTORED_KVA=/proc/xen/xsd_kva ;;
>> +esac
>> +AC_SUBST(XENSTORED_KVA)
>> +
>> +case "$host_os" in
>> +*freebsd*) XENSTORED_PORT=/dev/xen/xenstored ;;
>> +*) XENSTORED_PORT=/proc/xen/xsd_port ;;
>> +esac
>> +AC_SUBST(XENSTORED_PORT)
>
> I'm not sure why one wouldn't combine these two case statements, but I
> don't care enough to suggest reworking it.
>
> All three patches:
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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