[PATCH v4 0/4] tools: make xenstore domain/daemon configurable

[PATCH v4 0/4] tools: make xenstore domain/daemon configurable

[Juergen Gross]

Add a configuration option to /etc/sysconfig/xencommons to let the
user configure whether he wants to start xenstore service as a daemon
or as a stubdom.

Changes in V4:
- patch 1: remove sd_booted() test, undo unintended white space changes
- patch 3: use @XEN_RUN_DIR@ as requested by Wei Liu

Changes in V3:
- patch 1: re-add sd_notify() call
- split up former patch 2 into 3 patches as requested by Ian Jackson
- patch 4 (was 2): remove check for running xenstore domain, as this
  is done in init-xenstore-domain already
- patch 4 (was 2): if booted with systemd send a systemd-notify message
  in the xenstore domain case
- patch 4 (was 2): if booted with systemd don't wait until xenstore
  daemon is running, as the daemon will have sent a notify message by
  its own

Changes in V2:
- move service type modification form patch 2 to patch 1 as implied by
  Ross Lagerwall (at least I guess so)
- add .gitignore entry for launch-xenstore

Juergen Gross (4):
  tools: remove systemd xenstore socket definitions
  tools: split out xenstored starting form xencommons
  tools: use pidfile for test if xenstored is running
  tools: make xenstore domain easy configurable

 .gitignore                                         |   1 +
 tools/configure                                    |   7 +-
 tools/configure.ac                                 |   3 +-
 tools/hotplug/Linux/Makefile                       |   1 +
 tools/hotplug/Linux/init.d/sysconfig.xencommons.in |  42 ++++++++-
 tools/hotplug/Linux/init.d/xencommons.in           |  38 +-------
 tools/hotplug/Linux/launch-xenstore.in             |  87 +++++++++++++++++
 tools/hotplug/Linux/systemd/Makefile               |   5 -
 tools/hotplug/Linux/systemd/xenstored.service.in   |  13 +--
 tools/hotplug/Linux/systemd/xenstored.socket.in    |  13 ---
 tools/hotplug/Linux/systemd/xenstored_ro.socket.in |  13 ---
 tools/ocaml/xenstored/systemd.ml                   |   2 -
 tools/ocaml/xenstored/systemd.mli                  |   8 --
 tools/ocaml/xenstored/systemd_stubs.c              |  98 --------------------
 tools/ocaml/xenstored/utils.ml                     |   9 +-
 tools/ocaml/xenstored/xenstored.ml                 |   3 +-
 tools/xenstore/xenstored_core.c                    | 103 +--------------------
 17 files changed, 146 insertions(+), 300 deletions(-)
 create mode 100644 tools/hotplug/Linux/launch-xenstore.in
 delete mode 100644 tools/hotplug/Linux/systemd/xenstored.socket.in
 delete mode 100644 tools/hotplug/Linux/systemd/xenstored_ro.socket.in

-- 
2.6.6


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

[PATCH v4 1/4] tools: remove systemd xenstore socket definitions

[Juergen Gross]

On a system with systemd the xenstore sockets are created via systemd.
Remove the related configuration files in order to be able to decide
at runtime whether the sockets should be created or not. This will
enable Xen to start xenstore either via a daemon or via a stub domain.

As the xenstore domain start program will exit after it has done its
job prepare the same behaviour to be tolerated by systemd for the
xenstore daemon by specifying the appropriate flags in the service
file.

A rerun of autogen.sh is required.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4: remove sd_booted() test, undo unintended white space changes
V3: re-add sd_notify() call
---
 tools/configure                                    |   4 +-
 tools/configure.ac                                 |   2 -
 tools/hotplug/Linux/systemd/Makefile               |   5 -
 tools/hotplug/Linux/systemd/xenstored.service.in   |   5 +-
 tools/hotplug/Linux/systemd/xenstored.socket.in    |  13 ---
 tools/hotplug/Linux/systemd/xenstored_ro.socket.in |  13 ---
 tools/ocaml/xenstored/systemd.ml                   |   2 -
 tools/ocaml/xenstored/systemd.mli                  |   8 --
 tools/ocaml/xenstored/systemd_stubs.c              |  98 --------------------
 tools/ocaml/xenstored/utils.ml                     |   9 +-
 tools/ocaml/xenstored/xenstored.ml                 |   3 +-
 tools/xenstore/xenstored_core.c                    | 103 +--------------------
 12 files changed, 10 insertions(+), 255 deletions(-)
 delete mode 100644 tools/hotplug/Linux/systemd/xenstored.socket.in
 delete mode 100644 tools/hotplug/Linux/systemd/xenstored_ro.socket.in

diff --git a/tools/configure b/tools/configure
index 5b5dcce..1c84c6c 100755
--- a/tools/configure
+++ b/tools/configure
@@ -9670,7 +9670,7 @@ fi
 
 if test "x$systemd" = "xy"; then :
 
-    ac_config_files="$ac_config_files hotplug/Linux/systemd/proc-xen.mount hotplug/Linux/systemd/var-lib-xenstored.mount hotplug/Linux/systemd/xen-init-dom0.service hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service hotplug/Linux/systemd/xen-watchdog.service hotplug/Linux/systemd/xenconsoled.service hotplug/Linux/systemd/xendomains.service hotplug/Linux/systemd/xendriverdomain.service hotplug/Linux/systemd/xenstored.service hotplug/Linux/systemd/xenstored.socket hotplug/Linux/systemd/xenstored_ro.socket"
+    ac_config_files="$ac_config_files hotplug/Linux/systemd/proc-xen.mount hotplug/Linux/systemd/var-lib-xenstored.mount hotplug/Linux/systemd/xen-init-dom0.service hotplug/Linux/systemd/xen-qemu-dom0-disk-backend.service hotplug/Linux/systemd/xen-watchdog.service hotplug/Linux/systemd/xenconsoled.service hotplug/Linux/systemd/xendomains.service hotplug/Linux/systemd/xendriverdomain.service hotplug/Linux/systemd/xenstored.service"
 
 
 fi
