Pkg xen devel - Feb 2019 - [PATCH v2 1/3] sysconfig.xencommons.in: Strip and debianize

I redid the xenstored init script part, using snippets and ideas from
Ians feedback. The end result is much better overall.

The /etc/default/xen file now requires the user to set
/usr/bin/xenstored or /usr/sbin/xenstored, exactly as the upstream one
does. Since this will actually start the
/usr/lib/xen-X/bin/(o)xenstored, the code to detect a current running
xenstored using s-s-d just works (tm).

This is the current wip.testme branch on salsa again (I just
force-pushed), but excluding all commits that were already acked.

Hans van Kranenburg (3):
  sysconfig.xencommons.in: Strip and debianize
  xen init script: rewrite xenstored start logic
  xen init script: move init_dom0 into xenstored start

 debian/xen-utils-common.xen.init              | 58 ++++++++++-----
 .../Linux/init.d/sysconfig.xencommons.in      | 71 +------------------
 2 files changed, 44 insertions(+), 85 deletions(-)

-- 
2.20.1

Strip all options that are for stuff we don't ship, which is 1)
xenstored as stubdom and 2) xenbackendd, which seems to be dead code
anyway. [1]

It seems useful to give the user the option to revert to xenstored
instead of the default oxenstored if they really want.

[1] https://lists.xen.org/archives/html/xen-devel/2015-07/msg04427.html

Signed-off-by: Hans van Kranenburg <hans at knorrie.org>
---
 .../Linux/init.d/sysconfig.xencommons.in      | 71 +------------------
 1 file changed, 2 insertions(+), 69 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index 92569cd61b..af17925a16 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -6,26 +6,9 @@
 #XENCONSOLED_TRACE=[none|guest|hv|all]
 
 ## Type: string
-## Default: daemon
+## Default: oxenstored
 #
-# 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.
-# Only evaluated if XENSTORETYPE is "daemon".
-#
-# This can be either of:
+# Select xenstore implementation. This can be either of:
 #  * @sbindir@/oxenstored
 #  * @sbindir@/xenstored
 #
@@ -38,60 +21,10 @@
 # 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: not set, no autoballooning of xenstore domain
-#
-# Maximum xenstore domain memory size. Can be specified as:
-# - plain integer value for max size in MiB
-# - fraction of host memory, e.g. 1/100
-# - combination of both in form of <val>:<frac> (e.g. 8:1/100),
resulting
-#   value will be the higher of both specifications
-# Only evaluated if XENSTORETYPE is "domain".
-#XENSTORE_MAX_DOMAIN_SIZE-
-## 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
-#XENBACKENDD_DEBUG=[yes|on|1]
-
-# qemu path
-#QEMU_XEN=@qemu_xen_path@
-- 
2.20.1

We're adding oxenstored, and we want use it by default in Xen 4.11.

When doing a Debian upgrade from Stretch to Buster, the xen-utils-common
package will be upgraded to the new version, but it still needs to
support running the Xen 4.8 hypervisor and utils, because the user might
not have rebooted yet, or might boot into the 4.8 hypervisor again
because there were troubles running 4.11.

So, this means that oxenstored might or might not be available, and we
have to deal with that.

See comments in the code for more explanation about the new program
flow.

Also...
* Use if statements rather than constructs using || and && to make the
program flow a bit easier to understand.
* Remove the confuscating madness of having 1 as a success return code.
* Don't print the xenstored progress message if we're not touching it.

Signed-off-by: Hans van Kranenburg <hans at knorrie.org>
---
 debian/xen-utils-common.xen.init | 48 +++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init
index 4b793d5ac2..f8e4bffc54 100644
--- a/debian/xen-utils-common.xen.init
+++ b/debian/xen-utils-common.xen.init
@@ -28,7 +28,8 @@ fi
 
 XENCONSOLED="$ROOT"/bin/xenconsoled
 XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid"
-XENSTORED="$ROOT"/bin/xenstored
+CXENSTORED="$ROOT"/bin/xenstored
+OXENSTORED="$ROOT"/bin/oxenstored
 XENSTORED_PIDFILE="/var/run/xenstore.pid"
 QEMU=/usr/bin/qemu-system-i386
 QEMU_PIDFILE="/var/run/qemu-dom0.pid"
