[PATCH for-4.6 0/5] Patches for c/oxenstored

[PATCH for-4.6 0/5] Patches for c/oxenstored

[Wei Liu]

Wei Liu (5):
  cxenstored: fix systemd socket activation
  cxenstored: document a bunch of short options in help string
  cxenstored: remove dead option
  oxenstored: fix systemd socket activation
  oxenstored: move sd_notify_ready out of main loop

 tools/ocaml/xenstored/systemd.ml      |  2 +-
 tools/ocaml/xenstored/systemd.mli     |  4 +--
 tools/ocaml/xenstored/systemd_stubs.c |  6 ++--
 tools/ocaml/xenstored/utils.ml        |  2 +-
 tools/ocaml/xenstored/xenstored.ml    |  4 +--
 tools/xenstore/xenstored_core.c       | 58 ++++++++++++++++++++---------------
 6 files changed, 43 insertions(+), 33 deletions(-)

-- 
2.1.4

[PATCH for-4.6 1/5] cxenstored: fix systemd socket activation

[Wei Liu]

There were two problems with original code:

1. sd_booted() was used to determined if the process was started by
   systemd, which was wrong.
2. Exit with error if pidfile was specified, which was too harsh.

These two combined made cxenstored unable to start by hand if it ran
on a system which had systemd.

Fix issues with following changes:

1. Use sd_listen_fds to determine if the process is started by systemd.
2. Don't exit if pidfile is specified.

Rename function and restructure code to make things clearer.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
For 4.6: without this cxenstored is broken when running on a system with
systemd but not started by systemd.
---
 tools/xenstore/xenstored_core.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index b7e4936..57581e0 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1781,7 +1781,10 @@ static int xs_validate_active_socket(const char *connect_to)
 	return xs_get_sd_fd(connect_to);
 }
 
-static void xen_claim_active_sockets(int **psock, int **pro_sock)
+/* Return true if started by systemd and false if not. Exit with
+ * error if things go wrong.
+ */
+static bool systemd_checkin(int **psock, int **pro_sock)
 {
 	int *sock, *ro_sock;
 	const char *soc_str = xs_daemon_socket();
@@ -1789,7 +1792,11 @@ static void xen_claim_active_sockets(int **psock, int **pro_sock)
 	int n;
 
 	n = sd_listen_fds(0);
-	if (n <= 0) {
+
+	if (n == 0)
+		return false;
+
+	 if (n < 0) {
 		sd_notifyf(0, "STATUS=Failed to get any active sockets: %s\n"
 			   "ERRNO=%i",
 			   strerror(errno),
@@ -1816,6 +1823,8 @@ static void xen_claim_active_sockets(int **psock, int **pro_sock)
 
 	talloc_set_destructor(sock, destroy_fd);
 	talloc_set_destructor(ro_sock, destroy_fd);
+
+	return true;
 }
 #endif
 
@@ -1929,6 +1938,9 @@ int main(int argc, char *argv[])
 	bool no_domain_init = false;
 	const char *pidfile = NULL;
 	int timeout;
+#if defined(XEN_SYSTEMD_ENABLED)
+	bool systemd;
+#endif
 
 	while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
 				  NULL)) != -1) {
@@ -1990,10 +2002,11 @@ int main(int argc, char *argv[])
 		barf("%s: No arguments desired", argv[0]);
 
 #if defined(XEN_SYSTEMD_ENABLED)
-	if (sd_booted()) {
+	systemd = systemd_checkin(&sock, &ro_sock);
+	if (systemd) {
 		dofork = false;
 		if (pidfile)
-			barf("%s: PID file not needed on systemd", argv[0]);
+			xprintf("%s: PID file not needed on systemd", argv[0]);
 		pidfile = NULL;
 	}
 #endif
@@ -2020,9 +2033,7 @@ int main(int argc, char *argv[])
 	signal(SIGPIPE, SIG_IGN);
 
 #if defined(XEN_SYSTEMD_ENABLED)
-	if (sd_booted())
-		xen_claim_active_sockets(&sock, &ro_sock);
-	else
+	if (!systemd)
 #endif
 		init_sockets(&sock, &ro_sock);
 
@@ -2057,7 +2068,7 @@ int main(int argc, char *argv[])
 	xenbus_notify_running();
 
 #if defined(XEN_SYSTEMD_ENABLED)
-	if (sd_booted()) {
+	if (systemd) {
 		sd_notify(1, "READY=1");
 		fprintf(stderr, SD_NOTICE "xenstored is ready\n");
 	}
-- 
2.1.4

[PATCH for-4.6 2/5] cxenstored: document a bunch of short options in help string

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
For 4.6: pure doc changes, risk free.
---
 tools/xenstore/xenstored_core.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 57581e0..72f531b 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1886,21 +1886,21 @@ static void usage(void)
 "\n"
 "where options may include:\n"
 "\n"
-"  --no-domain-init    to state that xenstored should not initialise dom0,\n"
-"  --pid-file <file>   giving a file for the daemon's pid to be written,\n"
-"  --help              to output this message,\n"
-"  --no-fork           to request that the daemon does not fork,\n"
-"  --output-pid        to request that the pid of the daemon is output,\n"
-"  --trace-file <file> giving the file for logging, and\n"
-"  --entry-nb <nb>     limit the number of entries per domain,\n"
-"  --entry-size <size> limit the size of entry per domain, and\n"
-"  --watch-nb <nb>     limit the number of watches per domain,\n"
-"  --transaction <nb>  limit the number of transaction allowed per domain,\n"
-"  --no-recovery       to request that no recovery should be attempted when\n"
-"                      the store is corrupted (debug only),\n"
-"  --internal-db       store database in memory, not on disk\n"
-"  --preserve-local    to request that /local is preserved on start-up,\n"
-"  --verbose           to request verbose execution.\n");
+"  -D, --no-domain-init    to state that xenstored should not initialise dom0,\n"
+"  -F, --pid-file <file>   giving a file for the daemon's pid to be written,\n"
+"  -H, --help              to output this message,\n"
+"  -N, --no-fork           to request that the daemon does not fork,\n"
+"  -P, --output-pid        to request that the pid of the daemon is output,\n"
+"  -T, --trace-file <file> giving the file for logging, and\n"
+"  -E, --entry-nb <nb>     limit the number of entries per domain,\n"
+"  -S, --entry-size <size> limit the size of entry per domain, and\n"
+"  -W, --watch-nb <nb>     limit the number of watches per domain,\n"
+"  -t, --transaction <nb>  limit the number of transaction allowed per domain,\n"
+"  -R, --no-recovery       to request that no recovery should be attempted when\n"
+"                          the store is corrupted (debug only),\n"
+"  -I, --internal-db       store database in memory, not on disk\n"
+"  -L, --preserve-local    to request that /local is preserved on start-up,\n"
+"  -V, --verbose           to request verbose execution.\n");
 }
 
 
