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

[Qemu-devel] [PATCH v4 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.
* Option '--clear' is added, which allows to remove an already registered
  target interpreter or a list of them. The implementation is functional but
  partial. Please, see the corresponding commit.
* Option '--test|--dry-run' is added, which allows to execute the CHECK according
  to the provided arguments, but no interpreter is configured.
* Support to set options through environment variables: QEMU_TARGETS,
  QEMU_PATH, QEMU_SUFFIX, QEMU_PERSISTENT, QEMU_CREDENTIAL, QEMU_CLEAR and
  QEMU_TEST.

The following changes are not backward compatible:

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

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

The main differences compared to v3 are:

* Suggestions by Laurent Vivier regarding syntax and usage of redundant variables are
  addressed.
* The same presentation format as for qemu-XXXX ("Argument Env-variable Description")
  is used and defaults for envvars are shown.
* '--reset' (now '--clear') is exclusive, i.e., the list of positional arguments is used,
  and no registration function is executed.
* Commas are no longer accepted as separators for TARGETS.
* The order of some patches is reorganized and some are squashed:
  * v4 4/10 is new
  * v3 4/10 -> v4 5/10
  * v3 5/10 -> v4 6/10
  * v3 6/10 and 10/10 --> v4 7/10
  * v3 7/10 -> v4 8/10
  * v3 8/10 and v3 9/10 --> v4 9/10
  * v4 10/10 is new

Based on:

* [PATCH v3 0/10] qemu-binfmt-conf.sh
* [PATCH v2] qemu-binfmt-conf.sh: add CPUS, add --reset, make -p and -c boolean (no arg)
* [PATCH] qemu-binfmt-conf.sh: add CPUS, add --reset, make -p and -c boolean (no arg)

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: use the same presentation format as for qemu-*
      qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
      qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
      qemu-binfmt-conf.sh: generalize <CPU> to positional [TARGETS]
      qemu-binfmt-conf.sh: add option --clear
      qemu-binfmt-conf.sh: update usage()
      qemu-binfmt-conf.sh: add --test|--dry-run

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

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 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.21.0

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 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.21.0

[Qemu-devel] [PATCH v4 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.21.0

[Qemu-devel] [PATCH v4 4/10] qemu-binfmt-conf.sh: use the same presentation format as for qemu-*

[Unai Martinez-Corral]

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

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index e7a714e22c..489d7d2326 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -167,47 +167,43 @@ qemu_get_family() {

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

-       Configure binfmt_misc to use qemu interpreter
+Configure binfmt_misc to use qemu interpreter

-       --help:        display this usage
-       --qemu-path:   set path to qemu interpreter ($QEMU_PATH)
-       --qemu-suffix: add a suffix to the default interpreter name
-       --debian:      don't write into /proc,
-                      instead generate update-binfmts templates
-       --systemd:     don't write into /proc,
-                      instead generate file for systemd-binfmt.service
-                      for the given CPU. If CPU is "ALL", generate a
-                      file for all known cpus
-       --exportdir:   define where to write configuration files
-                      (default: $SYSTEMDDIR or $DEBIANDIR)
-       --credential:  if present, credential and security tokens are
-                      calculated according to the binary to interpret
-                      ($QEMU_CREDENTIAL=yes)
-       --persistent:  if present, the interpreter is loaded when binfmt is
-                      configured and remains in memory. All future uses
-                      are cloned from the open file.
-                      ($QEMU_PERSISTENT=yes)
+Options and associated environment variables:

-    To import templates with update-binfmts, use :
+Argument             Env-variable     Description
+-h|--help                             display this usage
+-Q|--qemu-path PATH: QEMU_PATH        set path to qemu interpreter
+-F|--qemu-suffix SUFFIX:              add a suffix to the default interpreter name
+-d|--debian:                          don't write into /proc, generate update-binfmts templates
+-s|--systemd CPU:                     don't write into /proc, generate file for
+                                      systemd-binfmt.service for the given CPU; if CPU is "ALL",
+                                      generate a file for all known cpus.
+-e|--exportdir PATH: DEBIANDIR        define where to write configuration files
+                     SYSTEMDDIR
+-c|--credential:     QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
+                                      to the binary to interpret
+-p|--persistent:     QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
+                                      uses are cloned from the open file.

-        sudo update-binfmts --importdir ${EXPORTDIR:-$DEBIANDIR} --import qemu-CPU
+To import templates with update-binfmts, use :

-    To remove interpreter, use :
+    sudo update-binfmts --importdir ${EXPORTDIR:-$DEBIANDIR} --import qemu-CPU

-        sudo update-binfmts --package qemu-CPU --remove qemu-CPU $QEMU_PATH
+To remove interpreter, use :

-    With systemd, binfmt files are loaded by systemd-binfmt.service
+    sudo update-binfmts --package qemu-CPU --remove qemu-CPU $QEMU_PATH

-    The environment variable HOST_ARCH allows to override 'uname' to generate
-    configuration files for a different architecture than the current one.
+With systemd, binfmt files are loaded by systemd-binfmt.service

-    where CPU is one of:
+The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for a
+different architecture than the current one.

-        $qemu_target_list
+where CPU is one of:
+
+    $qemu_target_list

 EOF
 }
--
2.21.0

[Qemu-devel] [PATCH v4 5/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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 scripts/qemu-binfmt-conf.sh | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 489d7d2326..0918f7fba6 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -167,6 +167,7 @@ qemu_get_family() {

 usage() {
     cat <<EOF
+
 Usage: qemu-binfmt-conf.sh [options]

 Configure binfmt_misc to use qemu interpreter
@@ -175,8 +176,8 @@ Options and associated environment variables:

 Argument             Env-variable     Description
 -h|--help                             display this usage
--Q|--qemu-path PATH: QEMU_PATH        set path to qemu interpreter
--F|--qemu-suffix SUFFIX:              add a suffix to the default interpreter name
+-Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
+-F|--suffix SUFFIX:                   add a suffix to the default interpreter name
 -d|--debian:                          don't write into /proc, generate update-binfmts templates
 -s|--systemd CPU:                     don't write into /proc, generate file for
                                       systemd-binfmt.service for the given CPU; if CPU is "ALL",
@@ -324,7 +325,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
@@ -356,11 +357,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.21.0

[Qemu-devel] [PATCH v4 6/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 | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 0918f7fba6..13e619794c 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -177,7 +177,7 @@ Options and associated environment variables:
 Argument             Env-variable     Description
 -h|--help                             display this usage
 -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
--F|--suffix SUFFIX:                   add a suffix to the default interpreter name
+-F|--suffix SUFFIX:  QEMU_SUFFIX      add a suffix to the default interpreter name
 -d|--debian:                          don't write into /proc, generate update-binfmts templates
 -s|--systemd CPU:                     don't write into /proc, generate file for
                                       systemd-binfmt.service for the given CPU; if CPU is "ALL",
@@ -318,13 +318,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.21.0

[Qemu-devel] [PATCH v4 7/10] qemu-binfmt-conf.sh: generalize CPU to positional TARGETS

[Unai Martinez-Corral]

This breaks brackward compatibility.

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

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

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

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

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 13e619794c..fde78517ff 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -6,6 +6,27 @@ mips mipsel mipsn32 mipsn32el mips64 mips64el \
 sh4 sh4eb s390x aarch64 aarch64_be hppa riscv32 riscv64 xtensa xtensaeb \
 microblaze microblazeel or1k x86_64"

+# check if given TARGETS is/are in the supported target list
+qemu_check_target_list() {
+    if [ $# -eq 0 ] ; then
+      checked_target_list="$qemu_target_list"
+      return
+    fi
+    for target ; do
+        for cpu in $qemu_target_list ; do
+            if [ "x$cpu" = "x$target" ] ; then
+                checked_target_list="$checked_target_list $target"
+                break
+            fi
+        done
+        if [ "$unknown_target" = "true" ] ; then
+            echo "ERROR: unknown CPU \"$target\"" 1>&2
+            usage
+            exit 1
+        fi
+    done
+}
+
 i386_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00'
 i386_mask='\xff\xff\xff\xff\xff\xfe\xfe\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'
 i386_family=i386
@@ -167,21 +188,25 @@ qemu_get_family() {

 usage() {
     cat <<EOF
+Usage: qemu-binfmt-conf.sh [options] [TARGETS]

-Usage: qemu-binfmt-conf.sh [options]
-
-Configure binfmt_misc to use qemu interpreter
+Configure binfmt_misc to use qemu interpreter for the given TARGETS.

 Options and associated environment variables:

 Argument             Env-variable     Description
+TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
+                                      if empty, configure all known targets;
+                                      if 'NONE', no interpreter is configured.
 -h|--help                             display this usage
 -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
 -F|--suffix SUFFIX:  QEMU_SUFFIX      add a suffix to the default interpreter name
 -d|--debian:                          don't write into /proc, generate update-binfmts templates
--s|--systemd CPU:                     don't write into /proc, generate file for
-                                      systemd-binfmt.service for the given CPU; if CPU is "ALL",
-                                      generate a file for all known cpus.
+-s|--systemd:                         don't write into /proc, generate file(s) for
+                                      systemd-binfmt.service; environment variable HOST_ARCH
+                                      allows to override 'uname' to generate configuration files
+                                      for a different architecture than the current one.
+
 -e|--exportdir PATH: DEBIANDIR        define where to write configuration files
                      SYSTEMDDIR
 -c|--credential:     QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
@@ -197,14 +222,7 @@ To remove interpreter, use :

     sudo update-binfmts --package qemu-CPU --remove qemu-CPU $QEMU_PATH

-With systemd, binfmt files are loaded by systemd-binfmt.service
-
-The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for a
-different architecture than the current one.
-
-where CPU is one of:
-
-    $qemu_target_list
+QEMU target list: $qemu_target_list

 EOF
 }
@@ -288,9 +306,15 @@ qemu_set_binfmts() {
     # probe cpu type
     host_family=$(qemu_get_family)

-    # register the interpreter for each cpu except for the native one
+    # reduce the list of target interpreters to those given in the CLI
+    [ $# -eq 0 ] && targets="${QEMU_TARGETS:-}" || targets="$@"
+    if [ "x$targets" = "xNONE" ] ; then
+      return
+    fi
+    qemu_check_target_list $targets

-    for cpu in ${qemu_target_list} ; do
+    # register the interpreter for each target except for the native one
+    for cpu in $checked_target_list ; do
         magic=$(eval echo \$${cpu}_magic)
         mask=$(eval echo \$${cpu}_mask)
         family=$(eval echo \$${cpu}_family)
@@ -318,12 +342,13 @@ BINFMT_SET=qemu_register_interpreter
 SYSTEMDDIR="/etc/binfmt.d"
 DEBIANDIR="/usr/share/binfmts"

+QEMU_TARGETS="${QEMU_TARGETS:-}"
 QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
 QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_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
@@ -337,23 +362,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
@@ -384,5 +392,7 @@ while true ; do
     shift
 done

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

[Qemu-devel] [PATCH v4 8/10] qemu-binfmt-conf.sh: add option --clear

[Unai Martinez-Corral]

This is a partial implementation.

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

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

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

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

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index fde78517ff..07d1ee1f04 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -196,7 +196,7 @@ Options and associated environment variables:

 Argument             Env-variable     Description
 TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
-                                      if empty, configure all known targets;
+                                      if empty, configure/clear all known targets;
                                       if 'NONE', no interpreter is configured.
 -h|--help                             display this usage
 -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
@@ -206,9 +206,10 @@ TARGETS              QEMU_TARGETS     A single arch name or a list of them (see
                                       systemd-binfmt.service; environment variable HOST_ARCH
                                       allows to override 'uname' to generate configuration files
                                       for a different architecture than the current one.
-
 -e|--exportdir PATH: DEBIANDIR        define where to write configuration files
                      SYSTEMDDIR
+-c|--clear:          QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;
+                                      then exit.
 -c|--credential:     QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
                                       to the binary to interpret
 -p|--persistent:     QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
@@ -336,6 +337,25 @@ qemu_set_binfmts() {
     done
 }

+qemu_clear_notimplemented() {
+    echo "ERROR: option clear not implemented for this mode yet" 1>&2
+    usage
+    exit 1
+}
+
+qemu_clear_interpreter() {
+    names='qemu-*'
+    if [ $# -ne 0 ] ; 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

@@ -347,12 +367,16 @@ QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
 QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
+QEMU_CLEAR="${QEMU_CLEAR:-no}"

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

 while true ; do
     case "$1" in
+    -c|--clear)
+        QEMU_CLEAR="yes"
+        ;;
     -d|--debian)
         CHECK=qemu_check_debian
         BINFMT_SET=qemu_generate_debian
@@ -395,4 +419,15 @@ done
 shift

 $CHECK
+
+if [ "x$QEMU_CLEAR" = "xyes" ] ; then
+  case "$BINFMT_SET" in
+    *debian)  BINFMT_CLEAR=qemu_clear_notimplemented ;;
+    *systemd) BINFMT_CLEAR=qemu_clear_notimplemented ;;
+    *)        BINFMT_CLEAR=qemu_clear_interpreter
+  esac
+  $BINFMT_CLEAR "$@"
+  exit
+fi
+
 qemu_set_binfmts "$@"
--
2.21.0

[Qemu-devel] [PATCH v4 9/10] qemu-binfmt-conf.sh: update 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.

Add list of default values for environment variables.

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

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 07d1ee1f04..a516181a3a 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -201,19 +201,27 @@ TARGETS              QEMU_TARGETS     A single arch name or a list of them (see
 -h|--help                             display this usage
 -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
 -F|--suffix SUFFIX:  QEMU_SUFFIX      add a suffix to the default interpreter name
--d|--debian:                          don't write into /proc, generate update-binfmts templates
--s|--systemd:                         don't write into /proc, 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.
+-p|--persistent:     QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
+                                      uses are cloned from the open file.
+-c|--credential:     QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
+                                      to the binary to interpret
 -e|--exportdir PATH: DEBIANDIR        define where to write configuration files
                      SYSTEMDDIR
 -c|--clear:          QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;
                                       then exit.
--c|--credential:     QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
-                                      to the binary to interpret
--p|--persistent:     QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
-                                      uses are cloned from the open file.
+-s|--systemd:                         don't write into /proc, 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.
+-d|--debian:                          don't write into /proc, generate update-binfmts templates
+
+Defaults:
+QEMU_TARGETS=$QEMU_TARGETS
+QEMU_PATH=$QEMU_PATH
+QEMU_SUFFIX=$QEMU_SUFFIX
+QEMU_PERSISTENT=$QEMU_PERSISTENT
+QEMU_CREDENTIAL=$QEMU_CREDENTIAL
+QEMU_CLEAR=$QEMU_CLEAR

 To import templates with update-binfmts, use :

@@ -365,8 +373,8 @@ DEBIANDIR="/usr/share/binfmts"
 QEMU_TARGETS="${QEMU_TARGETS:-}"
 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}"
 QEMU_CLEAR="${QEMU_CLEAR:-no}"

 options=$(getopt -o cdsQ:S:e:hcp -l clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
--
2.21.0

[Qemu-devel] [PATCH v4 10/10] qemu-binfmt-conf.sh: add --test|--dry-run

[Unai Martinez-Corral]

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

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index a516181a3a..db85798e76 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -196,8 +196,7 @@ Options and associated environment variables:

 Argument             Env-variable     Description
 TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
-                                      if empty, configure/clear all known targets;
-                                      if 'NONE', no interpreter is configured.
+                                      if empty, configure/clear all known targets.
 -h|--help                             display this usage
 -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
 -F|--suffix SUFFIX:  QEMU_SUFFIX      add a suffix to the default interpreter name
@@ -209,6 +208,8 @@ TARGETS              QEMU_TARGETS     A single arch name or a list of them (see
                      SYSTEMDDIR
 -c|--clear:          QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;
                                       then exit.
+-t|--test|--dry-run: QEMU_TEST        (yes) test the setup with the provided arguments, but do not
+                                      configure any of the interpreters.
 -s|--systemd:                         don't write into /proc, generate file(s) for
                                       systemd-binfmt.service; environment variable HOST_ARCH
                                       allows to override 'uname' to generate configuration files
@@ -222,6 +223,7 @@ QEMU_SUFFIX=$QEMU_SUFFIX
 QEMU_PERSISTENT=$QEMU_PERSISTENT
 QEMU_CREDENTIAL=$QEMU_CREDENTIAL
 QEMU_CLEAR=$QEMU_CLEAR
+QEMU_TEST=$QEMU_TEST

 To import templates with update-binfmts, use :

@@ -317,9 +319,6 @@ qemu_set_binfmts() {

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

     # register the interpreter for each target except for the native one
@@ -376,12 +375,16 @@ QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_CLEAR="${QEMU_CLEAR:-no}"
+QEMU_TEST="${QEMU_TEST:-no}"

-options=$(getopt -o cdsQ:S:e:hcp -l clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
+options=$(getopt -o tcdsQ:S:e:hcp -l test,dry-runclear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
 eval set -- "$options"

 while true ; do
     case "$1" in
+    -t|--test|--dry-run)
+      QEMU_TEST="yes"
+    ;;
     -c|--clear)
         QEMU_CLEAR="yes"
         ;;
@@ -438,4 +441,10 @@ if [ "x$QEMU_CLEAR" = "xyes" ] ; then
   exit
 fi

+if [ "x$QEMU_TEST" = "xyes" ] ; then
+    skip(){
+    :;}
+    BINFMT_SET=skip
+fi
+
 qemu_set_binfmts "$@"
--
2.21.0

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

[Laurent Vivier]

On 11/03/2019 11:26, 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.
> 
> 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.21.0
> 
Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH v4 10/10] qemu-binfmt-conf.sh: add --test|--dry-run

[Laurent Vivier]

On 11/03/2019 11:32, Unai Martinez-Corral wrote:
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index a516181a3a..db85798e76 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -196,8 +196,7 @@ Options and associated environment variables:
> 
>  Argument             Env-variable     Description
>  TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
> -                                      if empty, configure/clear all known targets;
> -                                      if 'NONE', no interpreter is configured.
> +                                      if empty, configure/clear all known targets.
>  -h|--help                             display this usage
>  -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
>  -F|--suffix SUFFIX:  QEMU_SUFFIX      add a suffix to the default interpreter name
> @@ -209,6 +208,8 @@ TARGETS              QEMU_TARGETS     A single arch name or a list of them (see
>                       SYSTEMDDIR
>  -c|--clear:          QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;
>                                        then exit.
> +-t|--test|--dry-run: QEMU_TEST        (yes) test the setup with the provided arguments, but do not
> +                                      configure any of the interpreters.
>  -s|--systemd:                         don't write into /proc, generate file(s) for
>                                        systemd-binfmt.service; environment variable HOST_ARCH
>                                        allows to override 'uname' to generate configuration files
> @@ -222,6 +223,7 @@ QEMU_SUFFIX=$QEMU_SUFFIX
>  QEMU_PERSISTENT=$QEMU_PERSISTENT
>  QEMU_CREDENTIAL=$QEMU_CREDENTIAL
>  QEMU_CLEAR=$QEMU_CLEAR
> +QEMU_TEST=$QEMU_TEST
> 
>  To import templates with update-binfmts, use :
> 
> @@ -317,9 +319,6 @@ qemu_set_binfmts() {
> 
>      # reduce the list of target interpreters to those given in the CLI
>      [ $# -eq 0 ] && targets="${QEMU_TARGETS:-}" || targets="$@"
> -    if [ "x$targets" = "xNONE" ] ; then
> -      return
> -    fi
>      qemu_check_target_list $targets
> 
>      # register the interpreter for each target except for the native one
> @@ -376,12 +375,16 @@ QEMU_SUFFIX="${QEMU_SUFFIX:-}"
>  QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
>  QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
>  QEMU_CLEAR="${QEMU_CLEAR:-no}"
> +QEMU_TEST="${QEMU_TEST:-no}"
> 
> -options=$(getopt -o cdsQ:S:e:hcp -l clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
> +options=$(getopt -o tcdsQ:S:e:hcp -l test,dry-runclear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")

missing comma here.

>  eval set -- "$options"
> 
>  while true ; do
>      case "$1" in
> +    -t|--test|--dry-run)

We don't need multiple parameter for the same effect. You must choose
between --test and --dry-run.

> +      QEMU_TEST="yes"
> +    ;;
>      -c|--clear)
>          QEMU_CLEAR="yes"
>          ;;
> @@ -438,4 +441,10 @@ if [ "x$QEMU_CLEAR" = "xyes" ] ; then
>    exit
>  fi
> 
> +if [ "x$QEMU_TEST" = "xyes" ] ; then
> +    skip(){
> +    :;}
> +    BINFMT_SET=skip

Why do you need a function?
Can't you set directly BINFMT_SET to ':'?

Thanks,
Laurent

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190311101428.GA11@765644dd90e5/



Hi,

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

Type: series
Message-id: 20190311101428.GA11@765644dd90e5
Subject: [Qemu-devel] [PATCH v4 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
Switched to a new branch 'test'
d8be11fa48 qemu-binfmt-conf.sh: add --test|--dry-run
eed2ee7cc9 qemu-binfmt-conf.sh: update usage()
265805545f qemu-binfmt-conf.sh: add option --clear
21298477f7 qemu-binfmt-conf.sh: generalize CPU to positional TARGETS
2b8acd4d8d qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
35c5a1c00a qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
536212c59a qemu-binfmt-conf.sh: use the same presentation format as for qemu-*
3c5495df43 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
e4a74a66df qemu-binfmt-conf.sh: make opts -p and -c boolean
c70398d854 qemu-binfmt-conf.sh: enforce safe style consistency

=== OUTPUT BEGIN ===
1/10 Checking commit c70398d85463 (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 e4a74a66df91 (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 3c5495df43d5 (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT)
4/10 Checking commit 536212c59aff (qemu-binfmt-conf.sh: use the same presentation format as for qemu-*)
WARNING: line over 80 characters
#50: FILE: scripts/qemu-binfmt-conf.sh:179:
+-F|--qemu-suffix SUFFIX:              add a suffix to the default interpreter name

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

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

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

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

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

total: 5 errors, 1 warnings, 74 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 35c5a1c00aed (qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options)
WARNING: line over 80 characters
#36: FILE: scripts/qemu-binfmt-conf.sh:180:
+-F|--suffix SUFFIX:                   add a suffix to the default interpreter name

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

total: 1 errors, 1 warnings, 38 lines checked

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

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

total: 0 errors, 1 warnings, 23 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 21298477f72e (qemu-binfmt-conf.sh: generalize CPU to positional TARGETS)
ERROR: line over 90 characters
#76: FILE: scripts/qemu-binfmt-conf.sh:198:
+TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);

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

ERROR: line over 90 characters
#87: FILE: scripts/qemu-binfmt-conf.sh:206:
+                                      systemd-binfmt.service; environment variable HOST_ARCH

ERROR: line over 90 characters
#88: FILE: scripts/qemu-binfmt-conf.sh:207:
+                                      allows to override 'uname' to generate configuration files

WARNING: line over 80 characters
#89: FILE: scripts/qemu-binfmt-conf.sh:208:
+                                      for a different architecture than the current one.

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

total: 4 errors, 2 warnings, 135 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 265805545ff8 (qemu-binfmt-conf.sh: add option --clear)
WARNING: line over 80 characters
#33: FILE: scripts/qemu-binfmt-conf.sh:199:
+                                      if empty, configure/clear all known targets;

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

WARNING: line over 80 characters
#69: FILE: scripts/qemu-binfmt-conf.sh:356:
+    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;

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

total: 2 errors, 2 warnings, 76 lines checked

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

9/10 Checking commit eed2ee7cc9c8 (qemu-binfmt-conf.sh: update usage())
ERROR: line over 90 characters
#29: FILE: scripts/qemu-binfmt-conf.sh:204:
+-p|--persistent:     QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future

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

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

ERROR: line over 90 characters
#42: FILE: scripts/qemu-binfmt-conf.sh:213:
+                                      systemd-binfmt.service; environment variable HOST_ARCH

ERROR: line over 90 characters
#43: FILE: scripts/qemu-binfmt-conf.sh:214:
+                                      allows to override 'uname' to generate configuration files

WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:215:
+                                      for a different architecture than the current one.

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

total: 5 errors, 2 warnings, 45 lines checked

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

10/10 Checking commit d8be11fa4837 (qemu-binfmt-conf.sh: add --test|--dry-run)
WARNING: line over 80 characters
#20: FILE: scripts/qemu-binfmt-conf.sh:199:
+                                      if empty, configure/clear all known targets.

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

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

total: 2 errors, 1 warnings, 60 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/20190311101428.GA11@765644dd90e5/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 v4 8/10] qemu-binfmt-conf.sh: add option --clear

[Laurent Vivier]

On 11/03/2019 11:31, Unai Martinez-Corral wrote:
> This is a partial implementation.
> 
> Allows to remove a single or a list of already registered binfmt
> interpreters. Valid values are those in qemu_target_list.
> If TARGETS is empty, all the existing 'qemu-*' interpreters are
> removed.
> 
> This is partial because 'debian' and 'systemd' configurations are not
> supported. The script will exit with error 'option clear not
> implemented for this mode yet'.
> 
> Removal is done by printing '-1' as explained at:
> https://www.kernel.org/doc/Documentation/admin-guide/binfmt-misc.rst
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 41 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index fde78517ff..07d1ee1f04 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -196,7 +196,7 @@ Options and associated environment variables:
> 
>  Argument             Env-variable     Description
>  TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
> -                                      if empty, configure all known targets;
> +                                      if empty, configure/clear all known targets;
>                                        if 'NONE', no interpreter is configured.
>  -h|--help                             display this usage
>  -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
> @@ -206,9 +206,10 @@ TARGETS              QEMU_TARGETS     A single arch name or a list of them (see
>                                        systemd-binfmt.service; environment variable HOST_ARCH
>                                        allows to override 'uname' to generate configuration files
>                                        for a different architecture than the current one.
> -
>  -e|--exportdir PATH: DEBIANDIR        define where to write configuration files
>                       SYSTEMDDIR
> +-c|--clear:          QEMU_CLEAR       (yes) remove registered interpreters for target TARGETS;
> +                                      then exit.
>  -c|--credential:     QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
>                                        to the binary to interpret
>  -p|--persistent:     QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future
> @@ -336,6 +337,25 @@ qemu_set_binfmts() {
>      done
>  }
> 
> +qemu_clear_notimplemented() {
> +    echo "ERROR: option clear not implemented for this mode yet" 1>&2
> +    usage
> +    exit 1
> +}
> +
> +qemu_clear_interpreter() {
> +    names='qemu-*'
> +    if [ $# -ne 0 ] ; 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 > {}' \;

The qemu-* will be expanded here if you have a qemu-XXX in the current
directory. You must use "$names".

But:

To remove all, you can do:
sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/status'

so something like

if [ $# -eq 0 ] ; then
  sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/status
fi
qemu_check_target_list $1
for t in $checked_target_list ; do
   sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/qemu-$t'
done

But I think you should also taking care of the suffix.

> +}
> +
>  CHECK=qemu_check_bintfmt_misc
>  BINFMT_SET=qemu_register_interpreter
> 
> @@ -347,12 +367,16 @@ QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
>  QEMU_SUFFIX="${QEMU_SUFFIX:-}"
>  QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
>  QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
> +QEMU_CLEAR="${QEMU_CLEAR:-no}"
> 
> -options=$(getopt -o dsQ:S:e:hcp -l debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
> +options=$(getopt -o cdsQ:S:e:hcp -l clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- "$@")
>  eval set -- "$options"
> 
>  while true ; do
>      case "$1" in
> +    -c|--clear)
> +        QEMU_CLEAR="yes"
> +        ;;
>      -d|--debian)
>          CHECK=qemu_check_debian
>          BINFMT_SET=qemu_generate_debian
> @@ -395,4 +419,15 @@ done
>  shift
> 
>  $CHECK
> +
> +if [ "x$QEMU_CLEAR" = "xyes" ] ; then
> +  case "$BINFMT_SET" in
> +    *debian)  BINFMT_CLEAR=qemu_clear_notimplemented ;;
> +    *systemd) BINFMT_CLEAR=qemu_clear_notimplemented ;;
> +    *)        BINFMT_CLEAR=qemu_clear_interpreter
> +  esac

Put this in the previous case for decoding options, please.

> +  $BINFMT_CLEAR "$@"
> +  exit
> +fi
> +
>  qemu_set_binfmts "$@"
> --
> 2.21.0
> 

Re: [Qemu-devel] [PATCH v4 7/10] qemu-binfmt-conf.sh: generalize CPU to positional TARGETS

[Laurent Vivier]

On 11/03/2019 11:30, 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.
> 
> If no positional arguments are provided, all of the architectures in
> qemu_target_list are registered. Conversely, argument value 'NONE'
> allows to make a 'dry run' of the script. I.e., checks are executed
> according to the mode, but no interpreter is registered.
> 
> Support QEMU_TARGETS environment variable, consistently with 'path',
> 'suffix', 'persistent' and 'credential', The supported formats are
> the same as for positional arguments, which have priority. If both
> the variable and the list of positional arguments are empty, defaults
> to qemu_target_list.
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 80 +++++++++++++++++++++----------------
>  1 file changed, 45 insertions(+), 35 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 13e619794c..fde78517ff 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -6,6 +6,27 @@ mips mipsel mipsn32 mipsn32el mips64 mips64el \
>  sh4 sh4eb s390x aarch64 aarch64_be hppa riscv32 riscv64 xtensa xtensaeb \
>  microblaze microblazeel or1k x86_64"
> 
> +# check if given TARGETS is/are in the supported target list
> +qemu_check_target_list() {
> +    if [ $# -eq 0 ] ; then
> +      checked_target_list="$qemu_target_list"
> +      return
> +    fi
> +    for target ; do
> +        for cpu in $qemu_target_list ; do
> +            if [ "x$cpu" = "x$target" ] ; then
> +                checked_target_list="$checked_target_list $target"
> +                break
> +            fi
> +        done
> +        if [ "$unknown_target" = "true" ] ; then

           if [ "x$cpu" != "x$target" ] ; then

> +            echo "ERROR: unknown CPU \"$target\"" 1>&2
> +            usage
> +            exit 1
> +        fi
> +    done
> +}
> +
>  i386_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00'
>  i386_mask='\xff\xff\xff\xff\xff\xfe\xfe\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'
>  i386_family=i386
> @@ -167,21 +188,25 @@ qemu_get_family() {
> 
>  usage() {
>      cat <<EOF
> +Usage: qemu-binfmt-conf.sh [options] [TARGETS]
> 
> -Usage: qemu-binfmt-conf.sh [options]
> -
> -Configure binfmt_misc to use qemu interpreter
> +Configure binfmt_misc to use qemu interpreter for the given TARGETS.
> 
>  Options and associated environment variables:
> 
>  Argument             Env-variable     Description
> +TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);
> +                                      if empty, configure all known targets;
> +                                      if 'NONE', no interpreter is configured.
>  -h|--help                             display this usage
>  -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
>  -F|--suffix SUFFIX:  QEMU_SUFFIX      add a suffix to the default interpreter name
>  -d|--debian:                          don't write into /proc, generate update-binfmts templates
> --s|--systemd CPU:                     don't write into /proc, generate file for
> -                                      systemd-binfmt.service for the given CPU; if CPU is "ALL",
> -                                      generate a file for all known cpus.
> +-s|--systemd:                         don't write into /proc, generate file(s) for
> +                                      systemd-binfmt.service; environment variable HOST_ARCH

why HOST_ARCH appears here?

> +                                      allows to override 'uname' to generate configuration files
> +                                      for a different architecture than the current one.
> +
>  -e|--exportdir PATH: DEBIANDIR        define where to write configuration files
>                       SYSTEMDDIR
>  -c|--credential:     QEMU_CREDENTIAL  (yes) credential and security tokens are calculated according
> @@ -197,14 +222,7 @@ To remove interpreter, use :
> 
>      sudo update-binfmts --package qemu-CPU --remove qemu-CPU $QEMU_PATH
> 
> -With systemd, binfmt files are loaded by systemd-binfmt.service
> -
> -The environment variable HOST_ARCH allows to override 'uname' to generate configuration files for a
> -different architecture than the current one.
> -
> -where CPU is one of:
> -
> -    $qemu_target_list
> +QEMU target list: $qemu_target_list
> 
>  EOF
>  }
> @@ -288,9 +306,15 @@ qemu_set_binfmts() {
>      # probe cpu type
>      host_family=$(qemu_get_family)
> 
> -    # register the interpreter for each cpu except for the native one
> +    # reduce the list of target interpreters to those given in the CLI
> +    [ $# -eq 0 ] && targets="${QEMU_TARGETS:-}" || targets="$@"
> +    if [ "x$targets" = "xNONE" ] ; then
> +      return
> +    fi
> +    qemu_check_target_list $targets
> 
> -    for cpu in ${qemu_target_list} ; do
> +    # register the interpreter for each target except for the native one
> +    for cpu in $checked_target_list ; do
>          magic=$(eval echo \$${cpu}_magic)
>          mask=$(eval echo \$${cpu}_mask)
>          family=$(eval echo \$${cpu}_family)
> @@ -318,12 +342,13 @@ BINFMT_SET=qemu_register_interpreter
>  SYSTEMDDIR="/etc/binfmt.d"
>  DEBIANDIR="/usr/share/binfmts"
> 
> +QEMU_TARGETS="${QEMU_TARGETS:-}"
>  QEMU_PATH="${QEMU_PATH:-/usr/local/bin}"
>  QEMU_SUFFIX="${QEMU_SUFFIX:-}"
>  QEMU_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
> @@ -337,23 +362,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
> @@ -384,5 +392,7 @@ while true ; do
>      shift
>  done
> 
> +shift
> +
>  $CHECK
> -qemu_set_binfmts
> +qemu_set_binfmts "$@"
> --
> 2.21.0
> 

Re: [Qemu-devel] [PATCH v4 4/10] qemu-binfmt-conf.sh: use the same presentation format as for qemu-*

[Laurent Vivier]

On 11/03/2019 11:27, Unai Martinez-Corral wrote:
> 
> Signed-off-by: Unai Martinez-Corral <unai.martinezcorral@ehu.eus>
> ---
>  scripts/qemu-binfmt-conf.sh | 58 +++++++++++++++++--------------------
>  1 file changed, 27 insertions(+), 31 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index e7a714e22c..489d7d2326 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -167,47 +167,43 @@ qemu_get_family() {
> 
>  usage() {
>      cat <<EOF
> -Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
> -                           [--help][--credential][--exportdir PATH]
> -                           [--persistent][--qemu-suffix SUFFIX]
> +Usage: qemu-binfmt-conf.sh [options]
> 
> -       Configure binfmt_misc to use qemu interpreter
> +Configure binfmt_misc to use qemu interpreter
> 
> -       --help:        display this usage
> -       --qemu-path:   set path to qemu interpreter ($QEMU_PATH)
> -       --qemu-suffix: add a suffix to the default interpreter name
> -       --debian:      don't write into /proc,
> -                      instead generate update-binfmts templates
> -       --systemd:     don't write into /proc,
> -                      instead generate file for systemd-binfmt.service
> -                      for the given CPU. If CPU is "ALL", generate a
> -                      file for all known cpus
> -       --exportdir:   define where to write configuration files
> -                      (default: $SYSTEMDDIR or $DEBIANDIR)
> -       --credential:  if present, credential and security tokens are
> -                      calculated according to the binary to interpret
> -                      ($QEMU_CREDENTIAL=yes)
> -       --persistent:  if present, the interpreter is loaded when binfmt is
> -                      configured and remains in memory. All future uses
> -                      are cloned from the open file.
> -                      ($QEMU_PERSISTENT=yes)
> +Options and associated environment variables:
> 
> -    To import templates with update-binfmts, use :
> +Argument             Env-variable     Description
> +-h|--help                             display this usage
> +-Q|--qemu-path PATH: QEMU_PATH        set path to qemu interpreter

Please, remove all the ':' above and below.
You should merge PATCH 9/10 with this one.

Thanks,
Laurent

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

[Laurent Vivier]

On 11/03/2019 11:29, 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 | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
> index 0918f7fba6..13e619794c 100755
> --- a/scripts/qemu-binfmt-conf.sh
> +++ b/scripts/qemu-binfmt-conf.sh
> @@ -177,7 +177,7 @@ Options and associated environment variables:
>  Argument             Env-variable     Description
>  -h|--help                             display this usage
>  -Q|--path PATH:      QEMU_PATH        set path to qemu interpreter(s)
> --F|--suffix SUFFIX:                   add a suffix to the default interpreter name
> +-F|--suffix SUFFIX:  QEMU_SUFFIX      add a suffix to the default interpreter name
>  -d|--debian:                          don't write into /proc, generate update-binfmts templates
>  -s|--systemd CPU:                     don't write into /proc, generate file for
>                                        systemd-binfmt.service for the given CPU; if CPU is "ALL",
> @@ -318,13 +318,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.21.0
> 

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

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

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190311101428.GA11@765644dd90e5/



Hi,

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

Type: series
Message-id: 20190311101428.GA11@765644dd90e5
Subject: [Qemu-devel] [PATCH v4 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/20190311101428.GA11@765644dd90e5 -> patchew/20190311101428.GA11@765644dd90e5
Switched to a new branch 'test'
04f54aebb7 qemu-binfmt-conf.sh: add --test|--dry-run
fd47672add qemu-binfmt-conf.sh: update usage()
9a07973da4 qemu-binfmt-conf.sh: add option --clear
1debedf8e2 qemu-binfmt-conf.sh: generalize CPU to positional TARGETS
89024f269c qemu-binfmt-conf.sh: honour QEMU_PATH and/or QEMU_SUFFIX
996793bc94 qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options
8100c0cdfc qemu-binfmt-conf.sh: use the same presentation format as for qemu-*
0516d54354 qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT
a59a10b55f qemu-binfmt-conf.sh: make opts -p and -c boolean
ae2b7e65b0 qemu-binfmt-conf.sh: enforce safe style consistency

=== OUTPUT BEGIN ===
1/10 Checking commit ae2b7e65b0a6 (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 a59a10b55f0d (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 0516d54354e5 (qemu-binfmt-conf.sh: add QEMU_CREDENTIAL and QEMU_PERSISTENT)
4/10 Checking commit 8100c0cdfc78 (qemu-binfmt-conf.sh: use the same presentation format as for qemu-*)
WARNING: line over 80 characters
#50: FILE: scripts/qemu-binfmt-conf.sh:179:
+-F|--qemu-suffix SUFFIX:              add a suffix to the default interpreter name

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

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

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

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

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

total: 5 errors, 1 warnings, 74 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 996793bc9412 (qemu-binfmt-conf.sh: remove 'qemu' prefix from cli options)
WARNING: line over 80 characters
#36: FILE: scripts/qemu-binfmt-conf.sh:180:
+-F|--suffix SUFFIX:                   add a suffix to the default interpreter name

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

total: 1 errors, 1 warnings, 38 lines checked

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

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

total: 0 errors, 1 warnings, 23 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 1debedf8e243 (qemu-binfmt-conf.sh: generalize CPU to positional TARGETS)
ERROR: line over 90 characters
#76: FILE: scripts/qemu-binfmt-conf.sh:198:
+TARGETS              QEMU_TARGETS     A single arch name or a list of them (see all names below);

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

ERROR: line over 90 characters
#87: FILE: scripts/qemu-binfmt-conf.sh:206:
+                                      systemd-binfmt.service; environment variable HOST_ARCH

ERROR: line over 90 characters
#88: FILE: scripts/qemu-binfmt-conf.sh:207:
+                                      allows to override 'uname' to generate configuration files

WARNING: line over 80 characters
#89: FILE: scripts/qemu-binfmt-conf.sh:208:
+                                      for a different architecture than the current one.

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

total: 4 errors, 2 warnings, 135 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 9a07973da45d (qemu-binfmt-conf.sh: add option --clear)
WARNING: line over 80 characters
#33: FILE: scripts/qemu-binfmt-conf.sh:199:
+                                      if empty, configure/clear all known targets;

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

WARNING: line over 80 characters
#69: FILE: scripts/qemu-binfmt-conf.sh:356:
+    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;

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

total: 2 errors, 2 warnings, 76 lines checked

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

9/10 Checking commit fd47672add95 (qemu-binfmt-conf.sh: update usage())
ERROR: line over 90 characters
#29: FILE: scripts/qemu-binfmt-conf.sh:204:
+-p|--persistent:     QEMU_PERSISTENT  (yes) load the interpreter and keep it in memory; all future

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

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

ERROR: line over 90 characters
#42: FILE: scripts/qemu-binfmt-conf.sh:213:
+                                      systemd-binfmt.service; environment variable HOST_ARCH

ERROR: line over 90 characters
#43: FILE: scripts/qemu-binfmt-conf.sh:214:
+                                      allows to override 'uname' to generate configuration files

WARNING: line over 80 characters
#44: FILE: scripts/qemu-binfmt-conf.sh:215:
+                                      for a different architecture than the current one.

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

total: 5 errors, 2 warnings, 45 lines checked

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

10/10 Checking commit 04f54aebb77d (qemu-binfmt-conf.sh: add --test|--dry-run)
WARNING: line over 80 characters
#20: FILE: scripts/qemu-binfmt-conf.sh:199:
+                                      if empty, configure/clear all known targets.

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

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

total: 2 errors, 1 warnings, 60 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/20190311101428.GA11@765644dd90e5/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 v4 10/10] qemu-binfmt-conf.sh: add --test|--dry-run

[Unai Martinez Corral]

2019/3/11 11:45, Laurent Vivier:
> >  while true ; do
> >      case "$1" in
> > +    -t|--test|--dry-run)
>
> We don't need multiple parameter for the same effect. You must choose
> between --test and --dry-run.

Ok. I'll take 'test', because the short for '--dry-run' would be '-d'
and it is already used for 'debian'.

> > +if [ "x$QEMU_TEST" = "xyes" ] ; then
> > +    skip(){
> > +    :;}
> > +    BINFMT_SET=skip
>
> Why do you need a function?
> Can't you set directly BINFMT_SET to ':'?

Indeed...

Regards,
Unai

Re: [Qemu-devel] [PATCH v4 8/10] qemu-binfmt-conf.sh: add option --clear

[Unai Martinez Corral]

2019/3/11 12:04, Laurent Vivier:
> > +    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;
>
> The qemu-* will be expanded here if you have a qemu-XXX in the current
> directory. You must use "$names".

You are correct. Indeed, I had not spotted it because I introduced a
bug when renaming 'reset' to 'clear'. Precisely, '-c' was being used
twice: for 'credential' and for 'clear'. Both issues will be fixed in
the next version.

> But:
>
> To remove all, you can do:
> sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/status'
>
> so something like
>
> if [ $# -eq 0 ] ; then
>   sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/status
> fi
> qemu_check_target_list $1
> for t in $checked_target_list ; do
>    sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/qemu-$t'
> done

Wouldn't writing to 'status' remove all the interpreters, and not only
those that correspond to qemu?

> But I think you should also taking care of the suffix.

I think that the suffix is not related to the entry in
'/proc/sys/fs/binfmt_misc/'. See, e.g.:

package qemu-$cpu
interpreter $qemu

So, independently of which is the executable (interpreter), the
package name does not include the suffix.

> > +if [ "x$QEMU_CLEAR" = "xyes" ] ; then
> > +  case "$BINFMT_SET" in
> > +    *debian)  BINFMT_CLEAR=qemu_clear_notimplemented ;;
> > +    *systemd) BINFMT_CLEAR=qemu_clear_notimplemented ;;
> > +    *)        BINFMT_CLEAR=qemu_clear_interpreter
> > +  esac
>
> Put this in the previous case for decoding options, please.

It won't work. This if/case block requires all the options to be
already parsed and processed. If I put it in the case for '-r|--clear'
above, it will only work only when '-r' is given after '--debian' or
'--systemd'. Otherwise, BINFMT_SET will always be
qemu_register_interpreter when '-crear' is evaluated. I think that we
should not rely on the users providing the options in a specific
order.

Regards,
Unai

Re: [Qemu-devel] [PATCH v4 7/10] qemu-binfmt-conf.sh: generalize CPU to positional TARGETS

[Unai Martinez Corral]

2019/3/11 12:14, Laurent Vivier:
> On 11/03/2019 11:30, Unai Martinez-Corral wrote:

> > +        if [ "$unknown_target" = "true" ] ; then
>
>            if [ "x$cpu" != "x$target" ] ; then

My bad. I fixed it before squashing, and broke it again when doing it.

> > +-s|--systemd:                         don't write into /proc, generate file(s) for
> > +                                      systemd-binfmt.service; environment variable HOST_ARCH
>
> why HOST_ARCH appears here?

The existing comment seems to be specific to systemd:
https://github.com/qemu/qemu/blob/master/scripts/qemu-binfmt-conf.sh#L201-L204
'systemd' is the single mode in which it makes sense to generate a
bunch of configuration files for a different architecture than the
current one, isn't it?

Regards,
Unai

Re: [Qemu-devel] [PATCH v4 8/10] qemu-binfmt-conf.sh: add option --clear

[Laurent Vivier]

On 11/03/2019 14:19, Unai Martinez Corral wrote:
> 2019/3/11 12:04, Laurent Vivier:
>>> +    find /proc/sys/fs/binfmt_misc/ -type f -name $names -exec sh -c 'printf %s -1 > {}' \;
>>
>> The qemu-* will be expanded here if you have a qemu-XXX in the current
>> directory. You must use "$names".
> 
> You are correct. Indeed, I had not spotted it because I introduced a
> bug when renaming 'reset' to 'clear'. Precisely, '-c' was being used
> twice: for 'credential' and for 'clear'. Both issues will be fixed in
> the next version.
> 
>> But:
>>
>> To remove all, you can do:
>> sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/status'
>>
>> so something like
>>
>> if [ $# -eq 0 ] ; then
>>   sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/status
>> fi
>> qemu_check_target_list $1
>> for t in $checked_target_list ; do
>>    sh -c 'printf %s -1 > /proc/sys/fs/binfmt_misc/qemu-$t'
>> done
> 
> Wouldn't writing to 'status' remove all the interpreters, and not only
> those that correspond to qemu?

Yes, you are right.

> 
>> But I think you should also taking care of the suffix.
> 
> I think that the suffix is not related to the entry in
> '/proc/sys/fs/binfmt_misc/'. See, e.g.:
> 
> package qemu-$cpu
> interpreter $qemu
> 
> So, independently of which is the executable (interpreter), the
> package name does not include the suffix.

Yes, you are right.

>>> +if [ "x$QEMU_CLEAR" = "xyes" ] ; then
>>> +  case "$BINFMT_SET" in
>>> +    *debian)  BINFMT_CLEAR=qemu_clear_notimplemented ;;
>>> +    *systemd) BINFMT_CLEAR=qemu_clear_notimplemented ;;
>>> +    *)        BINFMT_CLEAR=qemu_clear_interpreter
>>> +  esac
>>
>> Put this in the previous case for decoding options, please.
> 
> It won't work. This if/case block requires all the options to be
> already parsed and processed. If I put it in the case for '-r|--clear'
> above, it will only work only when '-r' is given after '--debian' or
> '--systemd'. Otherwise, BINFMT_SET will always be
> qemu_register_interpreter when '-crear' is evaluated. I think that we
> should not rely on the users providing the options in a specific
> order.

I meant something like this:

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index b5a16742a149..5c2651a6df57 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -327,17 +327,23 @@ QEMU_SUFFIX=""
 options=$(getopt -o ds:Q:S:e:hc:p: -l
debian,systemd:,qemu-path:,qemu-suffix:,exportdir:,help,credential:,persistent:
-- "$@")
 eval set -- "$options"

+BINFMT_CLEAR=qemu_clear_interpreter
 while true ; do
     case "$1" in
+    -c|--clear)
+        QEMU_CLEAR="yes"
+        ;;
     -d|--debian)
         CHECK=qemu_check_debian
         BINFMT_SET=qemu_generate_debian
         EXPORTDIR=${EXPORTDIR:-$DEBIANDIR}
+        BINFMT_CLEAR=qemu_clear_notimplemented
         ;;
     -s|--systemd)
         CHECK=qemu_check_systemd
         BINFMT_SET=qemu_generate_systemd
         EXPORTDIR=${EXPORTDIR:-$SYSTEMDDIR}
+        BINFMT_CLEAR=qemu_clear_notimplemented
         shift
         # check given cpu is in the supported CPU list
         if [ "$1" != "ALL" ] ; then
@@ -387,5 +393,9 @@ while true ; do
     shift
 done

+if [ "x$QEMU_CLEAR" = "xyes" ] ; then
+  $BINFMT_CLEAR "$@"
+  exit
+fi
 $CHECK
 qemu_set_binfmts

Re: [Qemu-devel] [PATCH v4 7/10] qemu-binfmt-conf.sh: generalize CPU to positional TARGETS

[Laurent Vivier]

On 11/03/2019 14:29, Unai Martinez Corral wrote:
> 2019/3/11 12:14, Laurent Vivier:
>> On 11/03/2019 11:30, Unai Martinez-Corral wrote:
> 
>>> +        if [ "$unknown_target" = "true" ] ; then
>>
>>            if [ "x$cpu" != "x$target" ] ; then
> 
> My bad. I fixed it before squashing, and broke it again when doing it.
> 
>>> +-s|--systemd:                         don't write into /proc, generate file(s) for
>>> +                                      systemd-binfmt.service; environment variable HOST_ARCH
>>
>> why HOST_ARCH appears here?
> 
> The existing comment seems to be specific to systemd:
> https://github.com/qemu/qemu/blob/master/scripts/qemu-binfmt-conf.sh#L201-L204
> 'systemd' is the single mode in which it makes sense to generate a
> bunch of configuration files for a different architecture than the
> current one, isn't it?

No, it's not specific to systemd. If we register an interpreter for the
current architecture we can make it unusable.

I've proposed sometime ago a kernel patch to set binfmt_misc
configuration by namespace to avoid this kind of problem but it has
never been merged:

  [v6,0/1] ns: introduce binfmt_misc namespace
  https://patchwork.kernel.org/cover/10634807/

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4 7/10] qemu-binfmt-conf.sh: generalize CPU to positional TARGETS

[Unai Martinez Corral]

2019/3/11 a las 14:36, Laurent Vivier:
> On 11/03/2019 14:29, Unai Martinez Corral wrote:
> > 2019/3/11 12:14, Laurent Vivier:
> >> On 11/03/2019 11:30, Unai Martinez-Corral wrote:
> >
> >>> +-s|--systemd:                         don't write into /proc, generate file(s) for
> >>> +                                      systemd-binfmt.service; environment variable HOST_ARCH
> >>
> >> why HOST_ARCH appears here?
> >
> > The existing comment seems to be specific to systemd:
> > https://github.com/qemu/qemu/blob/master/scripts/qemu-binfmt-conf.sh#L201-L204
> > 'systemd' is the single mode in which it makes sense to generate a
> > bunch of configuration files for a different architecture than the
> > current one, isn't it?
>
> No, it's not specific to systemd. If we register an interpreter for the
> current architecture we can make it unusable.

I think we are not understanding each other in this point:

The comment about HOST_ARCH in the current master branch is as follows:

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

That's why I assumed that the envvar is meant to be used with systemd
only. Certainly, withou systemd no configuration files are generated
at all; interpreters are directly registered/configured.

However, the implementation takes HOST_ARCH into account with no
regard to 'systemd' or 'debian'. It is used unconditionally. This has
been like this since the base of the current script was pushed in
2016/1/29.

So, what is your proposal?

- Move the comment below, as it was before.
- Add a patch to only take HOST_ARCH into account for systemd (and debian?)
- Remove HOST_ARCH and the comment from the script (and take it back
if you patch to the kernel is accepted some day).

I will wait to sort this out before pushing v5. I think that all the
remaining issues are addressed already:
https://github.com/umarcor/qemu/blob/series-qemu-binfmt-conf/scripts/qemu-binfmt-conf.sh

Regards,
Unai

Re: [Qemu-devel] [PATCH v4 7/10] qemu-binfmt-conf.sh: generalize CPU to positional TARGETS

[Laurent Vivier]

On 11/03/2019 20:23, Unai Martinez Corral wrote:
> 2019/3/11 a las 14:36, Laurent Vivier:
>> On 11/03/2019 14:29, Unai Martinez Corral wrote:
>>> 2019/3/11 12:14, Laurent Vivier:
>>>> On 11/03/2019 11:30, Unai Martinez-Corral wrote:
>>>
>>>>> +-s|--systemd:                         don't write into /proc, generate file(s) for
>>>>> +                                      systemd-binfmt.service; environment variable HOST_ARCH
>>>>
>>>> why HOST_ARCH appears here?
>>>
>>> The existing comment seems to be specific to systemd:
>>> https://github.com/qemu/qemu/blob/master/scripts/qemu-binfmt-conf.sh#L201-L204
>>> 'systemd' is the single mode in which it makes sense to generate a
>>> bunch of configuration files for a different architecture than the
>>> current one, isn't it?
>>
>> No, it's not specific to systemd. If we register an interpreter for the
>> current architecture we can make it unusable.
> 
> I think we are not understanding each other in this point:
> 
> The comment about HOST_ARCH in the current master branch is as follows:
> 
>> 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.
> 
> That's why I assumed that the envvar is meant to be used with systemd
> only. Certainly, withou systemd no configuration files are generated
> at all; interpreters are directly registered/configured.
> 
> However, the implementation takes HOST_ARCH into account with no
> regard to 'systemd' or 'debian'. It is used unconditionally. This has
> been like this since the base of the current script was pushed in
> 2016/1/29.
> 
> So, what is your proposal?
> 
> - Move the comment below, as it was before.

Yes, move the comment below as it was before.

Thanks,
LAurent