Pkg xen devel - Feb 2019 - [PATCH 09/13] xen init script: move init_dom0 into xenstored start

The contents are the wip.testme branch currently on salsa. I combined
the wip.initscript and wip.oxenstored into this and added more things
today. I think this is pretty gtg and it's smoke tested (in several
cases by scping files around instead of doing package build), so it
needs a final extra review and test round before putting it in master
branch (which I don't want to force push).

I redid the xenstored init script changes, also including stipped down
/etc/default/xen again. I tested happy and sad paths in the code, but
did not do a full test going back to 4.8 and upgrading again etc.

I finished my non-documentation part of the TODO, so next thing for this
for me will be looking into README.Debian and NEWS.Debian.

TODO:
https://salsa.debian.org/xen-team/debian-xen/issues/24

Also, there are no debian/changelog changes in here yet.

Hans van Kranenburg (8):
  sysconfig.xencommons.in: Strip and debianize
  d/xen-utils-common.install: ship /etc/default/xen
  xen init script: rewrite xenstored start logic
  xen init script: move init_dom0 into xenstored start
  tools/xl/bash-completion: improved completion
  tools/xl/bash-completion: also complete 'xen'
  d/x-u-common.install: install completion as as xl
  d/[..]/grub.d/xen.cfg: improve docs even more

Ian Jackson (5):
  xen version/upgrade handling: Improve an error message
  xen init script: silently exit status 0 if not running under xen
  xen init script: Do nothing if running for wrong Xen package
  xen init script: Tidy up wrong/missing Xen version error handling
  oxenstored: Build it

 debian/control                                |   5 +-
 debian/not-installed                          |   5 +
 debian/rules                                  |   1 -
 debian/scripts/xen-dir                        |   2 +-
 .../etc/default/grub.d/xen.cfg                |  43 ++--
 debian/xen-utils-common.install               |   6 +-
 debian/xen-utils-common.xen.init              | 111 +++++++--
 .../Linux/init.d/sysconfig.xencommons.in      |  79 +------
 tools/xl/bash-completion                      | 223 +++++++++++++++++-
 9 files changed, 341 insertions(+), 134 deletions(-)
 mode change 100644 => 100755 debian/xen-utils-common.install

-- 
2.20.1

From: Ian Jackson <ian.jackson at citrix.com>

When xen-dir cannot find xen-utils, mention that this might be because
xen-utils-<RUNNING-XEN-VERSION> was already removed.

This is generally helpful, but it does not solve the `missing
xenconsoled' problems because 1. it only changes messages and
2. actually in the init script, the error message is currently
discarded anyway (!)

But, anyway, it is an improvemennt.

Signed-off-by: Ian Jackson <ian.jackson at citrix.com>
---
 debian/scripts/xen-dir | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/scripts/xen-dir b/debian/scripts/xen-dir
index 32b5776cf3..48fe4043e1 100755
--- a/debian/scripts/xen-dir
+++ b/debian/scripts/xen-dir
@@ -5,6 +5,6 @@ VERSION=$(. /usr/lib/xen-common/bin/xen-version); RET=$?; [ $RET
-eq 0 ] || exit
 if [ -d "/usr/lib/xen-$VERSION" ]; then
     echo "/usr/lib/xen-$VERSION"
 else
-    echo "ERROR:  Can't find version $VERSION of xen utils, bailing
out!" >&2
+    echo "ERROR:  Can't find version $VERSION of xen utils (maybe
xen-utils-$VERSION was already removed before rebooting out of Xen $VERSION),
bailing out!" >&2
     exit 127
 fi
-- 
2.20.1

From: Ian Jackson <ian.jackson at citrix.com>

This script should not complain at all if we are not running under
Xen.  Check this right at the start.

This will enable improvements to the following code, which will no
longer have to deal with the `not running under Xen' case.

Signed-off-by: Ian Jackson <ian.jackson at citrix.com>
---
 debian/xen-utils-common.xen.init | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init
index 4b793d5ac2..da7d361453 100644
--- a/debian/xen-utils-common.xen.init
+++ b/debian/xen-utils-common.xen.init
@@ -20,6 +20,10 @@ XENSTORED_DIR="/var/run/xenstored"
 PATH=/sbin:/bin:/usr/sbin:/usr/bin
 DESC="Xen daemons"
 
+case "$(cat /sys/hypervisor/type 2>/dev/null)" in
+xen)		;;
+*)	exit 0	;; # not running under Xen
+esac
 ROOT=$(/usr/lib/xen-common/bin/xen-dir 2>/dev/null)
 if [ $? -ne 0 ]; then
 	log_warning_msg "Not running within Xen or no compatible utils"
-- 
2.20.1

From: Ian Jackson <ian.jackson at citrix.com>

See the big comment.  We think that this is responsible for various
bugs and, particularly, reports of mysteriously missing xenconsoled.

For example, this bug would mean that after a Xen version upgrade,
autoremoval of an obsolete xen-utils-V package would stop the running
xenconsoled.  This is obviously awkward to track down, and could occur
many weeks or months after the upgrade.