-- 
2.1.4

[PATCH for-4.6 3/5] cxenstored: remove dead option

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
For 4.6: remove dead code, risk free.
---
 tools/xenstore/xenstored_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 72f531b..100f59e 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1909,7 +1909,6 @@ static struct option options[] = {
 	{ "entry-nb", 1, NULL, 'E' },
 	{ "pid-file", 1, NULL, 'F' },
 	{ "event", 1, NULL, 'e' },
-	{ "master-domid", 1, NULL, 'm' },
 	{ "help", 0, NULL, 'H' },
 	{ "no-fork", 0, NULL, 'N' },
 	{ "priv-domid", 1, NULL, 'p' },
-- 
2.1.4

[PATCH for-4.6 4/5] oxenstored: fix systemd socket activation

[Wei Liu]

Use the correct API sd_listen_fds to determine whether the process is
started by systemd.

Change sd_booted to booted_by_systemd to avoid confusion with systemd's
API.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Dave Scott <dave.scott@eu.citrix.com>

For 4.6: without this oxenstored is broken when running on a system with
systemd but not started by systemd.
---
 tools/ocaml/xenstored/systemd.ml      | 2 +-
 tools/ocaml/xenstored/systemd.mli     | 4 ++--
 tools/ocaml/xenstored/systemd_stubs.c | 6 +++---
 tools/ocaml/xenstored/utils.ml        | 2 +-
 tools/ocaml/xenstored/xenstored.ml    | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/ocaml/xenstored/systemd.ml b/tools/ocaml/xenstored/systemd.ml
index 2aa39ea..6120fd4 100644
--- a/tools/ocaml/xenstored/systemd.ml
+++ b/tools/ocaml/xenstored/systemd.ml
@@ -13,5 +13,5 @@
  *)
 
 external sd_listen_fds: string -> Unix.file_descr = "ocaml_sd_listen_fds"
