[Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[Unai Martinez-Corral]

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. Valid separators are space, tab, newline and comma.
  Keywords 'ALL' and 'NONE' are supported.
* Option '--reset ARCHS' is added, which allows to remove an already registered
  target interpreter or a list of them. Valid separator is comma. Keyword 'ALL'
  is supported. The implementation is functional but partial. Please, see
  the corresponding commit.
* Support to set options through environment variables is added: QEMU_PATH,
  QEMU_SUFFIX, QEMU_PERSISTENT, QEMU_CREDENTIAL and QEMU_TARGETS.

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.

The main differences compared to version 2 are:

* Suggestions by Eric Blake regarding syntax and style consistency have been addressed.
* Changes are split in multiple patches, instead of a single one.
* Option names and support of environament variables is more consistent.

Based on:

* [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)

Regards

Unai Martinez-Corral (10):
  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: 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 <CPUS>
  qemu-binfmt-conf.sh: add option --reset <ARCHS>
  qemu-binfmt-conf.sh: refactor usage()
  qemu-binfmt-conf.sh: update usage()
  qemu-binfmt-conf.sh: support QEMU_TARGETS

 scripts/qemu-binfmt-conf.sh | 190
 1 file changed, 125 insertions(+), 65 deletions(-)

[Qemu-devel] [PATCH v3 1/10] 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>
---
 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 b5a16742a1..0009385be2 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -219,12 +219,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
@@ -255,10 +255,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

@@ -296,18 +296,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.20.1

[Qemu-devel] [PATCH v3 2/10] 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 reduntant. 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>
---
 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 0009385be2..ca15ff8092 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -168,8 +168,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

@@ -184,9 +184,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.

@@ -324,7 +324,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
@@ -373,12 +373,10 @@ while true ; do
         exit 1
         ;;
     -c|--credential)
-        shift
-        CREDENTIAL="$1"
+        CREDENTIAL="yes"
         ;;
     -p|--persistent)
-        shift
-        PERSISTENT="$1"
+        PERSISTENT="yes"
         ;;
     *)
         break
--
2.20.1

[Qemu-devel] [PATCH v3 3/10] 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>
---
 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 ca15ff8092..e7a714e22c 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -186,9 +186,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 :

@@ -255,10 +257,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

@@ -281,7 +283,7 @@ package qemu-$cpu
 interpreter $qemu
 magic $magic
 mask $mask
-credential $CREDENTIAL
+credential $QEMU_CREDENTIAL
 EOF
 }

@@ -320,8 +322,10 @@ SYSTEMDDIR="/etc/binfmt.d"
 DEBIANDIR="/usr/share/binfmts"

 QEMU_PATH=/usr/local/bin
-CREDENTIAL=no
-PERSISTENT=no
+
+QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
+QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
+
 QEMU_SUFFIX=""

 options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential,persistent -- "$@")
@@ -373,10 +377,10 @@ while true ; do
         exit 1
         ;;
     -c|--credential)
-        CREDENTIAL="yes"
+        QEMU_CREDENTIAL="yes"
         ;;
     -p|--persistent)
-        PERSISTENT="yes"
+        QEMU_PERSISTENT="yes"
         ;;
     *)
         break
--
2.20.1