Closes: #851654
Signed-off-by: Ian Jackson <ian.jackson at citrix.com>
---
 debian/xen-utils-common.xen.init | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init
index da7d361453..b0df7be8ab 100644
--- a/debian/xen-utils-common.xen.init
+++ b/debian/xen-utils-common.xen.init
@@ -24,6 +24,35 @@ case "$(cat /sys/hypervisor/type 2>/dev/null)"
in
 xen)		;;
 *)	exit 0	;; # not running under Xen
 esac
+
+VERSION=$(/usr/lib/xen-common/bin/xen-version)
+
+# The arrangements for the `xen' init script are a bit odd.
+# This script is part of xen-utils-common, of which there is one
+# version installed regardless of the Xen version.
+#
+# But it is called by the prerm and postinsts of xen-utils-VERSION.
+# The idea is that (for example) if xen-utils-VERSION is upgraded, the
+# daemons are restarted.
+#
+# However, this means that this script may be called by the
+# maintscript of a xen-utils-V package for a different V to the
+# running version of Xen (X, say).  Such a xen-utils-V package does
+# not actually want to start or stop its daemons.  Indeed, the version
+# selection machinery would redirect its efforts to the xen-utils-X
+# utilities.  But this is not right: we don't actually want to (for
+# example) stop xenconsoled from xen-utils-X just because some
+# not-currently-relevant xen-utils-V is installed/removed/whatever.
+#
+# So we use DPKG_MAINTSCRIPT_PACKAGE to detect this situation, and
+# turn these extraneous calls into no-ops.
+
+case $DPKG_MAINTSCRIPT_PACKAGE in
+xen-utils-$VERSION)	;;	# xen-utils-V maintscript, under Xen X=V
+xen-utils-*)		exit 0;; # xen-utils-V maintscript, but under Xen X!=V
+*)			;;	# maybe not under dpkg, etc.
+esac
+
 ROOT=$(/usr/lib/xen-common/bin/xen-dir 2>/dev/null)
 if [ $? -ne 0 ]; then
 	log_warning_msg "Not running within Xen or no compatible utils"
-- 
2.20.1

From: Ian Jackson <ian.jackson at citrix.com>

We no longer want to discard the stderr from xen-dir, and treat this
as a success.  All the reasons why this failure might previously have
been thought tolerable have been dealt with.

Specifically, we will no longer reach this code if we are not running
under Xen, or if we ran this init script on behalf of a xen-utils-V
package for some V different to the running Xen version.

We know we are running under Xen, and that either we're running not as
a result of a maint script, or as a result of a xen-utils-V maint
script for the running Xen version, or as a result of some other maint
script (of which we don't think there are any, but it presumably
expects this code to work).

So if xen-dir fails, let it print its error message, and also exit
nonzero.  And don't mention not running under Xen in our
log_warning_msg.

Signed-off-by: Ian Jackson <ian.jackson at citrix.com>
---
 debian/xen-utils-common.xen.init | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init
index b0df7be8ab..a352823040 100644
--- a/debian/xen-utils-common.xen.init
+++ b/debian/xen-utils-common.xen.init
@@ -53,10 +53,10 @@ xen-utils-*)		exit 0;; # xen-utils-V maintscript, but under
Xen X!=V
 *)			;;	# maybe not under dpkg, etc.
 esac
 
-ROOT=$(/usr/lib/xen-common/bin/xen-dir 2>/dev/null)
+ROOT=$(/usr/lib/xen-common/bin/xen-dir)
 if [ $? -ne 0 ]; then
-	log_warning_msg "Not running within Xen or no compatible utils"
-	exit 0
+	log_warning_msg "No compatible Xen utils for Xen $VERSION"
+	exit 1
 fi
 
 XENCONSOLED="$ROOT"/bin/xenconsoled
-- 
2.20.1

From: Ian Jackson <ian.jackson at citrix.com>

 * Add the relevant build dependencies

     ocaml-native-compilers is good on stretch because it
     will get us better output code.  In buster the
     ocaml-native-compilers package is merged into ocaml-nox.
     In bullseye we can drop ocaml-native-compilers from the list.

 * Drop the rules line that disables the ocaml build.

 * Ship /etc/xen/oxenstored.conf.

 * Placate dh_missing about ocaml development libraries.

Signed-off-by: Ian Jackson <ian.jackson at citrix.com>
[add trailing comma, fix typo, change bulleseye line]
Signed-off-by: Hans van Kranenburg <hans at knorrie.org>
---
 debian/control                  | 4 +++-
 debian/not-installed            | 5 +++++
 debian/rules                    | 1 -
 debian/xen-utils-common.install | 1 +
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/debian/control b/debian/control
index b7b670cd9c..72dc3e76de 100644
--- a/debian/control
+++ b/debian/control
@@ -28,7 +28,9 @@ Build-Depends:
    uuid-dev,
    zlib1g-dev,
    pandoc,
-   markdown
+   markdown,
+   ocaml-native-compilers | ocaml-nox,
+   ocaml-findlib,
 XS-Python-Version: current
 Homepage: https://xenproject.org/
 Vcs-Browser: https://salsa.debian.org/xen-team/debian-xen