@@ -10394,8 +10394,6 @@ do
     "hotplug/Linux/systemd/xendomains.service") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xendomains.service" ;;
     "hotplug/Linux/systemd/xendriverdomain.service") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xendriverdomain.service" ;;
     "hotplug/Linux/systemd/xenstored.service") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xenstored.service" ;;
-    "hotplug/Linux/systemd/xenstored.socket") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xenstored.socket" ;;
-    "hotplug/Linux/systemd/xenstored_ro.socket") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/systemd/xenstored_ro.socket" ;;
 
   *) as_fn_error $? "invalid argument: \`$ac_config_target'" "$LINENO" 5;;
   esac
diff --git a/tools/configure.ac b/tools/configure.ac
index 87e14a8..f991ab3 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -438,8 +438,6 @@ AS_IF([test "x$systemd" = "xy"], [
     hotplug/Linux/systemd/xendomains.service
     hotplug/Linux/systemd/xendriverdomain.service
     hotplug/Linux/systemd/xenstored.service
-    hotplug/Linux/systemd/xenstored.socket
-    hotplug/Linux/systemd/xenstored_ro.socket
     ])
 ])
 
diff --git a/tools/hotplug/Linux/systemd/Makefile b/tools/hotplug/Linux/systemd/Makefile
index 558e459..7d24bbe 100644
--- a/tools/hotplug/Linux/systemd/Makefile
+++ b/tools/hotplug/Linux/systemd/Makefile
@@ -6,9 +6,6 @@ XEN_SYSTEMD_MODULES = xen.conf
 XEN_SYSTEMD_MOUNT =  proc-xen.mount
 XEN_SYSTEMD_MOUNT += var-lib-xenstored.mount
 
-XEN_SYSTEMD_SOCKET  = xenstored.socket
-XEN_SYSTEMD_SOCKET += xenstored_ro.socket
-
 XEN_SYSTEMD_SERVICE  = xenstored.service
 XEN_SYSTEMD_SERVICE += xenconsoled.service
 XEN_SYSTEMD_SERVICE += xen-qemu-dom0-disk-backend.service
@@ -19,7 +16,6 @@ XEN_SYSTEMD_SERVICE += xendriverdomain.service
 
 ALL_XEN_SYSTEMD =	$(XEN_SYSTEMD_MODULES)  \
 			$(XEN_SYSTEMD_MOUNT)	\
-			$(XEN_SYSTEMD_SOCKET)	\
 			$(XEN_SYSTEMD_SERVICE)
 
 .PHONY: all
@@ -38,7 +34,6 @@ install: $(ALL_XEN_SYSTEMD)
 		$(INSTALL_DIR) $(DESTDIR)$(XEN_SYSTEMD_DIR)
 	[ -d $(DESTDIR)$(XEN_SYSTEMD_MODULES_LOAD) ] || \
 		$(INSTALL_DIR) $(DESTDIR)$(XEN_SYSTEMD_MODULES_LOAD)
-	$(INSTALL_DATA) *.socket $(DESTDIR)$(XEN_SYSTEMD_DIR)
 	$(INSTALL_DATA) *.service $(DESTDIR)$(XEN_SYSTEMD_DIR)
 	$(INSTALL_DATA) *.mount $(DESTDIR)$(XEN_SYSTEMD_DIR)
 	$(INSTALL_DATA) *.conf $(DESTDIR)$(XEN_SYSTEMD_MODULES_LOAD)
diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in b/tools/hotplug/Linux/systemd/xenstored.service.in
index a5f836b..d520d70 100644
--- a/tools/hotplug/Linux/systemd/xenstored.service.in
+++ b/tools/hotplug/Linux/systemd/xenstored.service.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=The Xen xenstore
-Requires=xenstored_ro.socket xenstored.socket proc-xen.mount var-lib-xenstored.mount
+Requires=proc-xen.mount var-lib-xenstored.mount
 After=proc-xen.mount var-lib-xenstored.mount
 Before=libvirtd.service libvirt-guests.service
 RefuseManualStop=true
@@ -8,6 +8,8 @@ ConditionPathExists=/proc/xen/capabilities
 
 [Service]
 Type=notify
+NotifyAccess=all
+RemainAfterExit=true
 KillMode=none
 Environment=XENSTORED_ARGS=
 Environment=XENSTORED=@XENSTORED@
@@ -19,6 +21,5 @@ ExecStart=/bin/sh -c "exec $XENSTORED --no-fork $XENSTORED_ARGS"
 
 [Install]
 WantedBy=multi-user.target
-Also=xenstored_ro.socket xenstored.socket
 Also=proc-xen.mount
 Also=var-lib-xenstored.mount
diff --git a/tools/hotplug/Linux/systemd/xenstored.socket.in b/tools/hotplug/Linux/systemd/xenstored.socket.in
deleted file mode 100644
index 375c4b7..0000000
--- a/tools/hotplug/Linux/systemd/xenstored.socket.in
+++ /dev/null
@@ -1,13 +0,0 @@
-[Unit]
-Description=xenstore socket
-Requires=proc-xen.mount var-lib-xenstored.mount
-After=proc-xen.mount var-lib-xenstored.mount
-ConditionPathExists=/proc/xen/capabilities
-
-[Socket]
-ListenStream=@XEN_RUN_STORED@/socket
-SocketMode=0600
-Service=xenstored.service
-
-[Install]
-WantedBy=sockets.target
diff --git a/tools/hotplug/Linux/systemd/xenstored_ro.socket.in b/tools/hotplug/Linux/systemd/xenstored_ro.socket.in
deleted file mode 100644
index 82fe377..0000000
--- a/tools/hotplug/Linux/systemd/xenstored_ro.socket.in
+++ /dev/null
@@ -1,13 +0,0 @@
-[Unit]
-Description=xenstore ro socket
-Requires=proc-xen.mount var-lib-xenstored.mount
-After=proc-xen.mount var-lib-xenstored.mount
-ConditionPathExists=/proc/xen/capabilities
-
-[Socket]
-ListenStream=@XEN_RUN_STORED@/socket_ro
-SocketMode=0660
-Service=xenstored.service
-
-[Install]
-WantedBy=sockets.target
diff --git a/tools/ocaml/xenstored/systemd.ml b/tools/ocaml/xenstored/systemd.ml
index 732446d..39127f7 100644
--- a/tools/ocaml/xenstored/systemd.ml
+++ b/tools/ocaml/xenstored/systemd.ml
@@ -12,6 +12,4 @@
  * GNU Lesser General Public License for more details.
  *)
 