[Qemu-devel] [PATCH v3 4/10] 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>
---
 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 e7a714e22c..68aa4c3f78 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -167,15 +167,15 @@ qemu_get_family() {

 usage() {
     cat <<EOF
-Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
+Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
                            [--help][--credential][--exportdir PATH]
-                           [--persistent][--qemu-suffix SUFFIX]
+                           [--persistent][--suffix SUFFIX]

        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
+       --path:        set path to qemu interpreter ($QEMU_PATH)
+       --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,
@@ -328,7 +328,7 @@ QEMU_PERSISTENT="${QEMU_PERSISTENT:-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
@@ -360,11 +360,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.20.1

[Qemu-devel] [PATCH v3 5/10] 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>
---
 scripts/qemu-binfmt-conf.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 68aa4c3f78..c113ff131e 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -176,6 +176,7 @@ Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
        --help:        display this usage
        --path:        set path to qemu interpreter ($QEMU_PATH)
        --suffix:      add a suffix to the default interpreter name
+                      ($QEMU_SUFFIX)
        --debian:      don't write into /proc,
                       instead generate update-binfmts templates
        --systemd:     don't write into /proc,
@@ -321,13 +322,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_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"

-QEMU_SUFFIX=""
-
 options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"

--
2.20.1

[Qemu-devel] [PATCH v3 6/10] qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>

[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.

The list can be separated by spaces, tabs, newlines or commas. If no
positional argument is provided, or when it is 'ALL', 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.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
---
 scripts/qemu-binfmt-conf.sh | 92 +++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 35 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index c113ff131e..2751363089 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -6,6 +6,36 @@ 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 target CPUS is/are in the supported target list
+qemu_check_target_list() {
+    all="$qemu_target_list"
+    if [ "x$1" = "xALL" ] ; then
+      checked_target_list="$all"
+      return
+    fi
+    list=""
+    bIFS="$IFS"
+    IFS=$"$IFS",
+    for target ; do
+        unknown_target="true"
+        for cpu in $all ; do
+            if [ "x$cpu" = "x$target" ] ; then
+                list="$list $target"
+                unknown_target="false"
+                break
+            fi
+        done
+        if [ "$unknown_target" = "true" ] ; then
+            IFS="$bIFS"
+            echo "ERROR: unknown CPU \"$target\"" 1>&2
+            usage
+            exit 1
+        fi
+    done
+    IFS="$bIFS"
+    checked_target_list="$list"
+}
+
 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
@@ -167,11 +197,14 @@ qemu_get_family() {

 usage() {
     cat <<EOF
-Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
+Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd]
                            [--help][--credential][--exportdir PATH]
-                           [--persistent][--suffix SUFFIX]
+                           [--persistent][--suffix SUFFIX][CPUS]

-       Configure binfmt_misc to use qemu interpreter
+       Configure binfmt_misc to use qemu interpreter for the given CPUS.
+       Supported formats for CPUS are: single arch or comma/space separated list.
+       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
+       cpus. If CPUS is 'NONE', no interpreter is configured.

        --help:        display this usage
        --path:        set path to qemu interpreter ($QEMU_PATH)
@@ -180,9 +213,10 @@ Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
        --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
+                      instead generate file for systemd-binfmt.service;
+                      environment variable HOST_ARCH allows to override 'uname'
+                      to generate configuration files for a different
+                      architecture than the current one.
        --exportdir:   define where to write configuration files
                       (default: $SYSTEMDDIR or $DEBIANDIR)
        --credential:  if present, credential and security tokens are
@@ -201,14 +235,7 @@ Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]

         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.
-
-    where CPU is one of:
-
-        $qemu_target_list
+    QEMU target list: $qemu_target_list

 EOF
 }
@@ -289,12 +316,22 @@ EOF
 }

 qemu_set_binfmts() {
+    if [ "x$1" = "xNONE" ] ; then
+      return
+    fi
+
     # probe cpu type
     host_family=$(qemu_get_family)

+    # reduce the list of target interpreters to those given in the CLI
+    targets="$@"
+    if [ $# -eq 0 ] ; then
+      targets="ALL"
+    fi
+    qemu_check_target_list $targets
+
     # register the interpreter for each cpu except for the native one
-
-    for cpu in ${qemu_target_list} ; do
+    for cpu in $checked_target_list ; do
         magic=$(eval echo \$${cpu}_magic)
         mask=$(eval echo \$${cpu}_mask)
         family=$(eval echo \$${cpu}_family)
@@ -327,7 +364,7 @@ QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-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
@@ -341,23 +378,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
@@ -388,5 +408,7 @@ while true ; do
     shift
 done

+shift
+
 $CHECK
-qemu_set_binfmts
+qemu_set_binfmts "$@"
--
2.20.1

[Qemu-devel] [PATCH v3 7/10] qemu-binfmt-conf.sh: add option --reset <ARCHS>

[Unai Martinez-Corral]

This is a partial implementation.

Allows to remove a single or a list of already registered binfmt
interpreters. If <ARCHS> is a list, it must be comma-separated.
Valid values are those in qemu_target_list. If <ARCHS> is 'ALL', all
the existing 'qemu-*' interpreters are removed.

This is partial because 'debian' and 'systemd' configurations are not
removed. If option 'reset' is provided before any of those, reset is
executed first and the configuration proceeds. However, if 'reset' is
provided after any of them, the script will exit with error 'option
reset 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>
---
 scripts/qemu-binfmt-conf.sh | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 2751363089..824e3c4c34 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -197,8 +197,8 @@ qemu_get_family() {

 usage() {
     cat <<EOF
-Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd]
-                           [--help][--credential][--exportdir PATH]
+Usage: qemu-binfmt-conf.sh [--help][--path PATH][--debian][--systemd]
+                           [--reset ARCHS][--credential][--exportdir PATH]
                            [--persistent][--suffix SUFFIX][CPUS]

        Configure binfmt_misc to use qemu interpreter for the given CPUS.
@@ -219,6 +219,9 @@ Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd]
                       architecture than the current one.
        --exportdir:   define where to write configuration files
                       (default: $SYSTEMDDIR or $DEBIANDIR)
+       --reset:       remove registered interpreter for target ARCHS (comma
+                      separated list). If ARCHS is 'ALL', remove all registered
+                      'qemu-*' interpreters.
        --credential:  if present, credential and security tokens are
                       calculated according to the binary to interpret
                       ($QEMU_CREDENTIAL=yes)
@@ -353,8 +356,28 @@ qemu_set_binfmts() {
     done
 }

+qemu_remove_notimplemented() {
+    echo "ERROR: option reset not implemented for this mode yet" 1>&2
+    usage
+    exit 1
+}
+
+qemu_remove_interpreter() {
+    names='qemu-*'
+    if [ "x$1" != "xALL" ] ; then
+        qemu_check_target_list $1
+        unset names pre
+        for t in $checked_target_list ; do
+            names="${names}${pre}qemu-$t"
+            pre=' -o -name '
+        done
+    fi
+    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;
+}
+
 CHECK=qemu_check_bintfmt_misc
 BINFMT_SET=qemu_register_interpreter
+BINFMT_REMOVE=qemu_remove_interpreter

 SYSTEMDDIR="/etc/binfmt.d"
 DEBIANDIR="/usr/share/binfmts"
@@ -364,19 +387,26 @@ QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"

-options=$(getopt -o :dsQ:S:e:hcp -l debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
+options=$(getopt -o r:dsQ:S:e:hcp -l reset:,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"

 while true ; do
     case "$1" in
+    -r|--reset)
+        shift
+        $CHECK
+        qemu_remove_interpreter $1
+        ;;
     -d|--debian)
         CHECK=qemu_check_debian
         BINFMT_SET=qemu_generate_debian
+        BINFMT_REMOVE=qemu_remove_notimplemented
         EXPORTDIR=${EXPORTDIR:-$DEBIANDIR}
         ;;
     -s|--systemd)
         CHECK=qemu_check_systemd
         BINFMT_SET=qemu_generate_systemd
+        BINFMT_REMOVE=qemu_remove_notimplemented
         EXPORTDIR=${EXPORTDIR:-$SYSTEMDDIR}
         ;;
     -Q|--path)
--
2.20.1

[Qemu-devel] [PATCH v3 8/10] qemu-binfmt-conf.sh: refactor usage()

[Unai Martinez-Corral]

Reorder how the options are presented to the user. Move 'systemd'
and 'debian' to the end, so that the latter is close to the additional
comments and example commands about it. Make the order in the prototype
consistent with the list where each option is explained.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
---
 scripts/qemu-binfmt-conf.sh | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 824e3c4c34..4a45636b53 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -197,9 +197,9 @@ qemu_get_family() {

 usage() {
     cat <<EOF
-Usage: qemu-binfmt-conf.sh [--help][--path PATH][--debian][--systemd]
-                           [--reset ARCHS][--credential][--exportdir PATH]
-                           [--persistent][--suffix SUFFIX][CPUS]
+Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
+                           [--persistent][--credential][--exportdir PATH]
+                           [--reset ARCHS][--systemd][--debian][CPUS]

        Configure binfmt_misc to use qemu interpreter for the given CPUS.
        Supported formats for CPUS are: single arch or comma/space separated list.
@@ -210,25 +210,25 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--debian][--systemd]
        --path:        set path to qemu interpreter ($QEMU_PATH)
        --suffix:      add a suffix to the default interpreter name
                       ($QEMU_SUFFIX)
-       --debian:      don't write into /proc,
-                      instead generate update-binfmts templates
-       --systemd:     don't write into /proc,
-                      instead generate file for systemd-binfmt.service;
-                      environment variable HOST_ARCH allows to override 'uname'
-                      to generate configuration files for a different
-                      architecture than the current one.
+       --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)
+       --credential:  if present, credential and security tokens are
+                      calculated according to the binary to interpret
+                      ($QEMU_CREDENTIAL=yes)
        --exportdir:   define where to write configuration files
                       (default: $SYSTEMDDIR or $DEBIANDIR)
        --reset:       remove registered interpreter for target ARCHS (comma
                       separated list). If ARCHS is 'ALL', remove all registered
                       'qemu-*' interpreters.
-       --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)
+       --systemd:     don't write into /proc,
+                      instead generate file for systemd-binfmt.service;
+                      environment variable HOST_ARCH allows to override 'uname'
+                      to generate configuration files for a different
+                      architecture than the current one.
+       --debian:      don't write into /proc,
+                      instead generate update-binfmts templates

     To import templates with update-binfmts, use :

--
2.20.1

[Qemu-devel] [PATCH v3 9/10] qemu-binfmt-conf.sh: update usage()

[Unai Martinez-Corral]

Reduce indentation to better use available space.