-external sd_booted: unit -> bool = "ocaml_sd_booted"
+external booted_by_systemd: unit -> bool = "ocaml_booted_by_systemd"
 external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
diff --git a/tools/ocaml/xenstored/systemd.mli b/tools/ocaml/xenstored/systemd.mli
index 85c9f2e..293b5bc 100644
--- a/tools/ocaml/xenstored/systemd.mli
+++ b/tools/ocaml/xenstored/systemd.mli
@@ -17,8 +17,8 @@
  *  us do sanity checks on the expected sockets *)
 val sd_listen_fds: string -> Unix.file_descr
 
-(** Tells us whether or not systemd support was compiled in *)
-val sd_booted: unit -> bool
+(** Tells us whether or not the process is booted by systemd *)
+val booted_by_systemd: unit -> bool
 
 (** Tells systemd we're ready *)
 external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
diff --git a/tools/ocaml/xenstored/systemd_stubs.c b/tools/ocaml/xenstored/systemd_stubs.c
index d924ff1..354539e 100644
--- a/tools/ocaml/xenstored/systemd_stubs.c
+++ b/tools/ocaml/xenstored/systemd_stubs.c
@@ -92,14 +92,14 @@ CAMLprim value ocaml_sd_listen_fds(value connect_to)
 	CAMLreturn(sock_ret);
 }
 
-CAMLprim value ocaml_sd_booted(value ignore)
+CAMLprim value ocaml_booted_by_systemd(value ignore)
 {
 	CAMLparam1(ignore);
 	CAMLlocal1(ret);
 
 	ret = Val_false;
 
-	if (sd_booted())
+	if (sd_listen_fds(0) > 0)
 		ret = Val_true;
 
 	CAMLreturn(ret);
@@ -129,7 +129,7 @@ CAMLprim value ocaml_sd_listen_fds(value connect_to)
 	CAMLreturn(sock_ret);
 }
 