-external sd_listen_fds: string -> Unix.file_descr = "ocaml_sd_listen_fds"
-external launched_by_systemd: unit -> bool = "ocaml_launched_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 538fc5e..18b9331 100644
--- a/tools/ocaml/xenstored/systemd.mli
+++ b/tools/ocaml/xenstored/systemd.mli
@@ -12,13 +12,5 @@
  * GNU Lesser General Public License for more details.
  *)
 
-(** Calls the C library sd_listen_fds() function for us. Although
- *  the library doesn't accept argument we send one over to help
- *  us do sanity checks on the expected sockets *)
-val sd_listen_fds: string -> Unix.file_descr
-
-(** Tells us whether the process is launched by systemd *)
-val launched_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 322f1e0..490156c 100644
--- a/tools/ocaml/xenstored/systemd_stubs.c
+++ b/tools/ocaml/xenstored/systemd_stubs.c
@@ -25,88 +25,10 @@
 
 #if defined(HAVE_SYSTEMD)
 
-#include <sys/socket.h>
 #include <systemd/sd-daemon.h>
 
 #include "_paths.h"
 
-/* Will work regardless of the order systemd gives them to us */
-static int oxen_get_sd_fd(const char *connect_to)
-{
-	int fd = SD_LISTEN_FDS_START;
-	int r;
-
-	while (fd <= SD_LISTEN_FDS_START + 1) {
-		r = sd_is_socket_unix(fd, SOCK_STREAM, 1, connect_to, 0);
-		if (r > 0)
-			return fd;
-		fd++;
-	}
-
-	return -EBADR;
-}
-
-static int oxen_verify_socket_socket(const char *connect_to)
-{
-	if ((strcmp(XEN_RUN_STORED "/socket_ro", connect_to) != 0) &&
-	    (strcmp(XEN_RUN_STORED "/socket", connect_to) != 0)) {
-		sd_notifyf(0, "STATUS=unexpected socket: %s\n"
-			   "ERRNO=%i",
-			   connect_to,
-			   EBADR);
-		return -EBADR;
-	}
-
-	return oxen_get_sd_fd(connect_to);
-}
-
-CAMLprim value ocaml_sd_listen_fds(value connect_to)
-{
-	CAMLparam1(connect_to);
-	CAMLlocal1(sock_ret);
-	int sock = -EBADR, n;
-
-	n = sd_listen_fds(0);
-	if (n <= 0) {
-		sd_notifyf(0, "STATUS=Failed to get any active sockets: %s\n"
-			   "ERRNO=%i",
-			   strerror(errno),
-			   errno);
-		caml_failwith("ocaml_sd_listen_fds() failed to get any sockets");
-	} else if (n != 2) {
-		fprintf(stderr, SD_ERR "Expected 2 fds but given %d\n", n);
-		sd_notifyf(0, "STATUS=Mismatch on number (2): %s\n"
-			   "ERRNO=%d",
-			   strerror(EBADR),
-			   EBADR);
-		caml_failwith("ocaml_sd_listen_fds() mismatch");
-	}
-
-	sock = oxen_verify_socket_socket(String_val(connect_to));
-	if (sock <= 0) {
-		fprintf(stderr, "failed to verify sock %s\n",
-			String_val(connect_to));
-		caml_failwith("ocaml_sd_listen_fds_init() invalid socket");
-	}
-
-	sock_ret = Val_int(sock);
-
-	CAMLreturn(sock_ret);
-}
-
-CAMLprim value ocaml_launched_by_systemd(value ignore)
-{
-	CAMLparam1(ignore);
-	CAMLlocal1(ret);
-
-	ret = Val_false;
-
-	if (sd_listen_fds(0) > 0)
-		ret = Val_true;
-
-	CAMLreturn(ret);
-}
-
 CAMLprim value ocaml_sd_notify_ready(value ignore)
 {
 	CAMLparam1(ignore);
@@ -121,26 +43,6 @@ CAMLprim value ocaml_sd_notify_ready(value ignore)
 
 #else
 
-CAMLprim value ocaml_sd_listen_fds(value connect_to)
-{
-	CAMLparam1(connect_to);
-	CAMLlocal1(sock_ret);
-
-	sock_ret = Val_int(-1U);
-
-	CAMLreturn(sock_ret);
-}
-
-CAMLprim value ocaml_launched_by_systemd(value ignore)
-{
-	CAMLparam1(ignore);
-	CAMLlocal1(ret);
-
-	ret = Val_false;
-
-	CAMLreturn(ret);
-}
-
 CAMLprim value ocaml_sd_notify_ready(value ignore)
 {
 	CAMLparam1(ignore);
diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml
index 9f82c1c..e89c1af 100644
--- a/tools/ocaml/xenstored/utils.ml
+++ b/tools/ocaml/xenstored/utils.ml
@@ -73,22 +73,15 @@ let trim_path path =
 let join_by_null ls = String.concat "\000" ls
 
 (* unix utils *)
-let create_regular_unix_socket name =
+let create_unix_socket name =
         Unixext.unlink_safe name;
         Unixext.mkdir_rec (Filename.dirname name) 0o700;
         let sockaddr = Unix.ADDR_UNIX(name) in
         let sock = Unix.socket Unix.PF_UNIX Unix.SOCK_STREAM 0 in
-        Unix.set_close_on_exec sock;
         Unix.bind sock sockaddr;
         Unix.listen sock 1;
         sock
 
-let create_unix_socket name =
-        if Systemd.launched_by_systemd() then
-                Systemd.sd_listen_fds name
-        else
-                create_regular_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
diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index 7ea4026..2efcce6 100644
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -428,8 +428,7 @@ let _ =
 		process_domains store cons domains
 		in
 
-	if Systemd.launched_by_systemd () then
-		Systemd.sd_notify_ready ();
+	Systemd.sd_notify_ready ();
 	while not !quit
 	do
 		try
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 693d47d..3df977b 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1788,84 +1788,6 @@ static int destroy_fd(void *_fd)
 	return 0;
 }
 
-#if defined(XEN_SYSTEMD_ENABLED)
-/* Will work regardless of the order systemd gives them to us */
-static int xs_get_sd_fd(const char *connect_to)
-{
-	int fd = SD_LISTEN_FDS_START;
-	int r;
-
-	while (fd <= SD_LISTEN_FDS_START + 1) {
-		r = sd_is_socket_unix(fd, SOCK_STREAM, 1, connect_to, 0);
-		if (r > 0)
-			return fd;
-		fd++;
-	}
-
-	return -EBADR;
-}
-
-static int xs_validate_active_socket(const char *connect_to)
-{
-	if ((strcmp(xs_daemon_socket_ro(), connect_to) != 0) &&
-	    (strcmp(xs_daemon_socket(), connect_to) != 0)) {
-		sd_notifyf(0, "STATUS=unexpected socket: %s\n"
-			   "ERRNO=%i",
-			   connect_to,
-			   EBADR);
-		return -EBADR;
-	}
-
-	return xs_get_sd_fd(connect_to);
-}
-
-/* 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();
-	const char *soc_str_ro = xs_daemon_socket_ro();
-	int n;
-
-	n = sd_listen_fds(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),
-			   errno);
-		barf_perror("sd_listen_fds() failed\n");
-	} else if (n != 2) {
-		fprintf(stderr, SD_ERR "Expected 2 fds but given %d\n", n);
-		sd_notifyf(0, "STATUS=Mismatch on number (2): %s\n"
-			   "ERRNO=%d",
-			   strerror(EBADR),
-			   EBADR);
-		barf_perror("sd_listen_fds() gave too many fds\n");
-	}
-
-	*psock = sock = talloc(talloc_autofree_context(), int);
-	*sock = xs_validate_active_socket(soc_str);
-	if (*sock <= 0)
-		barf_perror("%s", soc_str);
-
-	*pro_sock = ro_sock = talloc(talloc_autofree_context(), int);
-	*ro_sock = xs_validate_active_socket(soc_str_ro);
-	if (*ro_sock <= 0)
-		barf_perror("%s", soc_str_ro);
-
-	talloc_set_destructor(sock, destroy_fd);
-	talloc_set_destructor(ro_sock, destroy_fd);
-
-	return true;
-}
-#endif
-
 static void init_sockets(int **psock, int **pro_sock)
 {
 	struct sockaddr_un addr;
@@ -1984,9 +1906,7 @@ int main(int argc, char *argv[])
 	const char *pidfile = NULL;
 	const char *memfile = NULL;
 	int timeout;
-#if defined(XEN_SYSTEMD_ENABLED)
-	bool systemd;
-#endif
+
 
 	while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:M:", options,
 				  NULL)) != -1) {
@@ -2050,16 +1970,6 @@ int main(int argc, char *argv[])
 	if (optind != argc)
 		barf("%s: No arguments desired", argv[0]);
 
-#if defined(XEN_SYSTEMD_ENABLED)
-	systemd = systemd_checkin(&sock, &ro_sock);
-	if (systemd) {
-		dofork = false;
-		if (pidfile)
-			xprintf("%s: PID file not needed on systemd", argv[0]);
-		pidfile = NULL;
-	}
-#endif
-
 	reopen_log();
 
 	/* make sure xenstored directories exist */
@@ -2086,10 +1996,7 @@ int main(int argc, char *argv[])
 		signal(SIGUSR1, do_talloc_report);
 	}
 