Add list of supported environment variables and their default values.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
---
 scripts/qemu-binfmt-conf.sh | 61 ++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 4a45636b53..8ca2647ed0 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -201,34 +201,31 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
                            [--persistent][--credential][--exportdir PATH]
                            [--reset ARCHS][--systemd][--debian][CPUS]

-       Configure binfmt_misc to use qemu interpreter for the given CPUS.
-       Supported formats for CPUS are: single arch or comma/space separated list.
-       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
-       cpus. If CPUS is 'NONE', no interpreter is configured.
+    Configure binfmt_misc to use qemu interpreter for the given CPUS.
+    Supported formats for CPUS are: single arch or comma/space separated list.
+    See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
+    cpus. If CPUS is 'NONE', no interpreter is configured.

-       --help:        display this usage
-       --path:        set path to qemu interpreter ($QEMU_PATH)
-       --suffix:      add a suffix to the default interpreter name
-                      ($QEMU_SUFFIX)
-       --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)
-       --credential:  if present, credential and security tokens are
-                      calculated according to the binary to interpret
-                      ($QEMU_CREDENTIAL=yes)
-       --exportdir:   define where to write configuration files
-                      (default: $SYSTEMDDIR or $DEBIANDIR)
-       --reset:       remove registered interpreter for target ARCHS (comma
-                      separated list). If ARCHS is 'ALL', remove all registered
-                      'qemu-*' interpreters.
-       --systemd:     don't write into /proc,
-                      instead generate file for systemd-binfmt.service;
-                      environment variable HOST_ARCH allows to override 'uname'
-                      to generate configuration files for a different
-                      architecture than the current one.
-       --debian:      don't write into /proc,
-                      instead generate update-binfmts templates
+    --help:        display this usage.
+    --path:        set path to qemu interpreter.
+    --suffix:      add a suffix to the default interpreter name.
+    --persistent:  if present, the interpreter is loaded when binfmt is
+                   configured and remains in memory. All future uses
+                   are cloned from the open file.
+    --credential:  if present, credential and security tokens are
+                   calculated according to the binary to interpret.
+    --exportdir:   define where to write configuration files.
+                   (default: $SYSTEMDDIR or $DEBIANDIR)
+    --reset:       remove registered interpreter for target ARCHS (comma
+                   separated list). If ARCHS is 'ALL', remove all registered
+                   'qemu-*' interpreters.
+    --systemd:     don't write into /proc,
+                   instead generate file(s) for systemd-binfmt.service;
+                   environment variable HOST_ARCH allows to override 'uname'
+                   to generate configuration files for a different
+                   architecture than the current one.
+    --debian:      don't write into /proc,
+                   instead generate update-binfmts templates.

     To import templates with update-binfmts, use :

@@ -240,6 +237,14 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]

     QEMU target list: $qemu_target_list

+    Options 'path, 'suffix', 'persistent' and 'credential' are also supported
+    through environment variables. Defaults are:
+
+      QEMU_PATH=/usr/local/bin
+      QEMU_SUFFIX=
+      QEMU_PERSISTENT=no
+      QEMU_CREDENTIAL=no
+
 EOF
 }

@@ -384,8 +389,8 @@ DEBIANDIR="/usr/share/binfmts"

 QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
 QEMU_SUFFIX="${QEMU_SUFFIX:-}"
-QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
+QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"

 options=$(getopt -o r:dsQ:S:e:hcp -l reset:,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
--
2.20.1

[Qemu-devel] [PATCH v3 10/10] qemu-binfmt-conf.sh: support QEMU_TARGETS

[Unai Martinez-Corral]

Rename CPUS to TARGETS, and support QEMU_TARGETS environment variable.
This does not break backward compatibility, because it is just a placeholder.

Consistently with 'path', 'suffix', 'persistent' and 'credential',
provide an environment variable to set the list of target architectures.
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 'ALL'.

Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
---
 scripts/qemu-binfmt-conf.sh | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 8ca2647ed0..0676f86512 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -6,7 +6,7 @@ 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 target CPUS is/are in the supported target list
+# check if given TARGETS is/are in the supported target list
 qemu_check_target_list() {
     all="$qemu_target_list"
     if [ "x$1" = "xALL" ] ; then
@@ -199,12 +199,12 @@ usage() {
     cat <<EOF
 Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
                            [--persistent][--credential][--exportdir PATH]
-                           [--reset ARCHS][--systemd][--debian][CPUS]
+                           [--reset ARCHS][--systemd][--debian][TARGETS]

-    Configure binfmt_misc to use qemu interpreter for the given CPUS.
-    Supported formats for CPUS are: single arch or comma/space separated list.
-    See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
-    cpus. If CPUS is 'NONE', no interpreter is configured.
+    Configure binfmt_misc to use qemu interpreter for the given TARGETS.
+    Supported formats for TARGETS are: single arch or comma/space separated list.
+    See QEMU target list below. If TARGETS is 'ALL' or empty, configure all known
+    TARGETS. If TARGETS is 'NONE', no interpreter is configured.

     --help:        display this usage.
     --path:        set path to qemu interpreter.
@@ -244,6 +244,7 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
       QEMU_SUFFIX=
       QEMU_PERSISTENT=no
       QEMU_CREDENTIAL=no
+      QEMU_TARGETS=

 EOF
 }
@@ -324,17 +325,17 @@ EOF
 }

 qemu_set_binfmts() {
-    if [ "x$1" = "xNONE" ] ; then
-      return
-    fi
-
     # probe cpu type
     host_family=$(qemu_get_family)

     # reduce the list of target interpreters to those given in the CLI
-    targets="$@"
     if [ $# -eq 0 ] ; then
-      targets="ALL"
+      targets="${QEMU_TARGETS:-ALL}"
+    else
+      if [ "x$1" = "xNONE" ] ; then
+        return
+      fi
+      targets="$@"
     fi
     qemu_check_target_list $targets

@@ -391,6 +392,7 @@ QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
 QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
+QEMU_TARGETS="${QEMU_TARGETS:-}"

 options=$(getopt -o r:dsQ:S:e:hcp -l reset:,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"
--
2.20.1

Re: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190306031221.GA53@03612eec87fc/



Hi,

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

Type: series
Message-id: 20190306031221.GA53@03612eec87fc
Subject: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

=== 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
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190306031221.GA53@03612eec87fc -> patchew/20190306031221.GA53@03612eec87fc
Switched to a new branch 'test'
ba748151ce qemu-binfmt-conf.sh: support QEMU_TARGETS
0e0dad2d82 qemu-binfmt-conf.sh: update usage()
20a92d1c39 qemu-binfmt-conf.sh: refactor usage()
e272055870 qemu-binfmt-conf.sh: add option --reset <ARCHS>
87a83e5953 qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>
283e2ed563 qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
a9a99660ac qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
eb8911ad6d qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
4cfbfebe88 qemu-binfmt-conf.sh: make opts -p and -c boolean
7d848c25ae qemu-binfmt-conf.sh: enforce safe style consistency

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

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit 4cfbfebe88f7 (qemu-binfmt-conf.sh: make opts -p and -c boolean)
ERROR: line over 90 characters
#50: FILE: scripts/qemu-binfmt-conf.sh:327:
+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/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/10 Checking commit eb8911ad6dcb (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT)
4/10 Checking commit a9a99660ac52 (qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options)
ERROR: line over 90 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 40 lines checked

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

5/10 Checking commit 283e2ed56352 (qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX)
6/10 Checking commit 87a83e59538a (qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>)
WARNING: line over 80 characters
#80: FILE: scripts/qemu-binfmt-conf.sh:205:
+       Supported formats for CPUS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#81: FILE: scripts/qemu-binfmt-conf.sh:206:
+       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known

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

total: 1 errors, 2 warnings, 144 lines checked

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

7/10 Checking commit e272055870f8 (qemu-binfmt-conf.sh: add option --reset <ARCHS>)
WARNING: line over 80 characters
#71: FILE: scripts/qemu-binfmt-conf.sh:375:
+    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;

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

total: 1 errors, 1 warnings, 74 lines checked

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

8/10 Checking commit 20a92d1c3962 (qemu-binfmt-conf.sh: refactor usage())
9/10 Checking commit 0e0dad2d8241 (qemu-binfmt-conf.sh: update usage())
10/10 Checking commit ba748151cede (qemu-binfmt-conf.sh: support QEMU_TARGETS)
WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:205:
+    Supported formats for TARGETS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:206:
+    See QEMU target list below. If TARGETS is 'ALL' or empty, configure all known

total: 0 errors, 2 warnings, 62 lines checked

Patch 10/10 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/20190306031221.GA53@03612eec87fc/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v3 1/10] qemu-binfmt-conf.sh: enforce safe style consistency

[Laurent Vivier]

On 06/03/2019 05:36, Unai Martinez-Corral wrote:
> 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>
> ---
>  scripts/qemu-binfmt-conf.sh | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v3 2/10] qemu-binfmt-conf.sh: make opts -p and -c boolean

[Laurent Vivier]

On 06/03/2019 05:42, Unai Martinez-Corral wrote:
> This patch breaks backward compatibility.
> 
> Both '--persistent' and '--credential' default to 'no'. Hence, '-p no'
> or '-c no' are reduntant. 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>
> ---
>  scripts/qemu-binfmt-conf.sh | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190306031221.GA53@03612eec87fc/



Hi,

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

Type: series
Message-id: 20190306031221.GA53@03612eec87fc
Subject: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

=== 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
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190306031221.GA53@03612eec87fc -> patchew/20190306031221.GA53@03612eec87fc
Switched to a new branch 'test'
bedda47489 qemu-binfmt-conf.sh: support QEMU_TARGETS
ac6c160734 qemu-binfmt-conf.sh: update usage()
7c6effbb95 qemu-binfmt-conf.sh: refactor usage()
4f23b0b002 qemu-binfmt-conf.sh: add option --reset <ARCHS>
71392f21ea qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>
946395451d qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
abd051bb2e qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
700f8fcdf7 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
65a2b3aabf qemu-binfmt-conf.sh: make opts -p and -c boolean
3272769df3 qemu-binfmt-conf.sh: enforce safe style consistency

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

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit 65a2b3aabf28 (qemu-binfmt-conf.sh: make opts -p and -c boolean)
ERROR: line over 90 characters
#50: FILE: scripts/qemu-binfmt-conf.sh:327:
+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/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/10 Checking commit 700f8fcdf7cb (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT)
4/10 Checking commit abd051bb2edc (qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options)
ERROR: line over 90 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 40 lines checked

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

5/10 Checking commit 946395451d14 (qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX)
6/10 Checking commit 71392f21ea89 (qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>)
WARNING: line over 80 characters
#80: FILE: scripts/qemu-binfmt-conf.sh:205:
+       Supported formats for CPUS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#81: FILE: scripts/qemu-binfmt-conf.sh:206:
+       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known

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

total: 1 errors, 2 warnings, 144 lines checked

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

7/10 Checking commit 4f23b0b00271 (qemu-binfmt-conf.sh: add option --reset <ARCHS>)
WARNING: line over 80 characters
#71: FILE: scripts/qemu-binfmt-conf.sh:375:
+    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;

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

total: 1 errors, 1 warnings, 74 lines checked

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

8/10 Checking commit 7c6effbb95b5 (qemu-binfmt-conf.sh: refactor usage())
9/10 Checking commit ac6c160734be (qemu-binfmt-conf.sh: update usage())
10/10 Checking commit bedda4748942 (qemu-binfmt-conf.sh: support QEMU_TARGETS)
WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:205:
+    Supported formats for TARGETS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:206:
+    See QEMU target list below. If TARGETS is 'ALL' or empty, configure all known

total: 0 errors, 2 warnings, 62 lines checked

Patch 10/10 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/20190306031221.GA53@03612eec87fc/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190306031221.GA53@03612eec87fc/



Hi,

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

Type: series
Message-id: 20190306031221.GA53@03612eec87fc
Subject: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

=== 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
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190306031221.GA53@03612eec87fc -> patchew/20190306031221.GA53@03612eec87fc
Switched to a new branch 'test'
e8052727ed qemu-binfmt-conf.sh: support QEMU_TARGETS
a5fe269cca qemu-binfmt-conf.sh: update usage()
e65c0db987 qemu-binfmt-conf.sh: refactor usage()
66b0955155 qemu-binfmt-conf.sh: add option --reset <ARCHS>
2200f3015d qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>
686c9749cf qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
0c9fb8d9aa qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
df7c3ef8c3 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
e2f88c2bbf qemu-binfmt-conf.sh: make opts -p and -c boolean
08b6a8a8a1 qemu-binfmt-conf.sh: enforce safe style consistency

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

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit e2f88c2bbf66 (qemu-binfmt-conf.sh: make opts -p and -c boolean)
ERROR: line over 90 characters
#51: FILE: scripts/qemu-binfmt-conf.sh:327:
+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/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/10 Checking commit df7c3ef8c37d (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT)
4/10 Checking commit 0c9fb8d9aa9f (qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options)
ERROR: line over 90 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 40 lines checked

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

5/10 Checking commit 686c9749cff9 (qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX)
6/10 Checking commit 2200f3015dd0 (qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>)
WARNING: line over 80 characters
#80: FILE: scripts/qemu-binfmt-conf.sh:205:
+       Supported formats for CPUS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#81: FILE: scripts/qemu-binfmt-conf.sh:206:
+       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known

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

total: 1 errors, 2 warnings, 144 lines checked

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

7/10 Checking commit 66b095515580 (qemu-binfmt-conf.sh: add option --reset <ARCHS>)
WARNING: line over 80 characters
#71: FILE: scripts/qemu-binfmt-conf.sh:375:
+    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;

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

total: 1 errors, 1 warnings, 74 lines checked

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

8/10 Checking commit e65c0db9871e (qemu-binfmt-conf.sh: refactor usage())
9/10 Checking commit a5fe269cca58 (qemu-binfmt-conf.sh: update usage())
10/10 Checking commit e8052727edf4 (qemu-binfmt-conf.sh: support QEMU_TARGETS)
WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:205:
+    Supported formats for TARGETS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:206:
+    See QEMU target list below. If TARGETS is 'ALL' or empty, configure all known

total: 0 errors, 2 warnings, 62 lines checked

Patch 10/10 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/20190306031221.GA53@03612eec87fc/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v3 3/10] qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT

[Laurent Vivier]

On 06/03/2019 05:46, Unai Martinez-Corral wrote:
> 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.
> 

Could you explain why we could need to use the env variable instead of
the command line parameter?

I don't see any use case for that.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 4/10] qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options

[Laurent Vivier]

On 06/03/2019 05:49, Unai Martinez-Corral wrote:
> 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>
> ---
>  scripts/qemu-binfmt-conf.sh | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v3 5/10] qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX

[Laurent Vivier]

On 06/03/2019 05:49, Unai Martinez-Corral wrote:
> Allow to set 'path' or 'suffix' through environment variables,
> consistently with 'persistent' and 'credential'.
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 68aa4c3f78..c113ff131e 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -176,6 +176,7 @@ Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
>         --help:        display this usage
>         --path:        set path to qemu interpreter ($QEMU_PATH)
>         --suffix:      add a suffix to the default interpreter name
> +                      ($QEMU_SUFFIX)
>         --debian:      don't write into /proc,
>                        instead generate update-binfmts templates
>         --systemd:     don't write into /proc,
> @@ -321,13 +322,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_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
>  QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
> 
> -QEMU_SUFFIX=""
> -
>  options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
>  eval set -- "$options"
> 
> --
> 2.20.1
> 

Same question as for PATCH 3/10: why?

And I think the usage text should be formatted differently to be cleared and looks 
like more the one of qemu-XXX:

  usage: qemu-m68k [options] program [arguments...]
  Linux CPU emulator (compiled for m68k emulation)

  Options and associated environment variables:

  Argument             Env-variable      Description
  -h                                     print this help
  -help                                  
  -g port              QEMU_GDB          wait gdb connection to 'port'
  -L path              QEMU_LD_PREFIX    set the elf interpreter prefix to 'path'
  -s size              QEMU_STACK_SIZE   set the stack size to 'size' bytes
  ...

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190306031221.GA53@03612eec87fc/



Hi,

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

Type: series
Message-id: 20190306031221.GA53@03612eec87fc
Subject: [Qemu-devel] [PATCH v3 0/10] qemu-binfmt-conf.sh

=== 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
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190306031221.GA53@03612eec87fc -> patchew/20190306031221.GA53@03612eec87fc
Switched to a new branch 'test'
fba284afb5 qemu-binfmt-conf.sh: support QEMU_TARGETS
4457bd13fd qemu-binfmt-conf.sh: update usage()
151616125f qemu-binfmt-conf.sh: refactor usage()
fbdecd5e3d qemu-binfmt-conf.sh: add option --reset <ARCHS>
68a0016a98 qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>
443f62c7b4 qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
40a55b7af4 qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
21aa966104 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
7811dcc19c qemu-binfmt-conf.sh: make opts -p and -c boolean
2e880d1b68 qemu-binfmt-conf.sh: enforce safe style consistency

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

total: 0 errors, 1 warnings, 47 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit 7811dcc19c2e (qemu-binfmt-conf.sh: make opts -p and -c boolean)
ERROR: line over 90 characters
#51: FILE: scripts/qemu-binfmt-conf.sh:327:
+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/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/10 Checking commit 21aa96610492 (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT)
4/10 Checking commit 40a55b7af4ec (qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options)
ERROR: line over 90 characters
#46: FILE: scripts/qemu-binfmt-conf.sh:331:
+options=$(getopt -o ds:Q:S:e:hcp -l debian,systemd:,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

total: 1 errors, 0 warnings, 40 lines checked

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

5/10 Checking commit 443f62c7b47e (qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX)
6/10 Checking commit 68a0016a98c6 (qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>)
WARNING: line over 80 characters
#80: FILE: scripts/qemu-binfmt-conf.sh:205:
+       Supported formats for CPUS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#81: FILE: scripts/qemu-binfmt-conf.sh:206:
+       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known

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

total: 1 errors, 2 warnings, 144 lines checked

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

7/10 Checking commit fbdecd5e3dc1 (qemu-binfmt-conf.sh: add option --reset <ARCHS>)
WARNING: line over 80 characters
#71: FILE: scripts/qemu-binfmt-conf.sh:375:
+    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;

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

total: 1 errors, 1 warnings, 74 lines checked

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

8/10 Checking commit 151616125f5b (qemu-binfmt-conf.sh: refactor usage())
9/10 Checking commit 4457bd13fda7 (qemu-binfmt-conf.sh: update usage())
10/10 Checking commit fba284afb546 (qemu-binfmt-conf.sh: support QEMU_TARGETS)
WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:205:
+    Supported formats for TARGETS are: single arch or comma/space separated list.

WARNING: line over 80 characters
#45: FILE: scripts/qemu-binfmt-conf.sh:206:
+    See QEMU target list below. If TARGETS is 'ALL' or empty, configure all known

total: 0 errors, 2 warnings, 62 lines checked

Patch 10/10 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/20190306031221.GA53@03612eec87fc/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v3 3/10] qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT

[Unai Martinez Corral]

2019/3/9 10:42, Laurent Vivier:
> On 06/03/2019 05:46, Unai Martinez-Corral wrote:
> > 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.
>
> Could you explain why we could need to use the env variable instead of
> the command line parameter?
>
> I don't see any use case for that.

The main use case is to provide defaults when this script is included
in a docker container. There are three actors involved:

- Developers of QEMU providing some defaults in the script.
- Developer of a docker image including the script from upstream but
changing some defaults by setting envvars inside the container.
- User of the container that might want to override the settings
either by setting the envvars or through the command line.

If the entrypoint to the docker image is any script that executes
qemu-binfmt-conf.sh at some point, the user cannot provide options
through the command line. Envvars allow to do so, without requiring
the user to customize the docker image.

Regards,

Unai

Re: [Qemu-devel] [PATCH v3 5/10] qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX

[Unai Martinez Corral]

2019/3/9 10:46, Laurent Vivier:
> On 06/03/2019 05:49, Unai Martinez-Corral wrote:
> > -QEMU_PATH=/usr/local/bin
> > -
> > +QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
> > +QEMU_SUFFIX="${QEMU_SUFFIX:-}"
> >  QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
> >  QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
> >
> > -QEMU_SUFFIX=""
>
> Same question as for PATCH 3/10: why?

Same context. Moreover, in this case envvars already existed and were
hardcoded. I don't think it does any harm to let a external caller set
the defaults.