@@ -171,15 +172,48 @@ qemu_stop_real()
 
 xenstored_start()
 {
+	# First, we check if any of xenstored or oxenstored is already running. If
+	# so, we abort and leave it alone.
+	for try_xenstored in "$OXENSTORED" "$CXENSTORED"; do
+		if [ -x $try_xenstored ]; then
+			start-stop-daemon --start --quiet --pidfile "$XENSTORED_PIDFILE" \
+				--exec "$try_xenstored" --test > /dev/null
+			# s-s-b code 1 here means: "We found a matching live process!"
+			if [ $? -eq 1 ]; then
+				return 0
+			fi
+		fi
+	done
+	# If none of them are running, then try starting one. If the user made an
+	# explicit choice, then run that. Else try the different xenstored
+	# implementations we know about in order of preference.
 	log_progress_msg "xenstored"
-	start-stop-daemon --start --quiet --pidfile "$XENSTORED_PIDFILE"
--exec "$XENSTORED" --test > /dev/null \
-		|| return 1
 	[ -d "$XENSTORED_DIR" ] || mkdir -p "$XENSTORED_DIR"
 	[ -x /sbin/restorecon ] && /sbin/restorecon "$XENSTORED_DIR"
 	export XENSTORED_ROOTDIR="$XENSTORED_DIR"
-	start-stop-daemon --start --quiet --pidfile "$XENSTORED_PIDFILE"
--exec "$XENSTORED" -- \
-		$XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE" \
-		|| return 2
+	# $XENSTORED can be set in /etc/default/xen. If the user made an explicit
+	# choice about what to run, we only consider running exactly that an
+	# option. Otherwise, we run any available known xenstored implementation in
+	# order of preference.
+	case "$XENSTORED" in
+	?*) try_xenstoreds='XENSTORED' ;;
+	'') try_xenstoreds='OXENSTORED CXENSTORED' ;;
+	esac
+	for try_xenstored_var in $try_xenstoreds; do
+		eval "try_xenstored=\$$try_xenstored_var"
+		if [ -x $try_xenstored ]; then
+			if start-stop-daemon --start --quiet \
+				--pidfile "$XENSTORED_PIDFILE" --exec "$try_xenstored"
-- \
+				$XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE"; then
+					started_xenstored=$try_xenstored
+					break
+			fi
+		fi
+	done
+	if [ -z "$started_xenstored" ]; then
+		log_failure_msg "No suitable xenstored binary found to start."
+		return 1
+	fi
 
 	# Wait for xenstored to actually come up, timing out after 30 seconds
 	local time=0
@@ -223,7 +257,7 @@ case "$1" in
 	env_setup
 	xenstored_start
 	case "$?" in
-		0|1) ;;
+		0) ;;
 		*) log_end_msg 1; exit ;;
 	esac
 	xenconsoled_start
-- 
2.20.1

Executing this is only necessary after starting xenstored. In all other
cases it just prints noop spam to stderr.

    -# /usr/lib/xen-4.11/bin/xen-init-dom0 >/dev/null
    Dom0 is already set up

I don't want to silence stderr, so just move the code into the end of
the xenstored start function.
---
 debian/xen-utils-common.xen.init | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init
index f8e4bffc54..159620291a 100644
--- a/debian/xen-utils-common.xen.init
+++ b/debian/xen-utils-common.xen.init
@@ -227,11 +227,8 @@ xenstored_start()
 	if ! [ $time -lt $timeout ] ; then
 	    return 2
 	fi
-}
 
-init_dom0()
-{
-	log_progress_msg "init-dom0"
+	# Add some extra entries about dom0 into xenstore
 	if [ -e $ROOT/bin/xen-init-dom0 ] ; then
 	    $ROOT/bin/xen-init-dom0 > /dev/null
 	else
@@ -265,11 +262,6 @@ case "$1" in
 		0|1) ;;
 		*) log_end_msg 1; exit ;;
 	esac
-	init_dom0
-	case "$?" in
-		0|1) ;;
-		*) log_end_msg 1; exit ;;
-	esac
 	qemu_start
 	case "$?" in
 		0|1) ;;
-- 
2.20.1

On 2/13/19 1:24 AM, Hans van Kranenburg wrote:
> Strip all options that are for stuff we don't ship, which is 1)
> xenstored as stubdom and 2) xenbackendd, which seems to be dead code
> anyway. [1]
> 
> It seems useful to give the user the option to revert to xenstored
> instead of the default oxenstored if they really want.
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2015-07/msg04427.html
> 
> Signed-off-by: Hans van Kranenburg <hans at knorrie.org>
> ---
>  .../Linux/init.d/sysconfig.xencommons.in      | 71 +------------------
>  1 file changed, 2 insertions(+), 69 deletions(-)
> 
> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> index 92569cd61b..af17925a16 100644
> --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> @@ -6,26 +6,9 @@
>  #XENCONSOLED_TRACE=[none|guest|hv|all]
>  
>  ## Type: string
> -## Default: daemon
> +## Default: oxenstored
>  #
> -# 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.
> -# Only evaluated if XENSTORETYPE is "daemon".
> -#
> -# This can be either of:
> +# Select xenstore implementation. This can be either of:
>  #  * @sbindir@/oxenstored
>  #  * @sbindir@/xenstored
>  #
> @@ -38,60 +21,10 @@
>  # 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]
Grmbl, grmbl, still need to fix this one to be actually used. Note to
self: setting it to values like 0, off or no will still enable it in the
upstream code. (╯°□°)╯( ┻━┻
> -
> -## 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: not set, no autoballooning of xenstore domain
> -#
> -# Maximum xenstore domain memory size. Can be specified as:
> -# - plain integer value for max size in MiB
> -# - fraction of host memory, e.g. 1/100
> -# - combination of both in form of <val>:<frac> (e.g.
8:1/100), resulting
> -#   value will be the higher of both specifications
> -# Only evaluated if XENSTORETYPE is "domain".
> -#XENSTORE_MAX_DOMAIN_SIZE> -
> -## 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
> -#XENBACKENDD_DEBUG=[yes|on|1]
> -
> -# qemu path
> -#QEMU_XEN=@qemu_xen_path@
>

Hans van Kranenburg writes ("[PATCH v2 0/3] Remaining wip.testme
changes"):
> I redid the xenstored init script part, using snippets and ideas from
> Ians feedback. The end result is much better overall.
This all looks great to me, thanks!

Acked-by: Ian Jackson <ijackson at chiark.greenend.org.uk>

Ian.

-- 
Ian Jackson <ijackson at chiark.greenend.org.uk>   These opinions are my
own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.