-#if defined(XEN_SYSTEMD_ENABLED)
-	if (!systemd)
-#endif
-		init_sockets(&sock, &ro_sock);
+	init_sockets(&sock, &ro_sock);
 
 	init_pipe(reopen_log_pipe);
 
@@ -2122,10 +2029,8 @@ int main(int argc, char *argv[])
 	xenbus_notify_running();
 
 #if defined(XEN_SYSTEMD_ENABLED)
-	if (systemd) {
-		sd_notify(1, "READY=1");
-		fprintf(stderr, SD_NOTICE "xenstored is ready\n");
-	}
+	sd_notify(1, "READY=1");
+	fprintf(stderr, SD_NOTICE "xenstored is ready\n");
 #endif
 
 	/* Main loop. */
-- 
2.6.6


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

[PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Juergen Gross]

In order to prepare starting a xenstore domain split out the starting
of the xenstore daemon from the xencommons script into a dedicated
launch-xenstore script.

Correct one error: don't remove old tdb files in background, as this
could lead to very subtle races.

A rerun of autogen.sh is required.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 .gitignore                               |  1 +
 tools/configure                          |  3 +-
 tools/configure.ac                       |  1 +
 tools/hotplug/Linux/Makefile             |  1 +
 tools/hotplug/Linux/init.d/xencommons.in | 36 ++-------------------
 tools/hotplug/Linux/launch-xenstore.in   | 55 ++++++++++++++++++++++++++++++++
 6 files changed, 63 insertions(+), 34 deletions(-)
 create mode 100644 tools/hotplug/Linux/launch-xenstore.in

diff --git a/.gitignore b/.gitignore
index 9b8dece..d193820 100644
--- a/.gitignore
+++ b/.gitignore
@@ -157,6 +157,7 @@ tools/hotplug/Linux/init.d/xen-watchdog
 tools/hotplug/Linux/init.d/xencommons
 tools/hotplug/Linux/init.d/xendomains
 tools/hotplug/Linux/init.d/xendriverdomain
