[PATCH v8 0/9] qemu-binfmt-conf.sh

[PATCH v8 0/9] qemu-binfmt-conf.sh

[unai.martinezcorral]

Hi,

This series reworks qemu-binfmt-conf.sh:

* Argument <CPU> from option '--systemd' is generalized to <TARGETS>, and it is
  accepted for any mode (default, debian or systemd). It can be a single target
  arch or a list of them.
* Option '-r|--clear' is added, which allows to remove an already registered
  target interpreter or a list of them. The implementation is functional but
  partial. Please, see the corresponding commit.
* Option '-t|--test' is added, which allows to execute the CHECK according
  to the provided arguments, but no interpreter is configured.
* Support to set options through environment variables: QEMU_TARGETS, QEMU_PATH,
  QEMU_SUFFIX, QEMU_PERSISTENT, QEMU_CREDENTIAL, QEMU_CLEAR and QEMU_TEST.

The following changes are not backward compatible:

* Option '--persistent' no longer requires/accepts an argument.
* Option '--credential' no longer requires/accepts an argument.
* Option '--systemd' no longer requires/accepts an argument.
* Option '--qemu-path' is renamed to '--path'.
* Option '--qemu-suffix' is renamed to '--suffix'.

The functionality of all of them is untouched. Changes are related to syntax only.

v8:

* Fix typos and address review comments

v7:

* Check that a interpreter exists before trying to remove it.

v6:

* Don't need to use "find".
* Put the ";;" at the same position.
* Set BINFMT_CLEAR to ':', to allow --test to work with --clear.
* Do not show DEBIANDIR and SYSTEMDDIR in the Env-variable column.

 Based on:

* [PATCH v5 0/10] qemu-binfmt-conf.sh
   * should have been [PATCH v5 0/9] qemu-binfmt-conf.sh
* [PATCH v4 0/10] qemu-binfmt-conf.sh
* [PATCH v3 0/10] qemu-binfmt-conf.sh
* [PATCH v2] qemu-binfmt-conf.sh: add CPUS, add --reset, make -p and -c boolean (no arg)
* [PATCH] qemu-binfmt-conf.sh: add CPUS, add --reset, make -p and -c boolean (no arg)
* scripts/qemu-binfmt-conf.sh: allow clearing of entries

Regards

Unai Martinez-Corral (9):
      qemu-binfmt-conf.sh: enforce safe style consistency
      qemu-binfmt-conf.sh: make opts -p and -c boolean
      qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
      qemu-binfmt-conf.sh: use the same presentation format as for qemu-*
      qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
      qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
      qemu-binfmt-conf.sh: generalize <CPU> to positional TARGETS
      qemu-binfmt-conf.sh: add option --clear
      qemu-binfmt-conf.sh: add --test

scripts/qemu-binfmt-conf.sh | 211
1 file changed, 133 insertions(+), 78 deletions(-)

[PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency

[Unai Martinez-Corral]

Spaces are added before '; then', for consistency.

All the tests are prefixed with 'x', in order to avoid risky comparisons
(i.e. a user deliberately trying to provoke a syntax error).

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 9f1580a91c..672ce716b6 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -223,12 +223,12 @@ qemu_check_access() {
 
 qemu_check_bintfmt_misc() {
     # load the binfmt_misc module
-    if [ ! -d /proc/sys/fs/binfmt_misc ]; then
+    if [ ! -d /proc/sys/fs/binfmt_misc ] ; then
       if ! /sbin/modprobe binfmt_misc ; then
           exit 1
       fi
     fi
-    if [ ! -f /proc/sys/fs/binfmt_misc/register ]; then
+    if [ ! -f /proc/sys/fs/binfmt_misc/register ] ; then
       if ! mount binfmt_misc -t binfmt_misc /proc/sys/fs/binfmt_misc ; then
           exit 1
       fi
@@ -259,10 +259,10 @@ qemu_check_systemd() {
 
 qemu_generate_register() {
     flags=""
-    if [ "$CREDENTIAL" = "yes" ] ; then
+    if [ "x$CREDENTIAL" = "xyes" ] ; then
         flags="OC"
     fi
-    if [ "$PERSISTENT" = "yes" ] ; then
+    if [ "x$PERSISTENT" = "xyes" ] ; then
         flags="${flags}F"
     fi
 
@@ -300,18 +300,18 @@ qemu_set_binfmts() {
         mask=$(eval echo \$${cpu}_mask)
         family=$(eval echo \$${cpu}_family)
 
-        if [ "$magic" = "" ] || [ "$mask" = "" ] || [ "$family" = "" ] ; then
+        if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ] ; then
             echo "INTERNAL ERROR: unknown cpu $cpu" 1>&2
             continue
         fi
 
         qemu="$QEMU_PATH/qemu-$cpu"
-        if [ "$cpu" = "i486" ] ; then
+        if [ "x$cpu" = "xi486" ] ; then
             qemu="$QEMU_PATH/qemu-i386"
         fi
 
         qemu="$qemu$QEMU_SUFFIX"
-        if [ "$host_family" != "$family" ] ; then
+        if [ "x$host_family" != "x$family" ] ; then
             $BINFMT_SET
         fi
     done
-- 
2.25.1

[PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency

[Unai Martinez-Corral]

Spaces are added before '; then', for consistency.

All the tests are prefixed with 'x', in order to avoid risky comparisons
(i.e. a user deliberately trying to provoke a syntax error).

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 9f1580a91c..672ce716b6 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -223,12 +223,12 @@ qemu_check_access() {
 
 qemu_check_bintfmt_misc() {
     # load the binfmt_misc module
-    if [ ! -d /proc/sys/fs/binfmt_misc ]; then
+    if [ ! -d /proc/sys/fs/binfmt_misc ] ; then
       if ! /sbin/modprobe binfmt_misc ; then
           exit 1
       fi
     fi
-    if [ ! -f /proc/sys/fs/binfmt_misc/register ]; then
+    if [ ! -f /proc/sys/fs/binfmt_misc/register ] ; then
       if ! mount binfmt_misc -t binfmt_misc /proc/sys/fs/binfmt_misc ; then
           exit 1
       fi
@@ -259,10 +259,10 @@ qemu_check_systemd() {
 
 qemu_generate_register() {
     flags=""
-    if [ "$CREDENTIAL" = "yes" ] ; then
+    if [ "x$CREDENTIAL" = "xyes" ] ; then
         flags="OC"
     fi
-    if [ "$PERSISTENT" = "yes" ] ; then
+    if [ "x$PERSISTENT" = "xyes" ] ; then
         flags="${flags}F"
     fi
 
@@ -300,18 +300,18 @@ qemu_set_binfmts() {
         mask=$(eval echo \$${cpu}_mask)
         family=$(eval echo \$${cpu}_family)
 
-        if [ "$magic" = "" ] || [ "$mask" = "" ] || [ "$family" = "" ] ; then
+        if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ] ; then
             echo "INTERNAL ERROR: unknown cpu $cpu" 1>&2
             continue
         fi
 
         qemu="$QEMU_PATH/qemu-$cpu"
-        if [ "$cpu" = "i486" ] ; then
+        if [ "x$cpu" = "xi486" ] ; then
             qemu="$QEMU_PATH/qemu-i386"
         fi
 
         qemu="$qemu$QEMU_SUFFIX"
-        if [ "$host_family" != "$family" ] ; then
+        if [ "x$host_family" != "x$family" ] ; then
             $BINFMT_SET
         fi
     done
-- 
2.25.1

[PATCH v8 2/9] qemu-binfmt-conf.sh: make opts -p and -c boolean

[Unai Martinez-Corral]

This patch breaks backward compatibility.

Both '--persistent' and '--credential' default to 'no'. Hence, '-p no'
or '-c no' are redundant. Overall, accepting an argument might be
misleading because options are, indeed, boolean. This patch makes both
options boolean in getopt, so if any of them is provided the corresponding
variable is set to true.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 672ce716b6..75eb19c3bf 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -172,8 +172,8 @@ qemu_get_family() {
 usage() {
     cat <<EOF
 Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
-                           [--help][--credential yes|no][--exportdir PATH]
-                           [--persistent yes|no][--qemu-suffix SUFFIX]
+                           [--help][--credential][--exportdir PATH]
+                           [--persistent][--qemu-suffix SUFFIX]
 
        Configure binfmt_misc to use qemu interpreter
 
@@ -188,9 +188,9 @@ Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
                       file for all known cpus
        --exportdir:   define where to write configuration files
                       (default: $SYSTEMDDIR or $DEBIANDIR)
-       --credential:  if yes, credential and security tokens are
+       --credential:  if present, credential and security tokens are
                       calculated according to the binary to interpret
-       --persistent:  if yes, the interpreter is loaded when binfmt is
+       --persistent:  if present, the interpreter is loaded when binfmt is
                       configured and remains in memory. All future uses
                       are cloned from the open file.
 
@@ -328,7 +328,7 @@ CREDENTIAL=no
 PERSISTENT=no
 QEMU_SUFFIX=""
 
-options=$(getopt -o ds:Q:S:e:hc:p: -l debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential:,persistent: -- "$@")
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
 
 while true ; do
@@ -377,12 +377,10 @@ while true ; do
         exit 1
         ;;
     -c|--credential)
-        shift
-        CREDENTIAL="$1"
+        CREDENTIAL="yes"
         ;;
     -p|--persistent)
-        shift
-        PERSISTENT="$1"
+        PERSISTENT="yes"
         ;;
     *)
         break
-- 
2.25.1

[PATCH v8 3/9] qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT

[Unai Martinez-Corral]

Allow to set options '--persistent' and/or '--credential' through
environment variables. If not defined, defaults are used ('no').
Anyway, command-line arguments have priority over environment variables.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 75eb19c3bf..347cddf698 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -190,9 +190,11 @@ Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
                       (default: $SYSTEMDDIR or $DEBIANDIR)
        --credential:  if present, credential and security tokens are
                       calculated according to the binary to interpret
+                      (QEMU_CREDENTIAL=yes)
        --persistent:  if present, the interpreter is loaded when binfmt is
                       configured and remains in memory. All future uses
                       are cloned from the open file.
+                      (QEMU_PERSISTENT=yes)
 
     To import templates with update-binfmts, use :
 
@@ -259,10 +261,10 @@ qemu_check_systemd() {
 
 qemu_generate_register() {
     flags=""
-    if [ "x$CREDENTIAL" = "xyes" ] ; then
+    if [ "x$QEMU_CREDENTIAL" = "xyes" ] ; then
         flags="OC"
     fi
-    if [ "x$PERSISTENT" = "xyes" ] ; then
+    if [ "x$QEMU_PERSISTENT" = "xyes" ] ; then
         flags="${flags}F"
     fi
 
@@ -285,7 +287,7 @@ package qemu-$cpu
 interpreter $qemu
 magic $magic
 mask $mask
-credential $CREDENTIAL
+credential $QEMU_CREDENTIAL
 EOF
 }
 
@@ -324,8 +326,10 @@ SYSTEMDDIR="/etc/binfmt.d"
 DEBIANDIR="/usr/share/binfmts"
 
 QEMU_PATH=/usr/local/bin
-CREDENTIAL=no
-PERSISTENT=no
+
+QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
+QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
+
 QEMU_SUFFIX=""
 
 options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent -- "$@")
@@ -377,10 +381,10 @@ while true ; do
         exit 1
         ;;
     -c|--credential)
-        CREDENTIAL="yes"
+        QEMU_CREDENTIAL="yes"
         ;;
     -p|--persistent)
-        PERSISTENT="yes"
+        QEMU_PERSISTENT="yes"
         ;;
     *)
         break
-- 
2.25.1

[PATCH v8 4/9] qemu-binfmt-conf.sh: use the same presentation format as for qemu-*

[Unai Martinez-Corral]

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 63 +++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 347cddf698..251a78a2ce 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -171,47 +171,48 @@ qemu_get_family() {
 
 usage() {
     cat <<EOF
-Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
-                           [--help][--credential][--exportdir PATH]
-                           [--persistent][--qemu-suffix SUFFIX]
+Usage: qemu-binfmt-conf.sh [options]
 
-       Configure binfmt_misc to use qemu interpreter
+Configure binfmt_misc to use qemu interpreter
 
-       --help:        display this usage
-       --qemu-path:   set path to qemu interpreter ($QEMU_PATH)
-       --qemu-suffix: add a suffix to the default interpreter name
-       --debian:      don't write into /proc,
-                      instead generate update-binfmts templates
-       --systemd:     don't write into /proc,
-                      instead generate file for systemd-binfmt.service
-                      for the given CPU. If CPU is "ALL", generate a
-                      file for all known cpus
-       --exportdir:   define where to write configuration files
-                      (default: $SYSTEMDDIR or $DEBIANDIR)
-       --credential:  if present, credential and security tokens are
-                      calculated according to the binary to interpret
-                      (QEMU_CREDENTIAL=yes)
-       --persistent:  if present, the interpreter is loaded when binfmt is
-                      configured and remains in memory. All future uses
-                      are cloned from the open file.
-                      (QEMU_PERSISTENT=yes)
+Options and associated environment variables:
 
-    To import templates with update-binfmts, use :
+Argument             Env-variable     Description
+-h|--help                             display this usage
+-Q|--qemu-path PATH  QEMU_PATH        set path to qemu interpreter
+-F|--qemu-suffix SUFFIX               add a suffix to the default interpreter name
+-p|--persistent      QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
+                                      uses are cloned from the open file.
+-c|--credential      QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
+                                      to the binary to interpret
+-e|--exportdir PATH                   define where to write configuration files
+                                      (default: $SYSTEMDDIR or $DEBIANDIR)
+-s|--systemd CPU                      don't write into /proc, generate file for
+                                      systemd-binfmt.service for the given CPU; if CPU is "ALL",
+                                      generate a file for all known cpus.
+-d|--debian                           don't write into /proc, generate update-binfmts templates
 
-        sudo update-binfmts --importdir ${EXPORTDIR:-$DEBIANDIR} --import qemu-CPU
+Defaults:
+QEMU_PATH=$QEMU_PATH
+QEMU_PERSISTENT=$QEMU_PERSISTENT
+QEMU_CREDENTIAL=$QEMU_CREDENTIAL
 
-    To remove interpreter, use :
+To import templates with update-binfmts, use :
 
-        sudo update-binfmts --package qemu-CPU --remove qemu-CPU $QEMU_PATH
+    sudo update-binfmts --importdir ${EXPORTDIR:-$DEBIANDIR} --import qemu-CPU
 
-    With systemd, binfmt files are loaded by systemd-binfmt.service
+To remove interpreter, use :
 
-    The environment variable HOST_ARCH allows to override 'uname' to generate
-    configuration files for a different architecture than the current one.
+    sudo update-binfmts --package qemu-CPU --remove qemu-CPU $QEMU_PATH
 
-    where CPU is one of:
+With systemd, binfmt files are loaded by systemd-binfmt.service
 
-        $qemu_target_list
+The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for a
+different architecture than the current one.
+
+where CPU is one of:
+
+    $qemu_target_list
 
 EOF
 }
-- 
2.25.1

[PATCH v8 5/9] qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options

[Unai Martinez-Corral]

This breaks backward compatibility.

Options 'qemu-path' and 'qemu-suffix' have the 'qemu-' prefix, which is
not present in other option names ('debian', 'systemd', 'persistent',
'credential'...). In order to keep consistency, the prefix is removed.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 251a78a2ce..2a035394e0 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -171,6 +171,7 @@ qemu_get_family() {
 
 usage() {
     cat <<EOF
+
 Usage: qemu-binfmt-conf.sh [options]
 
 Configure binfmt_misc to use qemu interpreter
@@ -179,8 +180,8 @@ Options and associated environment variables:
 
 Argument             Env-variable     Description
 -h|--help                             display this usage
--Q|--qemu-path PATH  QEMU_PATH        set path to qemu interpreter
--F|--qemu-suffix SUFFIX               add a suffix to the default interpreter name
+-Q|--path PATH       QEMU_PATH        set path to qemu interpreter(s)
+-F|--suffix SUFFIX                    add a suffix to the default interpreter name
 -p|--persistent      QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
                                       uses are cloned from the open file.
 -c|--credential      QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
@@ -333,7 +334,7 @@ QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 
 QEMU_SUFFIX=""
 
-options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent -- "$@")
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
 
 while true ; do
@@ -365,11 +366,11 @@ while true ; do
             fi
         fi
         ;;
-    -Q|--qemu-path)
+    -Q|--path)
         shift
         QEMU_PATH="$1"
         ;;
-    -F|--qemu-suffix)
+    -F|--suffix)
         shift
         QEMU_SUFFIX="$1"
         ;;
-- 
2.25.1

[PATCH v8 6/9] qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX

[Unai Martinez-Corral]

Allow to set 'path' or 'suffix' through environment variables,
consistently with 'persistent' and 'credential'.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 2a035394e0..80ec164eab 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -181,7 +181,7 @@ Options and associated environment variables:
 Argument             Env-variable     Description
 -h|--help                             display this usage
 -Q|--path PATH       QEMU_PATH        set path to qemu interpreter(s)
--F|--suffix SUFFIX                    add a suffix to the default interpreter name
+-F|--suffix SUFFIX   QEMU_SUFFIX      add a suffix to the default interpreter name
 -p|--persistent      QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
                                       uses are cloned from the open file.
 -c|--credential      QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
@@ -195,6 +195,7 @@ Argument             Env-variable     Description
 
 Defaults:
 QEMU_PATH=$QEMU_PATH
+QEMU_SUFFIX=$QEMU_SUFFIX
 QEMU_PERSISTENT=$QEMU_PERSISTENT
 QEMU_CREDENTIAL=$QEMU_CREDENTIAL
 
@@ -327,13 +328,11 @@ BINFMT_SET=qemu_register_interpreter
 SYSTEMDDIR="/etc/binfmt.d"
 DEBIANDIR="/usr/share/binfmts"
 
-QEMU_PATH=/usr/local/bin
-
+QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
+QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 
-QEMU_SUFFIX=""
-
 options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
 
-- 
2.25.1

[PATCH v8 7/9] qemu-binfmt-conf.sh: generalize <CPU> to positional [TARGETS]

[Unai Martinez-Corral]

This breaks brackward compatibility.

Option '--systemd CPU' allows to register binfmt interpreters for a
single target architecture or for 'ALL' (of them). This patch
generalizes the approach to support it in any mode (default, '--debian'
or '--systemd'). To do so, option 'systemd' is changed to be boolean
(no args). Then, all the positional arguments are considered to be a
list of target architectures.

If no positional arguments are provided, all of the architectures in
qemu_target_list are registered. Conversely, argument value 'NONE'
allows to make a 'dry run' of the script. I.e., checks are executed
according to the mode, but no interpreter is registered.

Support QEMU_TARGETS environment variable, consistently with 'path',
'suffix', 'persistent' and 'credential', The supported formats are
the same as for positional arguments, which have priority. If both
the variable and the list of positional arguments are empty, defaults
to qemu_target_list.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 80 +++++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 80ec164eab..d5d3484b58 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -6,6 +6,28 @@ ppc ppc64 ppc64le m68k mips mipsel mipsn32 mipsn32el mips64 mips64el \
 sh4 sh4eb s390x aarch64 aarch64_be hppa riscv32 riscv64 xtensa xtensaeb \
 microblaze microblazeel or1k x86_64"
 
+# check if given TARGETS is/are in the supported target list
+qemu_check_target_list() {
+    if [ $# -eq 0 ] ; then
+      checked_target_list="$qemu_target_list"
+      return
+    fi
+    unset checked_target_list
+    for target ; do
+        for cpu in $qemu_target_list ; do
+            if [ "x$cpu" = "x$target" ] ; then
+                checked_target_list="$checked_target_list $target"
+                break
+            fi
+        done
+        if [ "x$cpu" != "x$target" ] ; then
+            echo "ERROR: unknown CPU \"$target\"" 1>&2
+            usage
+            exit 1
+        fi
+    done
+}
+
 i386_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00'
 i386_mask='\xff\xff\xff\xff\xff\xfe\xfe\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'
 i386_family=i386
@@ -171,14 +193,16 @@ qemu_get_family() {
 
 usage() {
     cat <<EOF
+Usage: qemu-binfmt-conf.sh [options] [TARGETS]
 
-Usage: qemu-binfmt-conf.sh [options]
-
-Configure binfmt_misc to use qemu interpreter
+Configure binfmt_misc to use qemu interpreter for the given TARGETS.
 
 Options and associated environment variables:
 
 Argument             Env-variable     Description
+TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
+                                      if empty, configure all known targets;
+                                      if 'NONE', no interpreter is configured.
 -h|--help                             display this usage
 -Q|--path PATH       QEMU_PATH        set path to qemu interpreter(s)
 -F|--suffix SUFFIX   QEMU_SUFFIX      add a suffix to the default interpreter name
@@ -188,12 +212,12 @@ Argument             Env-variable     Description
                                       to the binary to interpret
 -e|--exportdir PATH                   define where to write configuration files
                                       (default: $SYSTEMDDIR or $DEBIANDIR)
--s|--systemd CPU                      don't write into /proc, generate file for
-                                      systemd-binfmt.service for the given CPU; if CPU is "ALL",
-                                      generate a file for all known cpus.
+-s|--systemd                          don't write into /proc, generate file(s) for
+                                      systemd-binfmt.service;
 -d|--debian                           don't write into /proc, generate update-binfmts templates
 
 Defaults:
+QEMU_TARGETS=$QEMU_TARGETS
 QEMU_PATH=$QEMU_PATH
 QEMU_SUFFIX=$QEMU_SUFFIX
 QEMU_PERSISTENT=$QEMU_PERSISTENT
@@ -207,14 +231,10 @@ To remove interpreter, use :
 
     sudo update-binfmts --package qemu-CPU --remove qemu-CPU $QEMU_PATH
 
-With systemd, binfmt files are loaded by systemd-binfmt.service
+The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for
+a different architecture than the current one.
 
-The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for a
-different architecture than the current one.
-
-where CPU is one of:
-
-    $qemu_target_list
+QEMU target list: $qemu_target_list
 
 EOF
 }
@@ -298,9 +318,15 @@ qemu_set_binfmts() {
     # probe cpu type
     host_family=$(qemu_get_family)
 
-    # register the interpreter for each cpu except for the native one
+    # reduce the list of target interpreters to those given in the CLI
+    targets=${@:-$QEMU_TARGET}
+    if [ "x$targets" = "xNONE" ] ; then
+      return
+    fi
+    qemu_check_target_list $targets
 
-    for cpu in ${qemu_target_list} ; do
+    # register the interpreter for each target except for the native one
+    for cpu in $checked_target_list ; do
         magic=$(eval echo \$${cpu}_magic)
         mask=$(eval echo \$${cpu}_mask)
         family=$(eval echo \$${cpu}_family)
@@ -328,12 +354,13 @@ BINFMT_SET=qemu_register_interpreter
 SYSTEMDDIR="/etc/binfmt.d"
 DEBIANDIR="/usr/share/binfmts"
 
+QEMU_TARGETS="${QEMU_TARGETS:-}"
 QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
 QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 
-options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
+options=$(getopt -o dsQ:S:e:hcp -l debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
 
 while true ; do
@@ -347,23 +374,6 @@ while true ; do
         CHECK=qemu_check_systemd
         BINFMT_SET=qemu_generate_systemd
         EXPORTDIR=${EXPORTDIR:-$SYSTEMDDIR}
-        shift
-        # check given cpu is in the supported CPU list
-        if [ "$1" != "ALL" ] ; then
-            for cpu in ${qemu_target_list} ; do
-                if [ "$cpu" = "$1" ] ; then
-                    break
-                fi
-            done
-
-            if [ "$cpu" = "$1" ] ; then
-                qemu_target_list="$1"
-            else
-                echo "ERROR: unknown CPU \"$1\"" 1>&2
-                usage
-                exit 1
-            fi
-        fi
         ;;
     -Q|--path)
         shift
@@ -394,5 +404,7 @@ while true ; do
     shift
 done
 
+shift
+
 $CHECK
-qemu_set_binfmts
+qemu_set_binfmts "$@"
-- 
2.25.1

[PATCH v8 8/9] qemu-binfmt-conf.sh: add option --clear

[Unai Martinez-Corral]

This is a partial implementation.

Allows to remove a single or a list of already registered binfmt
interpreters. Valid values are those in qemu_target_list.
If TARGETS is empty, all the existing 'qemu-*' interpreters are
removed.

This is partial because 'debian' and 'systemd' configurations are not
supported. The script will exit with error 'option clear not
implemented for this mode yet'.

Removal is done by printing '-1' as explained at:
https://www.kernel.org/doc/Documentation/admin-guide/binfmt-misc.rst

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index d5d3484b58..9685456747 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -201,7 +201,7 @@ Options and associated environment variables:
 
 Argument             Env-variable     Description
 TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
-                                      if empty, configure all known targets;
+                                      if empty, configure/clear all known targets;
                                       if 'NONE', no interpreter is configured.
 -h|--help                             display this usage
 -Q|--path PATH       QEMU_PATH        set path to qemu interpreter(s)
@@ -210,6 +210,8 @@ TARGETS              QEMU_TARGETS     A single arch name or a list of them (see
                                       uses are cloned from the open file.
 -c|--credential      QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
                                       to the binary to interpret
+-r|--clear           QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;
+                                      then exit.
 -e|--exportdir PATH                   define where to write configuration files
                                       (default: $SYSTEMDDIR or $DEBIANDIR)
 -s|--systemd                          don't write into /proc, generate file(s) for
@@ -222,6 +224,7 @@ QEMU_PATH=$QEMU_PATH
 QEMU_SUFFIX=$QEMU_SUFFIX
 QEMU_PERSISTENT=$QEMU_PERSISTENT
 QEMU_CREDENTIAL=$QEMU_CREDENTIAL
+QEMU_CLEAR=$QEMU_CLEAR
 
 To import templates with update-binfmts, use :
 
@@ -348,8 +351,22 @@ qemu_set_binfmts() {
     done
 }
 
+qemu_clear_notimplemented() {
+    echo "ERROR: option clear not implemented for this mode yet" 1>&2
+    usage
+    exit 1
+}
+
+qemu_clear_interpreter() {
+    p="/proc/sys/fs/binfmt_misc/$1"
+    if [ -f "$p" ] ; then
+      printf %s -1 > "$p"
+    fi
+}
+
 CHECK=qemu_check_bintfmt_misc
 BINFMT_SET=qemu_register_interpreter
+BINFMT_CLEAR=qemu_clear_interpreter
 
 SYSTEMDDIR="/etc/binfmt.d"
 DEBIANDIR="/usr/share/binfmts"
@@ -359,20 +376,26 @@ QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
 QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
+QEMU_CLEAR="${QEMU_CLEAR:-no}"
 
-options=$(getopt -o dsQ:S:e:hcp -l debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
+options=$(getopt -o rdsQ:S:e:hcp -l clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
 
 while true ; do
     case "$1" in
+    -r|--clear)
+        QEMU_CLEAR="yes"
+        ;;
     -d|--debian)
         CHECK=qemu_check_debian
         BINFMT_SET=qemu_generate_debian
+        BINFMT_CLEAR=qemu_clear_notimplemented
         EXPORTDIR=${EXPORTDIR:-$DEBIANDIR}
         ;;
     -s|--systemd)
         CHECK=qemu_check_systemd
         BINFMT_SET=qemu_generate_systemd
+        BINFMT_CLEAR=qemu_clear_notimplemented
         EXPORTDIR=${EXPORTDIR:-$SYSTEMDDIR}
         ;;
     -Q|--path)
@@ -407,4 +430,13 @@ done
 shift
 
 $CHECK
+
+if [ "x$QEMU_CLEAR" = "xyes" ] ; then
+    qemu_check_target_list "$@"
+    for t in $checked_target_list ; do
+        $BINFMT_CLEAR "qemu-$t"
+    done
+    exit
+fi
+
 qemu_set_binfmts "$@"
-- 
2.25.1

[PATCH v8 9/9] qemu-binfmt-conf.sh: add --test

[Unai Martinez-Corral]

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 9685456747..4635871e6d 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -201,8 +201,7 @@ Options and associated environment variables:
 
 Argument             Env-variable     Description
 TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
-                                      if empty, configure/clear all known targets;
-                                      if 'NONE', no interpreter is configured.
+                                      if empty, configure/clear all known targets.
 -h|--help                             display this usage
 -Q|--path PATH       QEMU_PATH        set path to qemu interpreter(s)
 -F|--suffix SUFFIX   QEMU_SUFFIX      add a suffix to the default interpreter name
@@ -212,6 +211,8 @@ TARGETS              QEMU_TARGETS     A single arch name or a list of them (see
                                       to the binary to interpret
 -r|--clear           QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;
                                       then exit.
+-t|--test            QEMU_TEST        (yes) test the setup with the provided arguments, but do not
+                                      configure any of the interpreters.
 -e|--exportdir PATH                   define where to write configuration files
                                       (default: $SYSTEMDDIR or $DEBIANDIR)
 -s|--systemd                          don't write into /proc, generate file(s) for
@@ -225,6 +226,7 @@ QEMU_SUFFIX=$QEMU_SUFFIX
 QEMU_PERSISTENT=$QEMU_PERSISTENT
 QEMU_CREDENTIAL=$QEMU_CREDENTIAL
 QEMU_CLEAR=$QEMU_CLEAR
+QEMU_TEST=$QEMU_TEST
 
 To import templates with update-binfmts, use :
 
@@ -323,9 +325,6 @@ qemu_set_binfmts() {
 
     # reduce the list of target interpreters to those given in the CLI
     targets=${@:-$QEMU_TARGET}
-    if [ "x$targets" = "xNONE" ] ; then
-      return
-    fi
     qemu_check_target_list $targets
 
     # register the interpreter for each target except for the native one
@@ -377,12 +376,16 @@ QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_CLEAR="${QEMU_CLEAR:-no}"
+QEMU_TEST="${QEMU_TEST:-no}"
 
-options=$(getopt -o rdsQ:S:e:hcp -l clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
+options=$(getopt -o trdsQ:S:e:hcp -l test,clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
 
 while true ; do
     case "$1" in
+    -t|--test)
+        QEMU_TEST="yes"
+        ;;
     -r|--clear)
         QEMU_CLEAR="yes"
         ;;
@@ -431,6 +434,11 @@ shift
 
 $CHECK
 
+if [ "x$QEMU_TEST" = "xyes" ] ; then
+    BINFMT_SET=:
+    BINFMT_CLEAR=:
+fi
+
 if [ "x$QEMU_CLEAR" = "xyes" ] ; then
     qemu_check_target_list "$@"
     for t in $checked_target_list ; do
-- 
2.25.1

Re: [PATCH v8 0/9] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200307170251.GA7@dd5f6ec33fb0/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v8 0/9] qemu-binfmt-conf.sh
Message-id: 20200307170251.GA7@dd5f6ec33fb0
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7df5f33 qemu-binfmt-conf.sh: add --test
a58e556 qemu-binfmt-conf.sh: add option --clear
42db124 qemu-binfmt-conf.sh: generalize <CPU> to positional [TARGETS]
e5ea0d0 qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
633c743 qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
b68ad24 qemu-binfmt-conf.sh: use the same presentation format as for qemu-*
38b00f7 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
cc882cd qemu-binfmt-conf.sh: make opts -p and -c boolean
183e4d5 qemu-binfmt-conf.sh: enforce safe style consistency

=== OUTPUT BEGIN ===
1/9 Checking commit 183e4d5f9c6f (qemu-binfmt-conf.sh: enforce safe style consistency)
WARNING: line over 80 characters
#53: FILE: scripts/qemu-binfmt-conf.sh:303:
+        if [ "x$magic" = "x" ] || [ "x$mask" = "x" ] || [ "x$family" = "x" ] ; then

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/9 Checking commit cc882cde65df (qemu-binfmt-conf.sh: make opts -p and -c boolean)
ERROR: line over 90 characters
#51: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 43 lines checked

Patch 2/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/9 Checking commit 38b00f7dd75c (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT)
4/9 Checking commit b68ad248d5e0 (qemu-binfmt-conf.sh: use the same presentation format as for qemu-*)
WARNING: line over 80 characters
#51: FILE: scripts/qemu-binfmt-conf.sh:183:
+-F|--qemu-suffix SUFFIX               add a suffix to the default interpreter name

ERROR: line over 90 characters
#52: FILE: scripts/qemu-binfmt-conf.sh:184:
+-p|--persistent      QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future

ERROR: line over 90 characters
#54: FILE: scripts/qemu-binfmt-conf.sh:186:
+-c|--credential      QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according

ERROR: line over 90 characters
#59: FILE: scripts/qemu-binfmt-conf.sh:191:
+                                      systemd-binfmt.service for the given CPU; if CPU is "ALL",

ERROR: line over 90 characters
#61: FILE: scripts/qemu-binfmt-conf.sh:193:
+-d|--debian                           don't write into /proc, generate update-binfmts templates

ERROR: line over 90 characters
#86: FILE: scripts/qemu-binfmt-conf.sh:210:
+The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for a

total: 5 errors, 1 warnings, 79 lines checked

Patch 4/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/9 Checking commit 633c7439b752 (qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options)
WARNING: line over 80 characters
#36: FILE: scripts/qemu-binfmt-conf.sh:184:
+-F|--suffix SUFFIX                    add a suffix to the default interpreter name

ERROR: line over 90 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:337:
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 1 warnings, 38 lines checked

Patch 5/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/9 Checking commit e5ea0d0db84d (qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX)
WARNING: line over 80 characters
#23: FILE: scripts/qemu-binfmt-conf.sh:184:
+-F|--suffix SUFFIX   QEMU_SUFFIX      add a suffix to the default interpreter name

total: 0 errors, 1 warnings, 30 lines checked

Patch 6/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/9 Checking commit 42db124d284a (qemu-binfmt-conf.sh: generalize <CPU> to positional [TARGETS])
ERROR: line over 90 characters
#78: FILE: scripts/qemu-binfmt-conf.sh:203:
+TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);

WARNING: line over 80 characters
#91: FILE: scripts/qemu-binfmt-conf.sh:215:
+-s|--systemd                          don't write into /proc, generate file(s) for

ERROR: line over 90 characters
#105: FILE: scripts/qemu-binfmt-conf.sh:234:
+The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for

ERROR: line over 90 characters
#147: FILE: scripts/qemu-binfmt-conf.sh:363:
+options=$(getopt -o dsQ:S:e:hcp -l debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 3 errors, 1 warnings, 141 lines checked

Patch 7/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/9 Checking commit a58e5567f2be (qemu-binfmt-conf.sh: add option --clear)
WARNING: line over 80 characters
#34: FILE: scripts/qemu-binfmt-conf.sh:204:
+                                      if empty, configure/clear all known targets;

ERROR: line over 90 characters
#42: FILE: scripts/qemu-binfmt-conf.sh:213:
+-r|--clear           QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;

ERROR: line over 90 characters
#85: FILE: scripts/qemu-binfmt-conf.sh:381:
+options=$(getopt -o rdsQ:S:e:hcp -l clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 2 errors, 1 warnings, 85 lines checked

Patch 8/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

9/9 Checking commit 7df5f332f7ba (qemu-binfmt-conf.sh: add --test)
WARNING: line over 80 characters
#21: FILE: scripts/qemu-binfmt-conf.sh:204:
+                                      if empty, configure/clear all known targets.

ERROR: line over 90 characters
#29: FILE: scripts/qemu-binfmt-conf.sh:214:
+-t|--test            QEMU_TEST        (yes) test the setup with the provided arguments, but do not

ERROR: line over 90 characters
#59: FILE: scripts/qemu-binfmt-conf.sh:381:
+options=$(getopt -o trdsQ:S:e:hcp -l test,clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 2 errors, 1 warnings, 61 lines checked

Patch 9/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200307170251.GA7@dd5f6ec33fb0/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v8 0/9] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200307170251.GA7@dd5f6ec33fb0/



Hi,

This series failed the docker-clang@ubuntu build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-ubuntu V=1 NETWORK=1
time make docker-test-clang@ubuntu SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  LINK    fsdev/virtfs-proxy-helper
  LINK    scsi/qemu-pr-helper
  LINK    qemu-bridge-helper
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    virtiofsd
  LINK    vhost-user-input
  GEN     mips64-softmmu/hmp-commands.h
---
  CC      hppa-softmmu/hw/virtio/virtio-input-host-pci.o
  CC      alpha-softmmu/hw/virtio/vhost-user-scsi-pci.o
  CC      mips64-softmmu/hw/virtio/vhost-scsi-pci.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      aarch64-softmmu/hw/vfio/pci.o
  CC      mips-softmmu/hw/virtio/virtio-input-host-pci.o
  CC      alpha-softmmu/hw/virtio/vhost-scsi-pci.o
  CC      mips64-softmmu/hw/virtio/virtio-input-host-pci.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      i386-softmmu/hw/virtio/vhost-scsi-pci.o
  CC      arm-softmmu/hw/vfio/pci-quirks.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      aarch64-softmmu/hw/vfio/pci-quirks.o
  CC      hppa-softmmu/hw/virtio/virtio-input-pci.o
  CC      mipsel-softmmu/hw/virtio/virtio-input-pci.o
---
  CC      ppc-softmmu/cpus.o
  CC      or1k-softmmu/target/openrisc/exception.o
  CC      arm-softmmu/hw/arm/mcimx7d-sabre.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      nios2-softmmu/target/nios2/mmu.o
  CC      or1k-softmmu/target/openrisc/interrupt.o
  CC      ppc-softmmu/gdbstub.o
---
  CC      ppc-softmmu/accel/stubs/kvm-stub.o
  CC      ppc64-softmmu/arch_init.o
  CC      or1k-softmmu/softmmu/main.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      ppc-softmmu/accel/tcg/tcg-all.o
  CC      or1k-softmmu/target/openrisc/translate.o
  CC      arm-softmmu/qapi/qapi-types-machine-target.o
---
  CC      aarch64-softmmu/softmmu/vl.o
  CC      ppc-softmmu/hw/block/virtio-blk.o
  CC      ppc64-softmmu/accel/qtest.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      arm-softmmu/target/arm/arm-semi.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      ppc-softmmu/hw/block/vhost-user-blk.o
  CC      ppc64-softmmu/accel/stubs/hax-stub.o
  CC      arm-softmmu/target/arm/helper.o
  CC      ppc-softmmu/hw/block/dataplane/virtio-blk.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      arm-softmmu/target/arm/vfp_helper.o
  CC      aarch64-softmmu/target/arm/arm-semi.o
  CC      ppc64-softmmu/accel/stubs/hvf-stub.o
---
  CC      riscv32-softmmu/accel/tcg/tcg-runtime.o
  CC      ppc-softmmu/hw/vfio/amd-xgbe.o
  CC      riscv32-softmmu/accel/tcg/tcg-runtime-gvec.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      s390x-softmmu/tcg/tcg-op-gvec.o
  CC      aarch64-softmmu/target/arm/m_helper.o
  CC      arm-softmmu/target/arm/m_helper.o
---
  CC      sparc64-softmmu/hw/virtio/virtio-9p-pci.o
  CC      sparc64-softmmu/hw/virtio/virtio-scsi-pci.o
  CC      sh4-softmmu/hw/virtio/vhost-user-input-pci.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      riscv32-softmmu/target/riscv/fpu_helper.o
  CC      sparc-softmmu/target/sparc/mmu_helper.o
  CC      ppc64-softmmu/hw/ppc/spapr_pci_nvlink2.o
---
  CC      sh4-softmmu/target/sh4/helper.o
  CC      sh4-softmmu/target/sh4/cpu.o
  CC      sh4eb-softmmu/target/sh4/gdbstub.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      ppc64-softmmu/hw/ppc/mpc8544_guts.o
  CC      sh4-softmmu/target/sh4/monitor.o
  GEN     trace/generated-helpers.c
---
  LINK    sh4eb-softmmu/qemu-system-sh4eb
  CC      unicore32-softmmu/tcg/tcg.o
  CC      ppc64-softmmu/qapi/qapi-introspect.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      s390x-softmmu/hw/s390x/tod-qemu.o
  CC      sparc64-softmmu/target/sparc/win_helper.o
  LINK    sh4-softmmu/qemu-system-sh4
  CC      x86_64-softmmu/exec-vary.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      s390x-softmmu/hw/s390x/s390-ccw.o
  CC      s390x-softmmu/hw/s390x/ap-device.o
  CC      ppc64-softmmu/qapi/qapi-types-machine-target.o
---
  CC      x86_64-softmmu/fpu/softfloat.o
  CC      s390x-softmmu/qapi/qapi-visit-machine-target.o
  CC      s390x-softmmu/qapi/qapi-visit-misc-target.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      ppc64-softmmu/qapi/qapi-visit.o
  CC      ppc64-softmmu/qapi/qapi-events-machine-target.o
  CC      x86_64-softmmu/disas.o
---
  CC      unicore32-softmmu/accel/stubs/kvm-stub.o
  CC      x86_64-softmmu/accel/qtest.o
  CC      x86_64-softmmu/accel/kvm/kvm-all.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      x86_64-softmmu/accel/stubs/hax-stub.o
  CC      x86_64-softmmu/accel/stubs/hvf-stub.o
  CC      s390x-softmmu/target/s390x/cpu_models.o
---
  CC      xtensa-softmmu/exec-vary.o
  CC      x86_64-softmmu/accel/tcg/tcg-runtime-gvec.o
  CC      xtensa-softmmu/tcg/tcg.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      xtensaeb-softmmu/tcg/tcg.o
  CC      xtensaeb-softmmu/tcg/tcg-op.o
  CC      unicore32-softmmu/accel/tcg/cpu-exec.o
---
  CC      xtensa-softmmu/accel/tcg/tcg-runtime.o
  CC      x86_64-softmmu/hw/hyperv/hyperv.o
  CC      s390x-softmmu/softmmu/main.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      x86_64-softmmu/hw/hyperv/hyperv_testdev.o
  CC      alpha-linux-user/fpu/softfloat.o
  CC      unicore32-softmmu/target/unicore32/softmmu.o
---
  CC      x86_64-softmmu/hw/scsi/virtio-scsi.o
  CC      aarch64-linux-user/linux-user/signal.o
  CC      alpha-linux-user/accel/tcg/tcg-runtime-gvec.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      xtensa-softmmu/hw/core/machine-qmp-cmds.o
  CC      xtensaeb-softmmu/hw/display/virtio-gpu.o
  CC      alpha-linux-user/accel/tcg/cpu-exec.o
---
  CC      x86_64-softmmu/hw/scsi/virtio-scsi-dataplane.o
  GEN     arm-linux-user/config-target.h
  CC      arm-linux-user/exec.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      alpha-linux-user/accel/tcg/cpu-exec-common.o
  CC      arm-linux-user/exec-vary.o
  CC      xtensa-softmmu/hw/display/virtio-gpu.o
---
  GEN     mips64-linux-user/config-target.h
  CC      mips64-linux-user/exec.o
  CC      microblaze-linux-user/linux-user/exit.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      microblaze-linux-user/linux-user/fd-trans.o
  CC      mips-linux-user/accel/stubs/hvf-stub.o
  CC      microblazeel-linux-user/accel/stubs/kvm-stub.o
---
  CC      mipsn32el-linux-user/tcg/optimize.o
  CC      nios2-linux-user/tcg/tcg.o
  CC      nios2-linux-user/tcg/tcg-op.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      nios2-linux-user/tcg/tcg-op-vec.o
  CC      mipsel-linux-user/linux-user/uaccess.o
  CC      mips64el-linux-user/linux-user/mips64/cpu_loop.o
---
  LINK    tests/test-iov
  LINK    tests/test-bitmap
  LINK    fp-test
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-aio
  LINK    tests/test-aio-multithread
  LINK    tests/test-throttle
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-thread-pool
  LINK    tests/test-hbitmap
  LINK    tests/test-bdrv-drain
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-bdrv-graph-mod
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-blockjob
  LINK    tests/test-blockjob-txn
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-block-backend
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-block-iothread
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-image-locking
  LINK    tests/test-x86-cpuid
  LINK    tests/test-xbzrle
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-vmstate
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-cutils
  LINK    tests/test-shift128
  LINK    tests/test-mul64
  LINK    tests/test-int128
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/rcutorture
  LINK    tests/test-rcu-list
  LINK    tests/test-rcu-simpleq
---
  LINK    tests/test-crypto-hash
  LINK    tests/test-crypto-hmac
  LINK    tests/test-crypto-cipher
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  LINK    tests/test-crypto-secret
  LINK    tests/test-crypto-tlscredsx509
  LINK    tests/test-crypto-tlssession
---
  FLOAT TEST lt_quiet
  FLOAT TEST add
  FLOAT TEST sub
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  FLOAT TEST mul
  FLOAT TEST uint-to-float
  FLOAT TEST float-to-int
---
dbus-daemon[7868]: Could not get password database information for UID of current process: User "???" unknown or no memory to allocate password entry

**
ERROR:/tmp/qemu-test/src/tests/qtest/dbus-vmstate-test.c:114:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18)
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/dbus-vmstate-test.c:114:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18)
Aborted (core dumped)
cleaning up pid 7868
make: *** [/tmp/qemu-test/src/tests/Makefile.include:632: check-qtest-i386] Error 1
make: *** Waiting for unfinished jobs....
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory
---
dbus-daemon[9261]: Could not get password database information for UID of current process: User "???" unknown or no memory to allocate password entry

**
ERROR:/tmp/qemu-test/src/tests/qtest/dbus-vmstate-test.c:114:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18)
Aborted (core dumped)
cleaning up pid 9261
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/dbus-vmstate-test.c:114:get_connection: assertion failed (err == NULL): The connection is closed (g-io-error-quark, 18)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:632: check-qtest-x86_64] Error 1
  TEST    check-qtest-arm: tests/qtest/test-hmp
  TEST    check-qtest-arm: tests/qtest/qos-test
  TEST    check-qtest-aarch64: tests/qtest/test-hmp
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=7707505245c64dc984f787bb73ab9cb0', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-okbleiac/src/docker-src.2020-03-07-14.04.06.5872:/var/tmp/qemu:z,ro', 'qemu:ubuntu', '/var/tmp/qemu/run', 'test-clang']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=7707505245c64dc984f787bb73ab9cb0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-okbleiac/src'
make: *** [docker-run-test-clang@ubuntu] Error 2

real    19m23.296s
user    0m8.861s


The full log is available at
http://patchew.org/logs/20200307170251.GA7@dd5f6ec33fb0/testing.docker-clang@ubuntu/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency

[Eric Blake]

On 3/7/20 11:22 AM, Unai Martinez-Corral wrote:
> Spaces are added before '; then', for consistency.

For consistency with what?  This is not our prevailing style; as 
evidenced by this pre-patch search:

$ git grep 'if \[.*\];' | wc
     274    2186   18170
$ git grep 'if \[.*\] ;' | wc
      25     256    1573

and you are diverging from the dominant pattern.

> 
> All the tests are prefixed with 'x', in order to avoid risky comparisons
> (i.e. a user deliberately trying to provoke a syntax error).

This part, however, is good.  Since one part is controversial, you may 
want to split this into two patches, or even drop the reformatting part.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency

[Unai Martinez Corral]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

 2020/03/09 16:01, Eric Blake:

> On 3/7/20 11:22 AM, Unai Martinez-Corral wrote:
> > Spaces are added before '; then', for consistency.
>
> For consistency with what?  This is not our prevailing style; as
> evidenced by this pre-patch search:
>
> $ git grep 'if \[.*\];' | wc
>      274    2186   18170
> $ git grep 'if \[.*\] ;' | wc
>       25     256    1573
>
> and you are diverging from the dominant pattern.
>

For consistency within the script that is being modified. I'm not trying to
diverge, neither do I prefer any specific style.
Although the style in the current master is not consistent, ' ; ' is
significantly more frequent. When I was told to keep consistency in v2, I
picked that because it was the most common.
Anyway, I will push a new version where all these are changed to the
dominant pattern outside of this script.


> This part, however, is good.  Since one part is controversial, you may
> want to split this into two patches, or even drop the reformatting part.
>

Since the current master is neither consistent nor coherent with the
dominant pattern, I don't think I can drop the reformatting as long as I
want to fulfill your requirements.

[-- Attachment #2: Type: text/html, Size: 1775 bytes --]

Re: [PATCH v8 1/9] qemu-binfmt-conf.sh: enforce safe style consistency

[Eric Blake]

On 3/9/20 1:20 PM, Unai Martinez Corral wrote:
>   2020/03/09 16:01, Eric Blake:
> 
>> On 3/7/20 11:22 AM, Unai Martinez-Corral wrote:
>>> Spaces are added before '; then', for consistency.
>>
>> For consistency with what?  This is not our prevailing style; as
>> evidenced by this pre-patch search:
>>
>> $ git grep 'if \[.*\];' | wc
>>       274    2186   18170
>> $ git grep 'if \[.*\] ;' | wc
>>        25     256    1573
>>
>> and you are diverging from the dominant pattern.
>>
> 
> For consistency within the script that is being modified. I'm not trying to
> diverge, neither do I prefer any specific style.

Aha, I see what you were looking at: within the script itself, it was 10 
'] ;' vs. 2 '];'.  In which case, I'd recommend swapping the 10 
instances over to be common with the rest of the code base, rather than 
the 2 away from the rest of the code base but towards the rest of the 
script.

> Although the style in the current master is not consistent, ' ; ' is
> significantly more frequent. When I was told to keep consistency in v2, I
> picked that because it was the most common.
> Anyway, I will push a new version where all these are changed to the
> dominant pattern outside of this script.

Good to hear.

> 
> 
>> This part, however, is good.  Since one part is controversial, you may
>> want to split this into two patches, or even drop the reformatting part.
>>
> 
> Since the current master is neither consistent nor coherent with the
> dominant pattern, I don't think I can drop the reformatting as long as I
> want to fulfill your requirements.

Splitting into two patches (one to fix '] ;' spacing, the other to add 
'[ "x$..."' protection) is then the best course of action.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org