> And I think the usage text should be formatted differently to be cleared and looks
> like more the one of qemu-XXX:

Agree. I will update when the remaining patches are reviewed, since 6
out of 10 are affected by this change.

Unai

Re: [Qemu-devel] [PATCH v3 6/10] qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>

[Laurent Vivier]

On 06/03/2019 05:50, Unai Martinez-Corral wrote:
> 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.
> 
> The list can be separated by spaces, tabs, newlines or commas. If no

I think it would be simpler to not manage commas, and to use the default
field separator.

> positional argument is provided, or when it is 'ALL', 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.
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 92 +++++++++++++++++++++++--------------
>  1 file changed, 57 insertions(+), 35 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index c113ff131e..2751363089 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -6,6 +6,36 @@ 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 target CPUS is/are in the supported target list
> +qemu_check_target_list() {
> +    all="$qemu_target_list"

Why do you need to introduce a new variable "all", you can use directly
"qemu_target_list".

> +    if [ "x$1" = "xALL" ] ; then
> +      checked_target_list="$all"
> +      return
> +    fi
> +    list=""

No need to introduce a new variable, you can use directly
checked_target_list

> +    bIFS="$IFS"
> +    IFS=$"$IFS",

Keep it simple, don't manage comma.

> +    for target ; do
> +        unknown_target="true"
> +        for cpu in $all ; do

Use directly "qemu_target_list"

> +            if [ "x$cpu" = "x$target" ] ; then
> +                list="$list $target"
> +                unknown_target="false"
> +                break
> +            fi
> +        done
> +        if [ "$unknown_target" = "true" ] ; then

You don't need a new variable, you can check "x$cpu" = "x$target" to
know if you have found the target.

> +            IFS="$bIFS"
> +            echo "ERROR: unknown CPU \"$target\"" 1>&2
> +            usage
> +            exit 1
> +        fi
> +    done
> +    IFS="$bIFS"
> +    checked_target_list="$list"
> +}
> +
>  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
> @@ -167,11 +197,14 @@ qemu_get_family() {
> 
>  usage() {
>      cat <<EOF
> -Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
> +Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd]
>                             [--help][--credential][--exportdir PATH]
> -                           [--persistent][--suffix SUFFIX]
> +                           [--persistent][--suffix SUFFIX][CPUS]
> 
> -       Configure binfmt_misc to use qemu interpreter
> +       Configure binfmt_misc to use qemu interpreter for the given CPUS.
> +       Supported formats for CPUS are: single arch or comma/space separated list.
> +       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
> +       cpus. If CPUS is 'NONE', no interpreter is configured.
> 
>         --help:        display this usage
>         --path:        set path to qemu interpreter ($QEMU_PATH)
> @@ -180,9 +213,10 @@ Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
>         --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
> +                      instead generate file for systemd-binfmt.service;
> +                      environment variable HOST_ARCH allows to override 'uname'
> +                      to generate configuration files for a different
> +                      architecture than the current one.
>         --exportdir:   define where to write configuration files
>                        (default: $SYSTEMDDIR or $DEBIANDIR)
>         --credential:  if present, credential and security tokens are
> @@ -201,14 +235,7 @@ Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd CPU]
> 
>          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.
> -
> -    where CPU is one of:
> -
> -        $qemu_target_list
> +    QEMU target list: $qemu_target_list
> 
>  EOF
>  }
> @@ -289,12 +316,22 @@ EOF
>  }
> 
>  qemu_set_binfmts() {
> +    if [ "x$1" = "xNONE" ] ; then
> +      return
> +    fi
> +
>      # probe cpu type
>      host_family=$(qemu_get_family)
> 
> +    # reduce the list of target interpreters to those given in the CLI
> +    targets="$@"
> +    if [ $# -eq 0 ] ; then
> +      targets="ALL"
> +    fi
> +    qemu_check_target_list $targets
> +
>      # register the interpreter for each cpu except for the native one
> -
> -    for cpu in ${qemu_target_list} ; do
> +    for cpu in $checked_target_list ; do
>          magic=$(eval echo \$${cpu}_magic)
>          mask=$(eval echo \$${cpu}_mask)
>          family=$(eval echo \$${cpu}_family)
> @@ -327,7 +364,7 @@ QEMU_SUFFIX="${QEMU_SUFFIX:-}"
>  QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
>  QEMU_PERSISTENT="${QEMU_PERSISTENT:-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 -- "$@")

I think you don't need to add the ":" at the beginning of the list.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 7/10] qemu-binfmt-conf.sh: add option --reset <ARCHS>

[Laurent Vivier]

On 06/03/2019 05:52, Unai Martinez-Corral wrote:
> This is a partial implementation.
> 
> Allows to remove a single or a list of already registered binfmt
> interpreters. If <ARCHS> is a list, it must be comma-separated.

I think ARCHS and CPUS are the same thing, so use the same word.

> Valid values are those in qemu_target_list. If <ARCHS> is 'ALL', all
> the existing 'qemu-*' interpreters are removed.
> 
> This is partial because 'debian' and 'systemd' configurations are not
> removed. If option 'reset' is provided before any of those, reset is
> executed first and the configuration proceeds. However, if 'reset' is
> provided after any of them, the script will exit with error 'option
> reset 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>
> ---
>  scripts/qemu-binfmt-conf.sh | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 2751363089..824e3c4c34 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -197,8 +197,8 @@ qemu_get_family() {
> 
>  usage() {
>      cat <<EOF
> -Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd]
> -                           [--help][--credential][--exportdir PATH]
> +Usage: qemu-binfmt-conf.sh [--help][--path PATH][--debian][--systemd]
> +                           [--reset ARCHS][--credential][--exportdir PATH]
>                             [--persistent][--suffix SUFFIX][CPUS]

I don't like the idea of being able to set and reset different
interpretes with same command line call.

Make "--reset" exclusive without parameter to be able to use the common
cpu list at the end of the command line (this way, I think you would be
able to share more common code between setting an resetting).

You could have a look to Alex Bennée's patch:

  https://patchwork.ozlabs.org/patch/938796/

I think this is the good way to do this and it should merge easily with
your series.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 8/10] qemu-binfmt-conf.sh: refactor usage()

[Laurent Vivier]

On 06/03/2019 05:53, Unai Martinez-Corral wrote:
> Reorder how the options are presented to the user. Move 'systemd'
> and 'debian' to the end, so that the latter is close to the additional
> comments and example commands about it. Make the order in the prototype
> consistent with the list where each option is explained.
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 824e3c4c34..4a45636b53 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -197,9 +197,9 @@ qemu_get_family() {
> 
>  usage() {
>      cat <<EOF
> -Usage: qemu-binfmt-conf.sh [--help][--path PATH][--debian][--systemd]
> -                           [--reset ARCHS][--credential][--exportdir PATH]
> -                           [--persistent][--suffix SUFFIX][CPUS]
> +Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
> +                           [--persistent][--credential][--exportdir PATH]
> +                           [--reset ARCHS][--systemd][--debian][CPUS]
> 
>         Configure binfmt_misc to use qemu interpreter for the given CPUS.
>         Supported formats for CPUS are: single arch or comma/space separated list.
> @@ -210,25 +210,25 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--debian][--systemd]
>         --path:        set path to qemu interpreter ($QEMU_PATH)
>         --suffix:      add a suffix to the default interpreter name
>                        ($QEMU_SUFFIX)
> -       --debian:      don't write into /proc,
> -                      instead generate update-binfmts templates
> -       --systemd:     don't write into /proc,
> -                      instead generate file for systemd-binfmt.service;
> -                      environment variable HOST_ARCH allows to override 'uname'
> -                      to generate configuration files for a different
> -                      architecture than the current one.
> +       --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)
> +       --credential:  if present, credential and security tokens are
> +                      calculated according to the binary to interpret
> +                      ($QEMU_CREDENTIAL=yes)
>         --exportdir:   define where to write configuration files
>                        (default: $SYSTEMDDIR or $DEBIANDIR)
>         --reset:       remove registered interpreter for target ARCHS (comma
>                        separated list). If ARCHS is 'ALL', remove all registered
>                        'qemu-*' interpreters.
> -       --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)
> +       --systemd:     don't write into /proc,
> +                      instead generate file for systemd-binfmt.service;
> +                      environment variable HOST_ARCH allows to override 'uname'
> +                      to generate configuration files for a different
> +                      architecture than the current one.
> +       --debian:      don't write into /proc,
> +                      instead generate update-binfmts templates
> 
>      To import templates with update-binfmts, use :
> 

Please, use the same presentation format as for qemu-XXX ("Argument
       Env-variable      Description")

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 9/10] qemu-binfmt-conf.sh: update usage()

[Laurent Vivier]

On 06/03/2019 05:54, Unai Martinez-Corral wrote:
> Reduce indentation to better use available space.
> 
> Add list of supported environment variables and their default values.
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 61 ++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 4a45636b53..8ca2647ed0 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -201,34 +201,31 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
>                             [--persistent][--credential][--exportdir PATH]
>                             [--reset ARCHS][--systemd][--debian][CPUS]
> 
> -       Configure binfmt_misc to use qemu interpreter for the given CPUS.
> -       Supported formats for CPUS are: single arch or comma/space separated list.
> -       See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
> -       cpus. If CPUS is 'NONE', no interpreter is configured.
> +    Configure binfmt_misc to use qemu interpreter for the given CPUS.
> +    Supported formats for CPUS are: single arch or comma/space separated list.
> +    See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
> +    cpus. If CPUS is 'NONE', no interpreter is configured.
> 
> -       --help:        display this usage
> -       --path:        set path to qemu interpreter ($QEMU_PATH)
> -       --suffix:      add a suffix to the default interpreter name
> -                      ($QEMU_SUFFIX)
> -       --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)
> -       --credential:  if present, credential and security tokens are
> -                      calculated according to the binary to interpret
> -                      ($QEMU_CREDENTIAL=yes)
> -       --exportdir:   define where to write configuration files
> -                      (default: $SYSTEMDDIR or $DEBIANDIR)
> -       --reset:       remove registered interpreter for target ARCHS (comma
> -                      separated list). If ARCHS is 'ALL', remove all registered
> -                      'qemu-*' interpreters.
> -       --systemd:     don't write into /proc,
> -                      instead generate file for systemd-binfmt.service;
> -                      environment variable HOST_ARCH allows to override 'uname'
> -                      to generate configuration files for a different
> -                      architecture than the current one.
> -       --debian:      don't write into /proc,
> -                      instead generate update-binfmts templates
> +    --help:        display this usage.
> +    --path:        set path to qemu interpreter.
> +    --suffix:      add a suffix to the default interpreter name.
> +    --persistent:  if present, the interpreter is loaded when binfmt is
> +                   configured and remains in memory. All future uses
> +                   are cloned from the open file.
> +    --credential:  if present, credential and security tokens are
> +                   calculated according to the binary to interpret.
> +    --exportdir:   define where to write configuration files.
> +                   (default: $SYSTEMDDIR or $DEBIANDIR)
> +    --reset:       remove registered interpreter for target ARCHS (comma
> +                   separated list). If ARCHS is 'ALL', remove all registered
> +                   'qemu-*' interpreters.
> +    --systemd:     don't write into /proc,
> +                   instead generate file(s) for systemd-binfmt.service;
> +                   environment variable HOST_ARCH allows to override 'uname'
> +                   to generate configuration files for a different
> +                   architecture than the current one.
> +    --debian:      don't write into /proc,
> +                   instead generate update-binfmts templates.
> 
>      To import templates with update-binfmts, use :
> 
> @@ -240,6 +237,14 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
> 
>      QEMU target list: $qemu_target_list
> 
> +    Options 'path, 'suffix', 'persistent' and 'credential' are also supported
> +    through environment variables. Defaults are:
> +
> +      QEMU_PATH=/usr/local/bin