diff --git a/debian/not-installed b/debian/not-installed
index 17a331400e..5ffa447587 100644
--- a/debian/not-installed
+++ b/debian/not-installed
@@ -23,3 +23,8 @@ etc/xen/README
 # some of the code in the upstream init scripts.
 etc/xen/README.incompatibilities
 
+# When we build with ocaml support, do not try to ship development
+# libraries which we don't understand and are of limited use.
+# If someone wants this, suggestions from ocaml experts on what
+# to ship where would be welcome.
+usr/local/lib/ocaml
diff --git a/debian/rules b/debian/rules
index 09babb2133..d6a626430e 100755
--- a/debian/rules
+++ b/debian/rules
@@ -182,7 +182,6 @@ override_dh_auto_configure:
 		--with-libexec-leaf-dir=xen-$(upstream_version) \
 		--disable-blktap1 \
 		--disable-blktap2 \
-		--disable-ocamltools \
 		--disable-qemu-traditional --disable-rombios \
 		--with-system-qemu=/usr/bin/qemu-system-i386 \
 		--enable-ovmf --with-system-ovmf=/usr/share/ovmf/OVMF.fd \
diff --git a/debian/xen-utils-common.install b/debian/xen-utils-common.install
index efd0fa25ed..0f6cb8e3c2 100644
--- a/debian/xen-utils-common.install
+++ b/debian/xen-utils-common.install
@@ -3,6 +3,7 @@ etc/xen/xl*
 etc/bash_completion.d/xl.sh	usr/share/bash-completion/completions/
 
 etc/default/xendomains
+etc/xen/oxenstored.conf
 
 usr/bin
 usr/sbin
-- 
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.

In order to configure this, XENSTORED in here needs to contain the name
without a path, which is different than how the upstream file suggests
to use it.

However, we don't want to put a version-specific path in here (the user
keeps the config file on upgrades if it's changed, and then will end up
with a path to a non-existing binary). Also, alternatively,
/usr/sbin/oxenstored and /usr/sbin/xenstored could be used, but those
are symlinks to xen-utils-wrapper and not the actual binary, and I think
the test using start-stop-daemon to see if it's already running would
fail to detect it's the same as e.g. /usr/lib/xen-4.11/bin/oxenstored.
At least, it all sounds very fragile. :|

[1] https://lists.xen.org/archives/html/xen-devel/2015-07/msg04427.html
---
 .../Linux/init.d/sysconfig.xencommons.in      | 79 ++-----------------
 1 file changed, 6 insertions(+), 73 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index 92569cd61b..dc978bc75c 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -6,92 +6,25 @@
 #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:
-#  * @sbindir@/oxenstored
-#  * @sbindir@/xenstored
+# Select xenstore implementation. This can be either of:
+#  * oxenstored
+#  * xenstored
 #
 # Changing this requires a reboot to take effect.
-#XENSTORED=@XENSTORED@
+#XENSTORED=oxenstored
 
 ## Type: string
 ## Default: ""
 #
 # 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".
+# See "@LIBEXEC_LIB@/xenstored --help" for possible options.
 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

This file has been in the package before, it contained TOOLSTACK= to
switch between xm and xl. It was abandoned and never cleaned up, so old
installs still have this file.

Ship it again.
---
 debian/control                  | 1 +
 debian/xen-utils-common.install | 3 +++
 2 files changed, 4 insertions(+)
 mode change 100644 => 100755 debian/xen-utils-common.install

diff --git a/debian/control b/debian/control
index 72dc3e76de..167f8dfcc0 100644
--- a/debian/control
+++ b/debian/control
@@ -6,6 +6,7 @@ Section: admin
 Standards-Version: 3.9.4
 Build-Depends:
    debhelper (>= 10),
+   dh-exec,
    autotools-dev,
    dpkg-dev (>= 1.16.0~),
    rdfind,
diff --git a/debian/xen-utils-common.install b/debian/xen-utils-common.install
old mode 100644
new mode 100755
index 0f6cb8e3c2..6f48749139
--- a/debian/xen-utils-common.install
+++ b/debian/xen-utils-common.install
@@ -1,8 +1,11 @@
+#! /usr/bin/dh-exec --with=install
+
 etc/xen/scripts
 etc/xen/xl*
 etc/bash_completion.d/xl.sh	usr/share/bash-completion/completions/
 
 etc/default/xendomains
+etc/default/xencommons => /etc/default/xen
 etc/xen/oxenstored.conf
 
 usr/bin
-- 
2.20.1

By adding oxenstored, this just got a whole lot more complex.

In the Xen 4.11 packages and newer, we're shipping oxenstored. This init
script can still be used while the current running Xen version is e.g.
4.8.

We give the user the option to define which xenstored they want to run
(in /etc/default/xen). The C one or the ocaml one.

Whenever any xenstored is running, we don't touch it, because all state
about domains is only kept in memory in the process.

Combining all of the possiblities and limitations...

* If there's anything known running, just let it be.
* If the user has a specific (o)xenstored set as preference, try
starting that, but horribly explode if that's not possible.
* Otherwise, prefer oxenstored if available, and otherwise start normal
C xenstored.