-CAMLprim value ocaml_sd_booted(value ignore)
+CAMLprim value ocaml_booted_by_systemd(value ignore)
 {
 	CAMLparam1(ignore);
 	CAMLlocal1(ret);
diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml
index 61321c6..fad15b3 100644
--- a/tools/ocaml/xenstored/utils.ml
+++ b/tools/ocaml/xenstored/utils.ml
@@ -84,7 +84,7 @@ let create_regular_unix_socket name =
         sock
 
 let create_unix_socket name =
-        if Systemd.sd_booted() then
+        if Systemd.booted_by_systemd() then
                 Systemd.sd_listen_fds name
         else
                 create_regular_unix_socket name
diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index bfe689b..409223d 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -431,7 +431,7 @@ let _ =
 	while not !quit
 	do
 		try
-                        if Systemd.sd_booted() then
+                        if Systemd.booted_by_systemd() then
                                 Systemd.sd_notify_ready ();
 			main_loop ()
 		with exc ->
-- 
2.1.4

[PATCH for-4.6 5/5] oxenstored: move sd_notify_ready out of main loop

[Wei Liu]

Oxenstored only needs to notify systemd its readiness state once. Move
sd_notify_ready out of main loop.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Dave Scott <dave.scott@eu.citrix.com>

For 4.6: avoid wasting CPU cycles, easy to reason its correctness.

There is a small risk that either I wrote the wrong code or I
misunderstand the usage of systemd API. However I've tested the modified
oxenstored it worked fine.
---
 tools/ocaml/xenstored/xenstored.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index 409223d..5e32501 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -428,11 +428,11 @@ let _ =
 		process_domains store cons domains
 		in
 
+	if Systemd.booted_by_systemd () then
+		Systemd.sd_notify_ready ();
 	while not !quit
 	do
 		try
-                        if Systemd.booted_by_systemd() then
-                                Systemd.sd_notify_ready ();
 			main_loop ()
 		with exc ->
 			error "caught exception %s" (Printexc.to_string exc);
-- 
2.1.4

Re: [PATCH for-4.6 3/5] cxenstored: remove dead option

[Ian Campbell]

On Thu, 2015-08-06 at 14:38 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> For 4.6: remove dead code, risk free.

I'm pretty sure this isn't actually dead:

The 'm' option is handled by option parsing, setting the dom0_domid global.

xenbus_master_domid() returns dom0_domid and that is called by both
dom0_init() and, for mini-os, xenbus_map().

So I think this is used by stub-xenstored somehow.

At the least we shouldn't remove the option without also removing all the
above, and that is not 4.6 material at this point IMHO.

Ian.

Re: [PATCH for-4.6 4/5] oxenstored: fix systemd socket activation

[Ian Campbell]

On Thu, 2015-08-06 at 14:38 +0100, Wei Liu wrote:
> Use the correct API sd_listen_fds to determine whether the process is
> started by systemd.
> 
> Change sd_booted to booted_by_systemd to avoid confusion with systemd's
> API.

launched_by_systemd seems like a more conventional name for this.One
doesn't typically "boot" a process.

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Dave Scott <dave.scott@eu.citrix.com>
> 
> For 4.6: without this oxenstored is broken when running on a system with
> systemd but not started by systemd.
> ---
>  tools/ocaml/xenstored/systemd.ml      | 2 +-
>  tools/ocaml/xenstored/systemd.mli     | 4 ++--
>  tools/ocaml/xenstored/systemd_stubs.c | 6 +++---
>  tools/ocaml/xenstored/utils.ml        | 2 +-
>  tools/ocaml/xenstored/xenstored.ml    | 2 +-
>  5 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/ocaml/xenstored/systemd.ml 
> b/tools/ocaml/xenstored/systemd.ml
> index 2aa39ea..6120fd4 100644
> --- a/tools/ocaml/xenstored/systemd.ml
> +++ b/tools/ocaml/xenstored/systemd.ml
> @@ -13,5 +13,5 @@
>   *)
>  
>  external sd_listen_fds: string -> Unix.file_descr = 
> "ocaml_sd_listen_fds"
> -external sd_booted: unit -> bool = "ocaml_sd_booted"
> +external booted_by_systemd: unit -> bool = "ocaml_booted_by_systemd"
>  external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
> diff --git a/tools/ocaml/xenstored/systemd.mli 
> b/tools/ocaml/xenstored/systemd.mli
> index 85c9f2e..293b5bc 100644
> --- a/tools/ocaml/xenstored/systemd.mli
> +++ b/tools/ocaml/xenstored/systemd.mli
> @@ -17,8 +17,8 @@
>   *  us do sanity checks on the expected sockets *)
>  val sd_listen_fds: string -> Unix.file_descr
>  
> -(** Tells us whether or not systemd support was compiled in *)
> -val sd_booted: unit -> bool
> +(** Tells us whether or not the process is booted by systemd *)
> +val booted_by_systemd: unit -> bool
>  
>  (** Tells systemd we're ready *)
>  external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
> diff --git a/tools/ocaml/xenstored/systemd_stubs.c 
> b/tools/ocaml/xenstored/systemd_stubs.c
> index d924ff1..354539e 100644
> --- a/tools/ocaml/xenstored/systemd_stubs.c
> +++ b/tools/ocaml/xenstored/systemd_stubs.c
> @@ -92,14 +92,14 @@ CAMLprim value ocaml_sd_listen_fds(value connect_to)
>  	CAMLreturn(sock_ret);
>  }
>  
> -CAMLprim value ocaml_sd_booted(value ignore)
> +CAMLprim value ocaml_booted_by_systemd(value ignore)
>  {
>  	CAMLparam1(ignore);
>  	CAMLlocal1(ret);
>  
>  	ret = Val_false;
>  
> -	if (sd_booted())
> +	if (sd_listen_fds(0) > 0)
>  		ret = Val_true;
>  
>  	CAMLreturn(ret);
> @@ -129,7 +129,7 @@ CAMLprim value ocaml_sd_listen_fds(value connect_to)
>  	CAMLreturn(sock_ret);
>  }
>  
> -CAMLprim value ocaml_sd_booted(value ignore)
> +CAMLprim value ocaml_booted_by_systemd(value ignore)
>  {
>  	CAMLparam1(ignore);
>  	CAMLlocal1(ret);
> diff --git a/tools/ocaml/xenstored/utils.ml 
> b/tools/ocaml/xenstored/utils.ml
> index 61321c6..fad15b3 100644
> --- a/tools/ocaml/xenstored/utils.ml
> +++ b/tools/ocaml/xenstored/utils.ml
> @@ -84,7 +84,7 @@ let create_regular_unix_socket name =
>          sock
>  
>  let create_unix_socket name =
> -        if Systemd.sd_booted() then
> +        if Systemd.booted_by_systemd() then
>                  Systemd.sd_listen_fds name
>          else
>                  create_regular_unix_socket name
> diff --git a/tools/ocaml/xenstored/xenstored.ml 
> b/tools/ocaml/xenstored/xenstored.ml
> index bfe689b..409223d 100644
> --- a/tools/ocaml/xenstored/xenstored.ml
> +++ b/tools/ocaml/xenstored/xenstored.ml
> @@ -431,7 +431,7 @@ let _ =
>  	while not !quit
>  	do
>  		try
> -                        if Systemd.sd_booted() then
> +                        if Systemd.booted_by_systemd() then
>                                  Systemd.sd_notify_ready ();
>  			main_loop ()
>  		with exc ->

Re: [PATCH for-4.6 3/5] cxenstored: remove dead option

[Wei Liu]

On Thu, Aug 06, 2015 at 02:49:28PM +0100, Ian Campbell wrote:
> On Thu, 2015-08-06 at 14:38 +0100, Wei Liu wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > For 4.6: remove dead code, risk free.
> 
> I'm pretty sure this isn't actually dead:
> 
> The 'm' option is handled by option parsing, setting the dom0_domid global.
> 
> xenbus_master_domid() returns dom0_domid and that is called by both
> dom0_init() and, for mini-os, xenbus_map().
> 
> So I think this is used by stub-xenstored somehow.
> 
> At the least we shouldn't remove the option without also removing all the
> above, and that is not 4.6 material at this point IMHO.
> 

Right. I will drop this patch.

Wei.

> Ian.

Re: [PATCH for-4.6 2/5] cxenstored: document a bunch of short options in help string

[Ian Campbell]

On Thu, 2015-08-06 at 14:38 +0100, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH for-4.6 4/5] oxenstored: fix systemd socket activation