You can write

        QEMU_PATH=$QEMU_PATH

to have the actual value.

> +      QEMU_SUFFIX=
> +      QEMU_PERSISTENT=no
> +      QEMU_CREDENTIAL=no

and
         QEMU_SUFFIX=$QEMU_SUFFIX
         QEMU_PERSISTENT=$QEMU_PERSISTENT
         QEMU_CREDENTIAL=$QEMU_CREDENTIAL=

> +
>  EOF
>  }
> 
> @@ -384,8 +389,8 @@ DEBIANDIR="/usr/share/binfmts"
> 
>  QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
>  QEMU_SUFFIX="${QEMU_SUFFIX:-}"
> -QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
>  QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
> +QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
> 
>  options=$(getopt -o r:dsQ:S:e:hcp -l reset:,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
>  eval set -- "$options"


I think you can merge this patch with the previous one.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 10/10] qemu-binfmt-conf.sh: support QEMU_TARGETS

[Laurent Vivier]

On 06/03/2019 05:55, Unai Martinez-Corral wrote:
> Rename CPUS to TARGETS, and support QEMU_TARGETS environment variable.
> This does not break backward compatibility, because it is just a placeholder.
> 
> Consistently with 'path', 'suffix', 'persistent' and 'credential',
> provide an environment variable to set the list of target architectures.
> 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 'ALL'.
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 8ca2647ed0..0676f86512 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -6,7 +6,7 @@ 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 target CPUS is/are in the supported target list
> +# check if given TARGETS is/are in the supported target list
>  qemu_check_target_list() {
>      all="$qemu_target_list"
>      if [ "x$1" = "xALL" ] ; then
> @@ -199,12 +199,12 @@ usage() {
>      cat <<EOF
>  Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
>                             [--persistent][--credential][--exportdir PATH]
> -                           [--reset ARCHS][--systemd][--debian][CPUS]
> +                           [--reset ARCHS][--systemd][--debian][TARGETS]
> 
> -    Configure binfmt_misc to use qemu interpreter for the given CPUS.
> -    Supported formats for CPUS are: single arch or comma/space separated list.
> -    See QEMU target list below. If CPUS is 'ALL' or empty, configure all known
> -    cpus. If CPUS is 'NONE', no interpreter is configured.
> +    Configure binfmt_misc to use qemu interpreter for the given TARGETS.
> +    Supported formats for TARGETS are: single arch or comma/space separated list.
> +    See QEMU target list below. If TARGETS is 'ALL' or empty, configure all known
> +    TARGETS. If TARGETS is 'NONE', no interpreter is configured.
> 
>      --help:        display this usage.
>      --path:        set path to qemu interpreter.
> @@ -244,6 +244,7 @@ Usage: qemu-binfmt-conf.sh [--help][--path PATH][--suffix SUFFIX]
>        QEMU_SUFFIX=
>        QEMU_PERSISTENT=no
>        QEMU_CREDENTIAL=no
> +      QEMU_TARGETS=

         QEMU_TARGETS=$QEMU_TARGETS

> 
>  EOF
>  }
> @@ -324,17 +325,17 @@ EOF
>  }
> 
>  qemu_set_binfmts() {
> -    if [ "x$1" = "xNONE" ] ; then
> -      return
> -    fi
> -
>      # probe cpu type
>      host_family=$(qemu_get_family)
> 
>      # reduce the list of target interpreters to those given in the CLI
> -    targets="$@"
>      if [ $# -eq 0 ] ; then
> -      targets="ALL"
> +      targets="${QEMU_TARGETS:-ALL}"
> +    else
> +      if [ "x$1" = "xNONE" ] ; then

In fact, I think it would be cleaner to have a "--dry-run" option
instead (something setting BINFMT_SET to ':' I think)

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 6/10] qemu-binfmt-conf.sh: generalize <CPU> to positional <CPUS>

[Unai Martinez Corral]

2019/3/10 18:02, Laurent Vivier:
> On 06/03/2019 05:50, Unai Martinez-Corral wrote:
> > The list can be separated by spaces, tabs, newlines or commas. If no
>
> I think it would be simpler to not manage commas, and to use the default
> field separator.

Commas were supported because the same function is used to parse CPUS
here and ARCHS in patch 7/10. ARCHS must be separated by commas.
Nonetheless, if we do not support CPUS and ARCHS in a single command,
we can avoid managing commas.

Agree and thanks for all of the remaining comments. I will apply them:

> Why do you need to introduce a new variable "all", you can use directly
> "qemu_target_list".

> No need to introduce a new variable, you can use directly
> checked_target_list

> Use directly "qemu_target_list"
> You don't need a new variable, you can check "x$cpu" = "x$target" to
> know if you have found the target.

> I think you don't need to add the ":" at the beginning of the list.

Regards,
Unai

Re: [Qemu-devel] [PATCH v3 7/10] qemu-binfmt-conf.sh: add option --reset <ARCHS>

[Unai Martinez Corral]

2019/3/10 18:15, Laurent Vivier:
> On 06/03/2019 05:52, Unai Martinez-Corral wrote:
> > Allows to remove a single or a list of already registered binfmt
> > interpreters. If <ARCHS> is a list, it must be comma-separated.
>
> I think ARCHS and CPUS are the same thing, so use the same word.

They are similar, but the syntax is not the same. ARCHS supports a
single word or a comma separated list. CPUS supports also using
spaces, tabs, etc. as separators.

> > -Usage: qemu-binfmt-conf.sh [--path PATH][--debian][--systemd]
> > -                           [--help][--credential][--exportdir PATH]
> > +Usage: qemu-binfmt-conf.sh [--help][--path PATH][--debian][--systemd]
> > +                           [--reset ARCHS][--credential][--exportdir PATH]
> >                             [--persistent][--suffix SUFFIX][CPUS]
>
> I don't like the idea of being able to set and reset different
> interpretes with same command line call.

> Make "--reset" exclusive without parameter to be able to use the common
> cpu list at the end of the command line

It is useful in order to replace a single interpreter or a few of them
in a single call to the script. Anyway, I think that your comment
makes sense. It is enough if we support to reset a subset. Any user
can then wrap two consecutive calls in a helper function.

So, in the next version '--reset' will be 'exclusive', i.e., it will
prevent registration.

> (this way, I think you would be
> able to share more common code between setting an resetting).

The shared code is the same, because 'qemu_check_target_list' was
already being used in both. The simplification comes from the fact
that we do not support commas anymore, since ARCHS and CPUS are now
only positional.

> You could have a look to Alex Bennée's patch:
>
>   https://patchwork.ozlabs.org/patch/938796/
>
> I think this is the good way to do this and it should merge easily with
> your series.

Thanks for the reference; I had skipped it. Upon further inspection, I
think that the features proposed by Alex are already implemented in
this patch, or they will be in the next version:

- I am already checking with '$CHECK', which will use the
corresponding procedure depending on 'systemd' or 'debian'.
- I am using printf instead of 'echo -1', as suggested by Eric Blake
in v1 of the patch.
- I will rename the option from '--reset' to '--clear', as I find it
to be more descriptive.
- None of us support neither '--debian' nor '--systemd'. See comments below.
- I support providing a subset of targets, while Alex' approach
targeted all of them.
- Overall, I think that 'BINFMT_SET=qemu_clear_interpreter' is ok for
a proof of concept, but it is not as clean as it could be. Precisely,
all the magic, mask, family values are computed in the loop inside
qemu_set_binfmts. That should be avoided if we only want to remove
them. Nonetheless, I will try to integrate both approaches.

Regarding this comment:

> I think it would be also useful to remove the files from $EXPORTDIR.
> So could you also manage something like "--debian --clear" and "--systemd CPU --clear"?

ATM, this patchset supports '--reset --debian' or '--reset --systemd',
but it will proceed by updating /proc directly, which might not be
effective at all.
Furthermore, when '--debian --reset' or '--systemd --reset' are used,
a error message is shown: 'ERROR: option reset not implemented for
this mode yet'.

On the one hand, I can make '--reset --debian|systemd' and
'--debian|systemd --reset' consistent, by delaying the execution of
the reset/clear function after all the options are parsed.

On the other hand, I'd really like to have patches 6 and 7 available
in QEMU 4.0, if possible. So, I'd like to know if the partial
implementation of this feature is a stopper. If so, I'd be so glad to
have some help. These are the prototypes that I have not added to the
patchset because I am not sure about them:

For the debian case, after the comments in 'usage()', I think that the
procedure should be:

for t in $qemu_check_target_list ; do
  sudo update-binfmts --package "qemu-$t" --remove "qemu-$t" "$QEMU_PATH"
  rm "$EXPORTDIR/qemu-$t"
done

And for systemd:

sudo systemctl stop systemd-binfmt.service
for t in $qemu_check_target_list ; do
   rm -rf $EXPORTDIR/qemu-$t.conf"
do
sudo systemctl start systemd-binfmt.service

Thanks,
Unai