Hint: debugging this on a dom0 can be done by adding set -x and running
it with SYSTEMCTL_SKIP_REDIRECT=YOLO /etc/init.d/xen <action> so that
systemd does not hijack it and hide all output.
---
 debian/xen-utils-common.xen.init | 55 ++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init
index a352823040..f736b9a17c 100644
--- a/debian/xen-utils-common.xen.init
+++ b/debian/xen-utils-common.xen.init
@@ -61,7 +61,15 @@ fi
 
 XENCONSOLED="$ROOT"/bin/xenconsoled
 XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid"
-XENSTORED="$ROOT"/bin/xenstored
+# In /etc/default/xen, the user can set XENSTORED, which has to be either
+# 'xenstored' or 'oxenstored'. In here, we add the version
specific path.
+if [ -n "$XENSTORED" ]; then
+	USER_XENSTORED="$ROOT"/bin/$XENSTORED
+else
+	USER_XENSTORED=""
+fi
+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"
@@ -204,15 +212,50 @@ 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, even if the user has switched the value
+	# in /etc/default/xen from one to another.
+	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
+			if [ $? -eq 1 ]; then
+				return 1
+			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
+	if [ -n "$USER_XENSTORED" ]; then
+		if [ ! -x "$USER_XENSTORED" ]; then
+			log_failure_msg "Failed to start $USER_XENSTORED: no such
executable."
+			return 2
+		else
+			start-stop-daemon --start --quiet \
+				--pidfile "$XENSTORED_PIDFILE" --exec "$USER_XENSTORED"
-- \
+				$XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE"
+			if [ $? -ne 0 ]; then
+				return 2
+			fi
+		fi
+	else
+		for try_xenstored in "$OXENSTORED" "$CXENSTORED"; do
+			if [ -x $try_xenstored ]; then
+				start-stop-daemon --start --quiet \
+					--pidfile "$XENSTORED_PIDFILE" --exec "$try_xenstored"
-- \
+					$XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE"
+				if [ $? -ne 0 ]; then
+					return 2
+				fi
+				break
+			fi
+		done
+	fi
 
 	# Wait for xenstored to actually come up, timing out after 30 seconds
 	local time=0
-- 
2.20.1

This little xen-init-dom0 program is present in both our 4.8 and 4.11
packages, so there's no need to keep the if else code. Also, it only
makes sense to run it after starting xenstored, and not all other times
start is called.

xen-init-dom0 is a C program that by the way does exactly those two
writes into xenstore.
---
 debian/xen-utils-common.xen.init | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init
index f736b9a17c..748c1cfc0a 100644
--- a/debian/xen-utils-common.xen.init
+++ b/debian/xen-utils-common.xen.init
@@ -269,17 +269,9 @@ xenstored_start()
 	if ! [ $time -lt $timeout ] ; then
 	    return 2
 	fi
-}
 
-init_dom0()
-{
-	log_progress_msg "init-dom0"
-	if [ -e $ROOT/bin/xen-init-dom0 ] ; then
-	    $ROOT/bin/xen-init-dom0 > /dev/null
-	else
-	    xenstore-write "/local/domain/0/name" "Domain-0"
-	    xenstore-write "/local/domain/0/domid" "0"
-	fi
+	# Add some extra entries about dom0 into xenstore
+	$ROOT/bin/xen-init-dom0 > /dev/null
 }
 
 case "$1" in
@@ -307,11 +299,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

While requesting removal of xm command line completion in the Debian
bash-completion package, I ran into a bts about completion for xl:

  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=768005

I like the proposed improved version that Adi Kriegisch provides in
there. It makes sure  we're back on the same advanced level of
completion as in the past, with xm.

This change should be sent upstream asap, so that we can drop this patch
again. However don't wait for that to happen and ship it already.

Closes: #768005
---
 tools/xl/bash-completion | 223 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 210 insertions(+), 13 deletions(-)