+tools/hotplug/Linux/launch-xenstore
 tools/hotplug/Linux/systemd/*.conf
 tools/hotplug/Linux/systemd/*.mount
 tools/hotplug/Linux/systemd/*.socket
diff --git a/tools/configure b/tools/configure
index 1c84c6c..51f16c5 100755
--- a/tools/configure
+++ b/tools/configure
@@ -2410,7 +2410,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
-ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/FreeBSD/rc.d/xendriverdomain hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/sysconfig.xendomains hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/init.d/xendriverdomain hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons hotplug/NetBSD/rc.d/xendriverdomain libxl/xenlight.pc.in libxl/xlutil.pc.in ocaml/xenstored/oxenstored.conf"
+ac_config_files="$ac_config_files ../config/Tools.mk hotplug/FreeBSD/rc.d/xencommons hotplug/FreeBSD/rc.d/xendriverdomain hotplug/Linux/init.d/sysconfig.xencommons hotplug/Linux/init.d/sysconfig.xendomains hotplug/Linux/init.d/xen-watchdog hotplug/Linux/init.d/xencommons hotplug/Linux/init.d/xendomains hotplug/Linux/init.d/xendriverdomain hotplug/Linux/launch-xenstore hotplug/Linux/vif-setup hotplug/Linux/xen-hotplug-common.sh hotplug/Linux/xendomains hotplug/NetBSD/rc.d/xencommons hotplug/NetBSD/rc.d/xendriverdomain libxl/xenlight.pc.in libxl/xlutil.pc.in ocaml/xenstored/oxenstored.conf"
 
 ac_config_headers="$ac_config_headers config.h"
 
@@ -10376,6 +10376,7 @@ do
     "hotplug/Linux/init.d/xencommons") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xencommons" ;;
     "hotplug/Linux/init.d/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendomains" ;;
     "hotplug/Linux/init.d/xendriverdomain") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/init.d/xendriverdomain" ;;
+    "hotplug/Linux/launch-xenstore") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/launch-xenstore" ;;
     "hotplug/Linux/vif-setup") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/vif-setup" ;;
     "hotplug/Linux/xen-hotplug-common.sh") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xen-hotplug-common.sh" ;;
     "hotplug/Linux/xendomains") CONFIG_FILES="$CONFIG_FILES hotplug/Linux/xendomains" ;;
diff --git a/tools/configure.ac b/tools/configure.ac
index f991ab3..3a4abb5 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -15,6 +15,7 @@ hotplug/Linux/init.d/xen-watchdog
 hotplug/Linux/init.d/xencommons
 hotplug/Linux/init.d/xendomains
 hotplug/Linux/init.d/xendriverdomain
+hotplug/Linux/launch-xenstore
 hotplug/Linux/vif-setup
 hotplug/Linux/xen-hotplug-common.sh
 hotplug/Linux/xendomains
diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 6d6ccee..29280cb 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -30,6 +30,7 @@ XEN_SCRIPTS += block-drbd-probe
 XEN_SCRIPTS += block-dummy
 XEN_SCRIPTS += $(XEN_SCRIPTS-y)
 XEN_SCRIPTS += colo-proxy-setup
+XEN_SCRIPTS += launch-xenstore
 
 SUBDIRS-$(CONFIG_SYSTEMD) += systemd
 
diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in
index 2d8f30b..a32608c 100644
--- a/tools/hotplug/Linux/init.d/xencommons.in
+++ b/tools/hotplug/Linux/init.d/xencommons.in
@@ -18,7 +18,6 @@
 # Description:       Starts and stops the daemons neeeded for xl/xend
 ### END INIT INFO
 
-XENSTORED=@XENSTORED@
 BACKEND_MODULES="@LINUX_BACKEND_MODULES@"
 
 . @XEN_SCRIPT_DIR@/hotplugpath.sh
@@ -53,8 +52,6 @@ if test -f /proc/xen/capabilities && \
 fi
 
 do_start () {
-        local time=0
-	local timeout=30
 	local mod
 
 	for mod in $BACKEND_MODULES ; do modprobe "$mod" &>/dev/null ; done
@@ -62,37 +59,10 @@ do_start () {
 	mkdir -p ${XEN_RUN_DIR}
 	mkdir -p ${XEN_LOCK_DIR}
 
-	if ! `${bindir}/xenstore-read -s / >/dev/null 2>&1`
-	then
-		test -z "$XENSTORED_ROOTDIR" && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
-		rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null
-		test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T @XEN_LOG_DIR@/xenstored-trace.log"
+	@XEN_SCRIPT_DIR@/launch-xenstore || exit 1
 
-		if [ -n "$XENSTORED" ] ; then
-		    echo -n Starting $XENSTORED...
-		    $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
-		else
-		    echo "No xenstored found"
-		    exit 1
-		fi
-
-		# Wait for xenstored to actually come up, timing out after 30 seconds
-                while [ $time -lt $timeout ] && ! `${bindir}/xenstore-read -s / >/dev/null 2>&1` ; do
-                    echo -n .
-		    time=$(($time+1))
-                    sleep 1
-                done
-		echo
-
-		# Exit if we timed out
-		if ! [ $time -lt $timeout ] ; then
-		    echo Could not start xenstored
-		    exit 1
-		fi
-
-		echo Setting domain 0 name, domid and JSON config...
-		${LIBEXEC_BIN}/xen-init-dom0
-	fi
+	echo Setting domain 0 name, domid and JSON config...
+	${LIBEXEC_BIN}/xen-init-dom0
 
 	echo Starting xenconsoled...
 	test -z "$XENCONSOLED_TRACE" || XENCONSOLED_ARGS=" --log=$XENCONSOLED_TRACE"
diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
new file mode 100644
index 0000000..a0cbfd3
--- /dev/null
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -0,0 +1,55 @@
+#!/bin/bash
+#
+# Copyright (c) 2016 SUSE Linux GmbH
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of version 2.1 of the GNU Lesser General Public
+# License 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/>.
+#
+
+XENSTORED=@XENSTORED@
+
+. @XEN_SCRIPT_DIR@/hotplugpath.sh
+test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
+
+time=0
+timeout=30
+
+if ! `${bindir}/xenstore-read -s / >/dev/null 2>&1`
+then
+	test -z "$XENSTORED_ROOTDIR" && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
+	rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
+	test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T @XEN_LOG_DIR@/xenstored-trace.log"
+
+	if [ -n "$XENSTORED" ] ; then
+	    echo -n Starting $XENSTORED...
+	    $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
+	else
+	    echo "No xenstored found"
+	    exit 1
+	fi
+
+	# Wait for xenstored to actually come up, timing out after 30 seconds
+	while [ $time -lt $timeout ] && ! `${bindir}/xenstore-read -s / >/dev/null 2>&1` ; do
+	    echo -n .
+	    time=$(($time+1))
+	    sleep 1
+	done
+	echo
+
+	# Exit if we timed out
+	if ! [ $time -lt $timeout ] ; then
+	    echo Could not start xenstored
+	    exit 1
+	fi
+fi
+
+exit 0
-- 
2.6.6


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

[PATCH v4 3/4] tools: use pidfile for test if xenstored is running

[Juergen Gross]

Instead of trying to read xenstore via xenstore-read use the pidfile
of xenstored for the test whether xenstored is running. This prepares
support of xenstore domain, as trying to read xenstore will block
for ever in case xenstore domain is started after trying to read.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4: use @XEN_RUN_DIR@ as requested by Wei Liu
---
 tools/hotplug/Linux/init.d/xencommons.in |  2 +-
 tools/hotplug/Linux/launch-xenstore.in   | 58 +++++++++++++++++++-------------
 2 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/xencommons.in b/tools/hotplug/Linux/init.d/xencommons.in
index a32608c..a6a40d6 100644
--- a/tools/hotplug/Linux/init.d/xencommons.in
+++ b/tools/hotplug/Linux/init.d/xencommons.in
@@ -96,7 +96,7 @@ case "$1" in
 	do_start
 	;;
   status)
-        ${bindir}/xenstore-read -s /
+        test -f @XEN_RUN_DIR@/xenstored.pid
 	;;
   stop)
 	do_stop
diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
index a0cbfd3..caa9345 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -18,38 +18,48 @@
 XENSTORED=@XENSTORED@
 
 . @XEN_SCRIPT_DIR@/hotplugpath.sh
-test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
 
-time=0
-timeout=30
+test_xenstore () {
+	test -f @XEN_RUN_DIR@/xenstored.pid
+	return $?
+}
 
-if ! `${bindir}/xenstore-read -s / >/dev/null 2>&1`
-then
-	test -z "$XENSTORED_ROOTDIR" && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
-	rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
-	test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T @XEN_LOG_DIR@/xenstored-trace.log"
+timeout_xenstore () {
+	local time=0
+	local timeout=30
 
-	if [ -n "$XENSTORED" ] ; then
-	    echo -n Starting $XENSTORED...
-	    $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
-	else
-	    echo "No xenstored found"
-	    exit 1
-	fi
-
-	# Wait for xenstored to actually come up, timing out after 30 seconds
-	while [ $time -lt $timeout ] && ! `${bindir}/xenstore-read -s / >/dev/null 2>&1` ; do
-	    echo -n .
-	    time=$(($time+1))
-	    sleep 1
+	while [ $time -lt $timeout ] && ! test_xenstore ; do
+		echo -n .
+		time=$(($time+1))
+		sleep 1
 	done
 	echo
-
+ 
 	# Exit if we timed out
 	if ! [ $time -lt $timeout ] ; then
-	    echo Could not start xenstored
-	    exit 1
+		echo "Could not start $@"
+		return 1
 	fi
+
+	return 0
+}
+
+test_xenstore && exit 0
+
+test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
+
+test -z "$XENSTORED_ROOTDIR" && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
+rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
+test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T @XEN_LOG_DIR@/xenstored-trace.log"
+
+if [ -n "$XENSTORED" ] ; then
+	echo -n Starting $XENSTORED...
+	$XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
+else
+	echo "No xenstored found"
+	exit 1
 fi
 
+timeout_xenstore $XENSTORED || exit 1
+
 exit 0
-- 
2.6.6


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

[PATCH v4 4/4] tools: make xenstore domain easy configurable

[Juergen Gross]

Add configuration entries to sysconfig.xencommons for selection of the
xenstore type (domain or daemon) and start the selected xenstore
service via a script called from sysvinit or systemd.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3: - remove check for running xenstore domain, as this is done in
      init-xenstore-domain already
    - if booted with systemd send a systemd-notify message in the
      xenstore domain case
    - if booted with systemd don't wait until xenstore daemon is
      running, as the daemon will have sent a notify message by its
      own
    - split up patch as requested by Ian Jackson

V2: - add .gitignore entry for launch-xenstore
---
 tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 42 ++++++++++++++++++++--
 tools/hotplug/Linux/launch-xenstore.in             | 42 ++++++++++++++++------
 tools/hotplug/Linux/systemd/xenstored.service.in   |  8 +----
 3 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index c27a476..cc8185c 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -6,12 +6,24 @@
 #XENCONSOLED_TRACE=[none|guest|hv|all]
 
 ## Type: string
+## Default: daemon
+#
+# Select type of xentore service.
+#
+# This can be either of:
+#  * daemon
+#  * domain
+#
+# Changing this requires a reboot to take effect.
+#
+#XENSTORETYPE=daemon
+
+## Type: string
 ## Default: xenstored
 #
 # Select xenstore implementation, this can be either
-# of these below. If using systemd it's preferred that you
-# just edit the xenstored.service unit file and change
-# the XENSTORED variable there.
+# of these below.
+# Only evaluated if XENSTORETYPE is "daemon".
 #
 # This can be either of:
 #  * @sbindir@/oxenstored
@@ -26,21 +38,45 @@
 # Additional commandline arguments to start xenstored,
 # like "--trace-file @XEN_LOG_DIR@/xenstored-trace.log"
 # See "@sbindir@/xenstored --help" for possible options.
+# Only evaluated if XENSTORETYPE is "daemon".
 XENSTORED_ARGS=
 
 ## Type: string
 ## Default: Not defined, tracing off
 #
 # Log xenstored messages
+# Only evaluated if XENSTORETYPE is "daemon".
 #XENSTORED_TRACE=[yes|on|1]
 
 ## Type: string
 ## Default: "@XEN_LIB_STORED@"
 #
 # Running xenstored on XENSTORED_ROOTDIR
+# Only evaluated if XENSTORETYPE is "daemon".
 #XENSTORED_ROOTDIR=@XEN_LIB_STORED@
 
 ## Type: string
+## Default: @LIBEXEC@/boot/xenstore-stubdom.gz
+#
+# xenstore domain kernel.
+# Only evaluated if XENSTORETYPE is "domain".
+#XENSTORE_DOMAIN_KERNEL=@LIBEXEC@/boot/xenstore-stubdom.gz
+
+## Type: integer
+## Default: 8
+#
+# xenstore domain memory size in MiB.
+# Only evaluated if XENSTORETYPE is "domain".
+#XENSTORE_DOMAIN_SIZE=8
+
+## Type: string
+## Default: ""
+#
+# Additional arguments for starting the xenstore domain.
+# Only evaluated if XENSTORETYPE is "domain".
+XENSTORE_DOMAIN_ARGS=
+
+## Type: string
 ## Default: Not defined, xenbackendd debug mode off
 #
 # Running xenbackendd in debug mode
diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in
index caa9345..cf98bd1 100644
--- a/tools/hotplug/Linux/launch-xenstore.in
+++ b/tools/hotplug/Linux/launch-xenstore.in
@@ -48,18 +48,40 @@ test_xenstore && exit 0
 
 test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
 
-test -z "$XENSTORED_ROOTDIR" && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
-rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
-test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T @XEN_LOG_DIR@/xenstored-trace.log"
+[ "$XENSTORETYPE" = "" ] && XENSTORETYPE=daemon
+
+/bin/mkdir -p @XEN_RUN_DIR@
+
+[ "$XENSTORETYPE" = "daemon" ] && {
+	[ -z "$XENSTORED_ROOTDIR" ] && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
+	/bin/rm -f $XENSTORED_ROOTDIR/tdb* 2>/dev/null
+	[ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T @XEN_LOG_DIR@/xenstored-trace.log"
+	[ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@
+	[ -x "$XENSTORED" ] || {
+		echo "No xenstored found"
+		exit 1
+	}
 
-if [ -n "$XENSTORED" ] ; then
 	echo -n Starting $XENSTORED...
 	$XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
-else
-	echo "No xenstored found"
-	exit 1
-fi
 
-timeout_xenstore $XENSTORED || exit 1
+	systemd-notify --booted 2>/dev/null || timeout_xenstore $XENSTORED || exit 1
 
-exit 0
+	exit 0
+}
+
+[ "$XENSTORETYPE" = "domain" ] && {
+	[ -z "$XENSTORE_DOMAIN_KERNEL" ] && XENSTORE_DOMAIN_KERNEL=@LIBEXEC@/boot/xenstore-stubdom.gz
+	XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --kernel $XENSTORE_DOMAIN_KERNEL"
+	[ -z "$XENSTORE_DOMAIN_SIZE" ] && XENSTORE_DOMAIN_SIZE=8
+	XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --memory $XENSTORE_DOMAIN_SIZE"
+
+	echo -n Starting $XENSTORE_DOMAIN_KERNEL...
+	${LIBEXEC_BIN}/init-xenstore-domain $XENSTORE_DOMAIN_ARGS || exit 1
+	systemd-notify --ready 2>/dev/null
+
+	exit 0
+}
+
+echo "illegal value $XENSTORETYPE for XENSTORETYPE"
+exit 1
diff --git a/tools/hotplug/Linux/systemd/xenstored.service.in b/tools/hotplug/Linux/systemd/xenstored.service.in
index d520d70..80c1d40 100644
--- a/tools/hotplug/Linux/systemd/xenstored.service.in
+++ b/tools/hotplug/Linux/systemd/xenstored.service.in
@@ -10,14 +10,8 @@ ConditionPathExists=/proc/xen/capabilities
 Type=notify
 NotifyAccess=all
 RemainAfterExit=true
-KillMode=none
-Environment=XENSTORED_ARGS=
-Environment=XENSTORED=@XENSTORED@
-EnvironmentFile=-@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
 ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
-ExecStartPre=-/bin/rm -f @XEN_LIB_STORED@/tdb*
-ExecStartPre=/bin/mkdir -p @XEN_RUN_DIR@
-ExecStart=/bin/sh -c "exec $XENSTORED --no-fork $XENSTORED_ARGS"
+ExecStart=@XEN_SCRIPT_DIR@/launch-xenstore
 
 [Install]
 WantedBy=multi-user.target
-- 
2.6.6


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

Re: [PATCH v4 1/4] tools: remove systemd xenstore socket definitions

[Wei Liu]

On Tue, Aug 02, 2016 at 12:39:25PM +0200, Juergen Gross wrote:
> On a system with systemd the xenstore sockets are created via systemd.
> Remove the related configuration files in order to be able to decide
> at runtime whether the sockets should be created or not. This will
> enable Xen to start xenstore either via a daemon or via a stub domain.
> 
> As the xenstore domain start program will exit after it has done its
> job prepare the same behaviour to be tolerated by systemd for the
> xenstore daemon by specifying the appropriate flags in the service
> file.
> 
> A rerun of autogen.sh is required.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

The ocaml bits would need an ack from Dave.

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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Wei Liu]

On Tue, Aug 02, 2016 at 12:39:26PM +0200, Juergen Gross wrote:
> In order to prepare starting a xenstore domain split out the starting
> of the xenstore daemon from the xencommons script into a dedicated
> launch-xenstore script.
> 
> Correct one error: don't remove old tdb files in background, as this
> could lead to very subtle races.
> 

Sorry to only notice this now -- I suppose this shouldn't be in commit
message because it is a changelog for different revision of your patch,
or you need to describe what the race is if there is really such issue
-- and that's a potential backport candidate.

As far as I can tell this is pure code motion and shouldn't have any
change in behaviour. Hence this line in commit message makes me feel a
bit confused.

Wei.

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

Re: [PATCH v4 3/4] tools: use pidfile for test if xenstored is running

[Wei Liu]

On Tue, Aug 02, 2016 at 12:39:27PM +0200, Juergen Gross wrote:
> Instead of trying to read xenstore via xenstore-read use the pidfile
> of xenstored for the test whether xenstored is running. This prepares
> support of xenstore domain, as trying to read xenstore will block
> for ever in case xenstore domain is started after trying to read.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v4 4/4] tools: make xenstore domain easy configurable

[Wei Liu]

On Tue, Aug 02, 2016 at 12:39:28PM +0200, Juergen Gross wrote:
> Add configuration entries to sysconfig.xencommons for selection of the
> xenstore type (domain or daemon) and start the selected xenstore
> service via a script called from sysvinit or systemd.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Juergen Gross]

On 02/08/16 13:13, Wei Liu wrote:
> On Tue, Aug 02, 2016 at 12:39:26PM +0200, Juergen Gross wrote:
>> In order to prepare starting a xenstore domain split out the starting
>> of the xenstore daemon from the xencommons script into a dedicated
>> launch-xenstore script.
>>
>> Correct one error: don't remove old tdb files in background, as this
>> could lead to very subtle races.
>>
> 
> Sorry to only notice this now -- I suppose this shouldn't be in commit
> message because it is a changelog for different revision of your patch,
> or you need to describe what the race is if there is really such issue
> -- and that's a potential backport candidate.

-		rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null
...
+	rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null

Please note the "&" in the removed line. And the description just tells
you that: the file shouldn't be removed in the background. How else
would you describe it?

And the race should be obvious: removing a file in the background which
is going to be used by the next command is racy.

> As far as I can tell this is pure code motion and shouldn't have any
> change in behaviour. Hence this line in commit message makes me feel a
> bit confused.

Still confused?


Juergen

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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Wei Liu]

On Tue, Aug 02, 2016 at 01:20:40PM +0200, Juergen Gross wrote:
> On 02/08/16 13:13, Wei Liu wrote:
> > On Tue, Aug 02, 2016 at 12:39:26PM +0200, Juergen Gross wrote:
> >> In order to prepare starting a xenstore domain split out the starting
> >> of the xenstore daemon from the xencommons script into a dedicated
> >> launch-xenstore script.
> >>
> >> Correct one error: don't remove old tdb files in background, as this
> >> could lead to very subtle races.
> >>
> > 
> > Sorry to only notice this now -- I suppose this shouldn't be in commit
> > message because it is a changelog for different revision of your patch,
> > or you need to describe what the race is if there is really such issue
> > -- and that's a potential backport candidate.
> 
> -		rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null
> ...
> +	rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
> 
> Please note the "&" in the removed line. And the description just tells
> you that: the file shouldn't be removed in the background. How else
> would you describe it?
> 
> And the race should be obvious: removing a file in the background which
> is going to be used by the next command is racy.
> 
> > As far as I can tell this is pure code motion and shouldn't have any
> > change in behaviour. Hence this line in commit message makes me feel a
> > bit confused.
> 
> Still confused?

Ah, the 2> vs &> thing is really subtle. I missed that, sorry.

I think you need to separate that one change out so that we can backport
it to stable releases to avoid the race.

Wei.

> 
> 
> Juergen

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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 494 bytes --]

On Tue, Aug 02, Juergen Gross wrote:

> -		rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null
> +	rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
> 
> Please note the "&" in the removed line. And the description just tells
> you that: the file shouldn't be removed in the background. How else
> would you describe it?

"&>" redirects stdout and stderr at the same time. See bash(1)
"Redirecting Standard Output and Standard Error".
Are you saying it does fork into background instead?

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Juergen Gross]

On 02/08/16 13:59, Olaf Hering wrote:
> On Tue, Aug 02, Juergen Gross wrote:
> 
>> -		rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null
>> +	rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
>>
>> Please note the "&" in the removed line. And the description just tells
>> you that: the file shouldn't be removed in the background. How else
>> would you describe it?
> 
> "&>" redirects stdout and stderr at the same time. See bash(1)
> "Redirecting Standard Output and Standard Error".
> Are you saying it does fork into background instead?

No. I'm saying that I managed to overlook that part of the man page.

I explicitly had a look as I didn't know the &> notation and I wanted to
make sure I don't miss anything. I failed obviously. :-(

Thanks for the bash lesson. :-)

V5 is about to come...


Juergen


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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons"):
> On Tue, Aug 02, 2016 at 01:20:40PM +0200, Juergen Gross wrote:
> > Still confused?
> 
> Ah, the 2> vs &> thing is really subtle. I missed that, sorry.

`&>' is a (bash-specific, I think) syntax for redirection, not a
request to run anything in the background.

Ian.

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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Ian Jackson]

Olaf Hering writes ("Re: [Xen-devel] [PATCH v4 2/4] tools: split out xenstored starting form xencommons"):
> "&>" redirects stdout and stderr at the same time. See bash(1)
> "Redirecting Standard Output and Standard Error".
> Are you saying it does fork into background instead?

Snap.  That'll teach me not to read the whole thread :-).

Ian.

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

Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons

[Wei Liu]

On Tue, Aug 02, 2016 at 02:48:54PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v4 2/4] tools: split out xenstored starting form xencommons"):
> > On Tue, Aug 02, 2016 at 01:20:40PM +0200, Juergen Gross wrote:
> > > Still confused?
> > 
> > Ah, the 2> vs &> thing is really subtle. I missed that, sorry.
> 
> `&>' is a (bash-specific, I think) syntax for redirection, not a
> request to run anything in the background.
> 

I skimmed too fast and  read that as "& >". :-/

> Ian.

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

Re: [PATCH v4 1/4] tools: remove systemd xenstore socket definitions

[David Scott]

> On 2 Aug 2016, at 12:07, Wei Liu <wei.liu2@citrix.com> wrote:
> 
> On Tue, Aug 02, 2016 at 12:39:25PM +0200, Juergen Gross wrote:
>> On a system with systemd the xenstore sockets are created via systemd.
>> Remove the related configuration files in order to be able to decide
>> at runtime whether the sockets should be created or not. This will
>> enable Xen to start xenstore either via a daemon or via a stub domain.
>> 
>> As the xenstore domain start program will exit after it has done its
>> job prepare the same behaviour to be tolerated by systemd for the
>> xenstore daemon by specifying the appropriate flags in the service
>> file.
>> 
>> A rerun of autogen.sh is required.
>> 
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> The ocaml bits would need an ack from Dave.

The OCaml bits look fine to me

Acked-by: David Scott <dave@recoil.org>

Cheers,
Dave

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