[Dave Scott]

I’m not familiar with the systemd side of things, but the OCaml stub changes look ok:

Acked-by: David Scott <dave.scott@citrix.com>

> On 6 Aug 2015, at 14:38, Wei Liu <wei.liu2@citrix.com> wrote:
> 
> Use the correct API sd_listen_fds to determine whether the process is
> started by systemd.
> 
> Change sd_booted to booted_by_systemd to avoid confusion with systemd's
> API.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Dave Scott <dave.scott@eu.citrix.com>
> 
> For 4.6: without this oxenstored is broken when running on a system with
> systemd but not started by systemd.
> ---
> tools/ocaml/xenstored/systemd.ml      | 2 +-
> tools/ocaml/xenstored/systemd.mli     | 4 ++--
> tools/ocaml/xenstored/systemd_stubs.c | 6 +++---
> tools/ocaml/xenstored/utils.ml        | 2 +-
> tools/ocaml/xenstored/xenstored.ml    | 2 +-
> 5 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/ocaml/xenstored/systemd.ml b/tools/ocaml/xenstored/systemd.ml
> index 2aa39ea..6120fd4 100644
> --- a/tools/ocaml/xenstored/systemd.ml
> +++ b/tools/ocaml/xenstored/systemd.ml
> @@ -13,5 +13,5 @@
>  *)
> 
> external sd_listen_fds: string -> Unix.file_descr = "ocaml_sd_listen_fds"
> -external sd_booted: unit -> bool = "ocaml_sd_booted"
> +external booted_by_systemd: unit -> bool = "ocaml_booted_by_systemd"
> external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
> diff --git a/tools/ocaml/xenstored/systemd.mli b/tools/ocaml/xenstored/systemd.mli
> index 85c9f2e..293b5bc 100644
> --- a/tools/ocaml/xenstored/systemd.mli
> +++ b/tools/ocaml/xenstored/systemd.mli
> @@ -17,8 +17,8 @@
>  *  us do sanity checks on the expected sockets *)
> val sd_listen_fds: string -> Unix.file_descr
> 
> -(** Tells us whether or not systemd support was compiled in *)
> -val sd_booted: unit -> bool
> +(** Tells us whether or not the process is booted by systemd *)
> +val booted_by_systemd: unit -> bool
> 
> (** Tells systemd we're ready *)
> external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
> diff --git a/tools/ocaml/xenstored/systemd_stubs.c b/tools/ocaml/xenstored/systemd_stubs.c
> index d924ff1..354539e 100644
> --- a/tools/ocaml/xenstored/systemd_stubs.c
> +++ b/tools/ocaml/xenstored/systemd_stubs.c
> @@ -92,14 +92,14 @@ CAMLprim value ocaml_sd_listen_fds(value connect_to)
> 	CAMLreturn(sock_ret);
> }
> 
> -CAMLprim value ocaml_sd_booted(value ignore)
> +CAMLprim value ocaml_booted_by_systemd(value ignore)
> {
> 	CAMLparam1(ignore);
> 	CAMLlocal1(ret);
> 
> 	ret = Val_false;
> 
> -	if (sd_booted())
> +	if (sd_listen_fds(0) > 0)
> 		ret = Val_true;
> 
> 	CAMLreturn(ret);
> @@ -129,7 +129,7 @@ CAMLprim value ocaml_sd_listen_fds(value connect_to)
> 	CAMLreturn(sock_ret);
> }
> 
> -CAMLprim value ocaml_sd_booted(value ignore)
> +CAMLprim value ocaml_booted_by_systemd(value ignore)
> {
> 	CAMLparam1(ignore);
> 	CAMLlocal1(ret);
> diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml
> index 61321c6..fad15b3 100644
> --- a/tools/ocaml/xenstored/utils.ml
> +++ b/tools/ocaml/xenstored/utils.ml
> @@ -84,7 +84,7 @@ let create_regular_unix_socket name =
>         sock
> 
> let create_unix_socket name =
> -        if Systemd.sd_booted() then
> +        if Systemd.booted_by_systemd() then
>                 Systemd.sd_listen_fds name
>         else
>                 create_regular_unix_socket name
> diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
> index bfe689b..409223d 100644
> --- a/tools/ocaml/xenstored/xenstored.ml
> +++ b/tools/ocaml/xenstored/xenstored.ml
> @@ -431,7 +431,7 @@ let _ =
> 	while not !quit
> 	do
> 		try
> -                        if Systemd.sd_booted() then
> +                        if Systemd.booted_by_systemd() then
>                                 Systemd.sd_notify_ready ();
> 			main_loop ()
> 		with exc ->
> -- 
> 2.1.4
> 

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

Re: [PATCH for-4.6 5/5] oxenstored: move sd_notify_ready out of main loop

[Dave Scott]

Although I’m not familiar with systemd, this also looks fine:

Acked-by: David Scott <dave.scott@citrix.com>

> On 6 Aug 2015, at 14:38, Wei Liu <wei.liu2@citrix.com> wrote:
> 
> Oxenstored only needs to notify systemd its readiness state once. Move
> sd_notify_ready out of main loop.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Dave Scott <dave.scott@eu.citrix.com>
> 
> For 4.6: avoid wasting CPU cycles, easy to reason its correctness.
> 
> There is a small risk that either I wrote the wrong code or I
> misunderstand the usage of systemd API. However I've tested the modified
> oxenstored it worked fine.
> ---
> tools/ocaml/xenstored/xenstored.ml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
> index 409223d..5e32501 100644
> --- a/tools/ocaml/xenstored/xenstored.ml
> +++ b/tools/ocaml/xenstored/xenstored.ml
> @@ -428,11 +428,11 @@ let _ =
> 		process_domains store cons domains
> 		in
> 
> +	if Systemd.booted_by_systemd () then
> +		Systemd.sd_notify_ready ();
> 	while not !quit
> 	do
> 		try
> -                        if Systemd.booted_by_systemd() then
> -                                Systemd.sd_notify_ready ();
> 			main_loop ()
> 		with exc ->
> 			error "caught exception %s" (Printexc.to_string exc);
> -- 
> 2.1.4
> 

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

Re: [PATCH for-4.6 4/5] oxenstored: fix systemd socket activation

[Ian Campbell]

On Thu, 2015-08-06 at 14:50 +0100, Ian Campbell wrote:
> On Thu, 2015-08-06 at 14:38 +0100, Wei Liu wrote:
> > Use the correct API sd_listen_fds to determine whether the process is
> > started by systemd.
> > 
> > Change sd_booted to booted_by_systemd to avoid confusion with systemd's
> > API.
> 
> launched_by_systemd seems like a more conventional name for this.One
> doesn't typically "boot" a process.

Apart from the name the code itself looks correct to me though, so with a
name change to either launch_by_systemd or maybe have_sd_sockets or
something like that then:
    Acked-by: Ian Campbell <ian.campbell@citrix.com>