diff --git a/tools/xl/bash-completion b/tools/xl/bash-completion
index b7cd6b3992..1db0bdc48e 100644
--- a/tools/xl/bash-completion
+++ b/tools/xl/bash-completion
@@ -1,20 +1,217 @@
-# Copy this file to /etc/bash_completion.d/xl.sh
+# bash completion for xl                                   -*- shell-script -*-
+# this is the original bash completion script for xm modified for use with xl
+
+_xen_domain_names()
+{
+    COMPREPLY=( $( compgen -W "$( xl list 2>/dev/null | \
+        awk '!/Name|Domain-0/ { print $1 }'  )" --
"$cur" ) )
+}
+
+_xen_domain_ids()
+{
+    COMPREPLY=( $( compgen -W "$( xl list 2>/dev/null | \
+        awk '!/Name|Domain-0/ { print $2 }' )" -- "$cur"
) )
+}
 
 _xl()
 {
-	local IFS=$'\n,'
+    local cur prev words cword
+    _init_completion || return
 
-	local cur opts xl
-	COMPREPLY=()
-	cur="${COMP_WORDS[COMP_CWORD]}"
-	xl=xl
+    # TODO: _split_longopt
 
-	if [[ $COMP_CWORD == 1 ]] ; then
-		opts=`${xl} help 2>/dev/null | sed '1,4d' | awk '/^ [^ ]/
{print $1}' | sed 's/$/ ,/g'` && COMPREPLY=( $(compgen -W
"${opts}" -- ${cur}) )
-		return 0
-	fi
+    local args command commands options
 
-	return 0
-}
+    # command list is taken from xen-util-common's xl.sh command completion
+    commands=$(xl help 2>/dev/null | sed '1,4d' | awk '/^ [^ ]/
{print $1}' | tr '\n' ' ')
+
+    if [[ $cword -eq 1 ]] ; then
+        COMPREPLY=( $( compgen -W "$commands" -- "$cur" ) )
+    else
+        if [[ "$cur" == *=* ]]; then
+            prev=${cur/=*/}
+            cur=${cur/*=/}
+        fi
+
+        command=${words[1]}
+        if [[ "$cur" == -* ]]; then
+            # possible options for the command
+            case $command in
+                create)
+                    options='-c'
+                    ;;
+                dmesg)
+                    options='--clear'
+                    ;;
+                list)
+                    options='--long'
+                    ;;
+                reboot)
+                    options='-w -a'
+                    ;;
+                shutdown)
+                    options='-w -a -R -H'
+                    ;;
+                sched-credit)
+                    options='-d -w -c'
+                    ;;
+                block-list|network-list|vtpm-list|vnet-list)
+                    options='-l --long'
+                    ;;
+                getpolicy)
+                    options='--dumpxml'
+                    ;;
+                new)
+                    options='-h --help --help_config -q --quiet --path= -f+
--defconfig= -F= --config= -b --dryrun -x --xmldryrun
+                        -s --skipdtd -p --paused -c --console_autoconnect'
+                    ;;
+            esac
+            COMPREPLY=( $( compgen -W "$options" -- "$cur"
) )
+        else
+            case $command in
+                console|destroy|domname|domid|list|mem-set|mem-max| \
+                pause|reboot|rename|shutdown|unpause|vcpu-list|vcpu-pin| \
+                vcpu-set|block-list|network-list|vtpm-list)
+                    _count_args
+                    case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                    esac
+                    ;;
+                migrate)
+                    _count_args
+                        case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                        3)
+                            _known_hosts_real -- "$cur"
+                            ;;
+                    esac
+                    ;;
+                restore|dry-run|vnet-create)
+                    _filedir
+                    ;;
+                save)
+                    _count_args
+                    case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                        3)
+                            _filedir
+                            ;;
+                    esac
+                    ;;
+                sysrq)
+                    _count_args
+                    case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                        3)
+                            COMPREPLY=( $( compgen -W "r s e i u b" \
+                                -- "$cur" ) )
+                            ;;
+                    esac
+                    ;;
+                block-attach)
+                    _count_args
+                    case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                        3)
+                            COMPREPLY=( $( compgen -W "phy: file:" \
+                                -- "$cur" ) )
+                            ;;
+                        5)
+                            COMPREPLY=( $( compgen -W "w r" --
"$cur" ) )
+                            ;;
+                        6)
+                            _xen_domain_names
+                            ;;
+                    esac
+                    ;;
+                block-detach)
+                    _count_args
+                    case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                        3)
+                            COMPREPLY=( $( compgen -W "$( xl block-list
$prev \
+                                2>/dev/null | awk '!/Vdev/ { print $1
}' )" \
+                                -- "$cur" ) )
+                            ;;
+                    esac
+                    ;;
+                network-attach)
+                    _count_args
+                    case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                        *)
+                            COMPREPLY=( $( compgen -W "script= ip= mac=
bridge+                                backend=" -- "$cur" ) )
+                            ;;
+                    esac
+                    ;;
+                network-detach)
+                    _count_args
+                    case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                        3)
+                            COMPREPLY=( $(compgen -W "$( xl network-list
$prev \
+                                2>/dev/null | awk '!/Idx/ { print $1
}' )" \
+                                -- "$cur" ) )
+                            ;;
+                    esac
+                    ;;
+                sched-credit)
+                    case $prev in
+                        -d)
+                            _xen_domain_names
+                            return
+                            ;;
+                    esac
+                    ;;
+                create)
+                    # we need to give the full path to the config file and
+                    # do not want to complete any other file outside /etc/xen
+                    _filedir
+                    COMPREPLY+=( \
+                        $( compgen -W '$( command ls /etc/xen/*.cfg
2>/dev/null )' \
+                        -- "$cur" ) )
+                    ;;
+                new)
+                    case $prev in
+                        -f|-F|--defconfig|--config)
+                            _filedir
+                            return
+                            ;;
+                        --path)
+                            _filedir -d
+                            return
+                            ;;
+                    esac
+
+                    _count_args
+                        case $args in
+                        2)
+                            _xen_domain_names
+                            ;;
+                    esac
+                    ;;
+            esac
+        fi
+    fi
+} &&
+complete -F _xl xl
 
-complete -F _xl -o nospace -o default xl
+# ex: filetype=sh
-- 
2.20.1

We have the `xen` alias for xl in Debian, since in the past it was a
command that could execute either xl or xm.

Now, it always does xl, so, complete the same stuff for it as we have
for xl.
---
 tools/xl/bash-completion | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/xl/bash-completion b/tools/xl/bash-completion
index 1db0bdc48e..022119a252 100644
--- a/tools/xl/bash-completion
+++ b/tools/xl/bash-completion
@@ -212,6 +212,6 @@ _xl()
         fi
     fi
 } &&
-complete -F _xl xl
+complete -F _xl xl xen
 
 # ex: filetype=sh
-- 
2.20.1

...and not as xl.sh. All other files in that completions directory also
don't have .sh. This is just cosmetics, but it annoys me. And we have
dh-exec in here now anyway. :]
---
 debian/xen-utils-common.install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/xen-utils-common.install b/debian/xen-utils-common.install
index 6f48749139..60642c9a9c 100755
--- a/debian/xen-utils-common.install
+++ b/debian/xen-utils-common.install
@@ -2,7 +2,7 @@
 
 etc/xen/scripts
 etc/xen/xl*
-etc/bash_completion.d/xl.sh	usr/share/bash-completion/completions/
+etc/bash_completion.d/xl.sh => usr/share/bash-completion/completions/xl
 
 etc/default/xendomains
 etc/default/xencommons => /etc/default/xen
-- 
2.20.1

Ok, I finally tried this, and it's really great that I now have correct
(different) linux kernel options generated for just linux and xen+dom0.

Do more cosmetics and reorganizing to make this wall of comments better
parseable by the eye. It was not obvious to me where sections started
and ended, and I like to have first the explanation and then at the end
the actual variables you can set.
---
 .../etc/default/grub.d/xen.cfg                | 43 ++++++++++---------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/debian/tree/xen-hypervisor-common/etc/default/grub.d/xen.cfg
b/debian/tree/xen-hypervisor-common/etc/default/grub.d/xen.cfg
index feb4be78e9..d528293fb5 100644
--- a/debian/tree/xen-hypervisor-common/etc/default/grub.d/xen.cfg
+++ b/debian/tree/xen-hypervisor-common/etc/default/grub.d/xen.cfg
@@ -1,20 +1,16 @@
-#
-# Uncomment the following variable and set to 0 or 1 to avoid warning.
-#
-#XEN_OVERRIDE_GRUB_DEFAULT=0
-
-echo "Including Xen overrides from /etc/default/grub.d/xen.cfg"
-
-#
 # When running update-grub with the Xen hypervisor installed, there are
 # some additional variables that can be used to pass options to the
 # hypervisor or the dom0 kernel.
-
-# The following two are used to generate arguments for the hypervisor:
 #
-#GRUB_CMDLINE_XEN_DEFAULT=""
-#GRUB_CMDLINE_XEN=""
+# The configuration in here makes it possible to have different options set
+# for the linux kernel when booting with or without Xen.
+
+echo "Including Xen overrides from /etc/default/grub.d/xen.cfg"
+
+#######################################################################
+# Xen Hypervisor Command Line Options
 #
+# The first two options are used to generate arguments for the hypervisor.
 # Commonly used options are:
 #
 # dom0_mem=<size>
@@ -47,12 +43,15 @@ echo "Including Xen overrides from
/etc/default/grub.d/xen.cfg"
 # documentation for the version of Xen you have installed. This
 # documentation can be found at https://xenbits.xen.org/
 #
-# The next two lines are used for creating kernel arguments for the dom0
-# kernel. This allows to have different options for the same kernel used
-# natively or as dom0 kernel.
+#GRUB_CMDLINE_XEN_DEFAULT=""
+#GRUB_CMDLINE_XEN=""
+
+#######################################################################
+# Dom0 Kernel Command Line Options
 #
-#GRUB_CMDLINE_LINUX_XEN_REPLACE_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT"
-#GRUB_CMDLINE_LINUX_XEN_REPLACE="$GRUB_CMDLINE_LINUX"
+# The next options are used for creating kernel arguments for the dom0
+# kernel.  This allows to have different options for the same kernel used
+# natively or as dom0 kernel.
 #
 # For example:
 #
@@ -60,14 +59,18 @@ echo "Including Xen overrides from
/etc/default/grub.d/xen.cfg"
 #   Allows to send early printk messages to the Xen hypervisor console
 #
 # console=hvc0
-#   Redirects the Linux console to the hypervisor console. When running
+#   Redirects the Linux console to the hypervisor console.  When running
 #   under Xen as dom0, this needs to be specified, because the usual ttyS0
 #   will not be available. When viewing the serial console, pressing Ctrl-A
 #   three times will switch between the Linux dom0 and the Xen Hypervisor.
+#
+#GRUB_CMDLINE_LINUX_XEN_REPLACE_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT"
+#GRUB_CMDLINE_LINUX_XEN_REPLACE="$GRUB_CMDLINE_LINUX"
 
+# Make booting into Xen the default.  Set the following variable to 0 or 1
+# explicitly to avoid the message warning from being printed.
 #
-# Make booting into Xen the default if not changed above. Finding the
-# current string for it always has been a problem.
+#XEN_OVERRIDE_GRUB_DEFAULT #
 if [ "$XEN_OVERRIDE_GRUB_DEFAULT" = "" ]; then
 	echo "WARNING: GRUB_DEFAULT changed to boot into Xen by default!"
-- 
2.20.1

On 2/11/19 12:42 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.
> 
> In order to configure this, XENSTORED in here needs to contain the name
> without a path, which is different than how the upstream file suggests
> to use it.
> 
> However, we don't want to put a version-specific path in here (the user
> keeps the config file on upgrades if it's changed, and then will end up
> with a path to a non-existing binary). Also, alternatively,
> /usr/sbin/oxenstored and /usr/sbin/xenstored could be used, but those
> are symlinks to xen-utils-wrapper and not the actual binary, and I think
> the test using start-stop-daemon to see if it's already running would
> fail to detect it's the same as e.g. /usr/lib/xen-4.11/bin/oxenstored.
> At least, it all sounds very fragile. :|
> 
> [1] https://lists.xen.org/archives/html/xen-devel/2015-07/msg04427.html
> ---
>  .../Linux/init.d/sysconfig.xencommons.in      | 79 ++-----------------
>  1 file changed, 6 insertions(+), 73 deletions(-)
> 
> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> index 92569cd61b..dc978bc75c 100644
> --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> @@ -6,92 +6,25 @@
>  #XENCONSOLED_TRACE=[none|guest|hv|all]
Hmz, this one still gets ignored in the init script.
>  
>  ## 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:
> -#  * @sbindir@/oxenstored
> -#  * @sbindir@/xenstored
> +# Select xenstore implementation. This can be either of:
> +#  * oxenstored
> +#  * xenstored
>  #
>  # Changing this requires a reboot to take effect.
> -#XENSTORED=@XENSTORED@
> +#XENSTORED=oxenstored
>  
>  ## Type: string
>  ## Default: ""
>  #
>  # 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".
> +# See "@LIBEXEC_LIB@/xenstored --help" for possible options.
>  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@
>

H'okay, so...

On 2/11/19 12:42 AM, Hans van Kranenburg wrote:
> We have the `xen` alias for xl in Debian, since in the past it was a
> command that could execute either xl or xm.
> 
> Now, it always does xl, so, complete the same stuff for it as we have
> for xl.
> ---
>  tools/xl/bash-completion | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/xl/bash-completion b/tools/xl/bash-completion
> index 1db0bdc48e..022119a252 100644
> --- a/tools/xl/bash-completion
> +++ b/tools/xl/bash-completion
> @@ -212,6 +212,6 @@ _xl()
>          fi
>      fi
>  } &&
> -complete -F _xl xl
> +complete -F _xl xl xen
This apparently does not do the job completely. I'm testing things and
what I see is that:

1. xl command does give completion
2. xen command does not, until I trigger it one time for xl, and then
afterwards it does.

What I see in some other files is that the complete line at the end has
both commands, like xl xen above, but there's a symlink for the second
command back to the same script:

-# ll /usr/share/bash-completion/completions/yp*
lrwxrwxrwx 1 root root   7 Dec 22 01:34
/usr/share/bash-completion/completions/ypcat -> ypmatch
-rw-r--r-- 1 root root 768 Dec 22 01:34
/usr/share/bash-completion/completions/ypmatch

Is this the right way to solve this problem?
>  
>  # ex: filetype=sh
> 
K

Hans van Kranenburg writes ("[PATCH 06/13] sysconfig.xencommons.in: Strip
and debianize"):
> 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.
> 
> In order to configure this, XENSTORED in here needs to contain the name
> without a path, which is different than how the upstream file suggests
> to use it.
> 
> However, we don't want to put a version-specific path in here (the user
> keeps the config file on upgrades if it's changed, and then will end up
> with a path to a non-existing binary). Also, alternatively,
> /usr/sbin/oxenstored and /usr/sbin/xenstored could be used, but those
> are symlinks to xen-utils-wrapper and not the actual binary, and I think
> the test using start-stop-daemon to see if it's already running would
> fail to detect it's the same as e.g. /usr/lib/xen-4.11/bin/oxenstored.
> At least, it all sounds very fragile. :|
Thanks.  This LGTM (for the Debian package).

Acked-by: Ian Jackson <ijackson at chiark.greenend.org.uk>
^ for Debian, not upstream
> -## Default: Not defined, xenbackendd debug mode off
> -#
> -# Running xenbackendd in debug mode
> -#XENBACKENDD_DEBUG=[yes|on|1]
We might want this if we ship the driver domains init script too.

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.

Hans van Kranenburg writes ("[PATCH 07/13] d/xen-utils-common.install: ship
/etc/default/xen"):
> This file has been in the package before, it contained TOOLSTACK= to
> switch between xm and xl. It was abandoned and never cleaned up, so old
> installs still have this file.
Acked-by: Ian Jackson <ijackson at chiark.greenend.org.uk>

Hans van Kranenburg writes ("[PATCH 08/13] xen init script: rewrite
xenstored start logic"):
> -XENSTORED="$ROOT"/bin/xenstored
> +# In /etc/default/xen, the user can set XENSTORED, which has to be either
> +# 'xenstored' or 'oxenstored'. In here, we add the version
specific path.
> +if [ -n "$XENSTORED" ]; then
> +	USER_XENSTORED="$ROOT"/bin/$XENSTORED
IMO it would be better to do this only if XENSTORED does not start
with /.  So maybe

   case "$XENSTORED" in
   /*) USER_XENSTORED="$XENSTORED" ;;
   ?*) USER_XENSTORED="$ROOT"/bin/$XENSTORED ;;
   '') USER_XENSTORED="" ;;
   esac

?
> +	# First, we check if any of xenstored or oxenstored is already running.
If
> +	# so, we abort and leave it alone, even if the user has switched the
value
> +	# in /etc/default/xen from one to another.
> +	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
> +			if [ $? -eq 1 ]; then
> +				return 1
This exit status handling is confusing.  AFAICT from the manpage for
start-stop-daemon, exit status 1 means "--oknodo was not specified and
nothing was done".  So I think that would occur when --start does
nothing because the daemon is already running.  It might be worth
adding a comment.
> +	if [ -n "$USER_XENSTORED" ]; then
> +		if [ ! -x "$USER_XENSTORED" ]; then
> +			log_failure_msg "Failed to start $USER_XENSTORED: no such
executable."
> +			return 2
> +		else
> +			start-stop-daemon --start --quiet \
> +				--pidfile "$XENSTORED_PIDFILE" --exec
"$USER_XENSTORED" -- \
> +				$XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE"
> +			if [ $? -ne 0 ]; then
> +				return 2
> +			fi
> +		fi
> +	else
> +		for try_xenstored in "$OXENSTORED" "$CXENSTORED"; do
> +			if [ -x $try_xenstored ]; then
> +				start-stop-daemon --start --quiet \
> +					--pidfile "$XENSTORED_PIDFILE" --exec
"$try_xenstored" -- \
> +					$XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE"
I really don't like the way all of this is open-coded.  There are two
almost-identical runes here.  How about this:

   case "$XENSTORED" in
   /*) USER_XENSTORED="$XENSTORED"           ;
try_xenstoreds=USER_XENSTORED ;;
   ?*) USER_XENSTORED="$ROOT"/bin/$XENSTORED ;
try_xenstoreds=USER_XENSTORED ;;
   '') USER_XENSTORED="" ; try_xenstoreds='OXENSTORED
CXENSTORED' ;;
   esac

and then

  for try_xenstored_var in $try_xenstoreds; do
    eval "try_xenstored=\$$try_xenstored_var"
    ...

Thanks,
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.

Hans van Kranenburg writes ("[PATCH 09/13] xen init script: move init_dom0
into xenstored start"):
> This little xen-init-dom0 program is present in both our 4.8 and 4.11
> packages, so there's no need to keep the if else code. Also, it only
> makes sense to run it after starting xenstored, and not all other times
> start is called.
I think xen-init-dom0 is not in jessie and although skip-upgrades are
not "supported" I don't see a reason to gratuitously break them.
Would you mind if we deferred removing this compat code until after
buster ?
> @@ -307,11 +299,6 @@ case "$1" in
>  		0|1) ;;
>  		*) log_end_msg 1; exit ;;
>  	esac
> -	init_dom0
> -	case "$?" in
> -		0|1) ;;
> -		*) log_end_msg 1; exit ;;
> -	esac
It would have been less confusing to me if you had separated out the
change to remove the open-coded fallback, from the change to when this
is called.

Also the latter change seems right and here I am complaining about
the former...

Thanks,
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.

Hans van Kranenburg writes ("[PATCH 10/13] tools/xl/bash-completion:
improved completion"):
> While requesting removal of xm command line completion in the Debian
> bash-completion package, I ran into a bts about completion for xl:
> 
>   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=768005
> 
> I like the proposed improved version that Adi Kriegisch provides in
> there. It makes sure  we're back on the same advanced level of
> completion as in the past, with xm.
> 
> This change should be sent upstream asap, so that we can drop this patch
> again. However don't wait for that to happen and ship it already.
I have no opinion about this.  I don't use or understand
bash-completion.  I find that the various completion rules which are
provided by vaous packages are too unreliable and annoying.  So I
don't midn if you do this :-).

Do send the patch upstream after we haqve released 4.12.

Thanks,
Ian.

Hans van Kranenburg writes ("[PATCH 13/13] d/[..]/grub.d/xen.cfg: improve
docs even more"):
> Ok, I finally tried this, and it's really great that I now have correct
> (different) linux kernel options generated for just linux and xen+dom0.
> 
> Do more cosmetics and reorganizing to make this wall of comments better
> parseable by the eye. It was not obvious to me where sections started
> and ended, and I like to have first the explanation and then at the end
> the actual variables you can set.
Thanks, I like this attention to docs.

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