diff mbox series

scripts/qemu-binfmt-conf.sh: allow clearing of entries

Message ID 20180703160022.10705-1-alex.bennee@linaro.org
State New
Headers show
Series scripts/qemu-binfmt-conf.sh: allow clearing of entries | expand

Commit Message

Alex Bennée July 3, 2018, 4 p.m. UTC 
Currently running the script twice will fail with "sh: echo: I/O
error" as the registration is already complete. Add a new option
--clear to reset the entries to save the user doing it by hand.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 scripts/qemu-binfmt-conf.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

-- 
2.17.1

Comments

no-reply@patchew.org July 3, 2018, 4:56 p.m. UTC | #1 
Hi,

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

Type: series
Message-id: 20180703160022.10705-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH] scripts/qemu-binfmt-conf.sh: allow clearing of entries

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20180703105305.45398-1-vsementsov@virtuozzo.com -> patchew/20180703105305.45398-1-vsementsov@virtuozzo.com
 * [new tag]               patchew/20180703160022.10705-1-alex.bennee@linaro.org -> patchew/20180703160022.10705-1-alex.bennee@linaro.org
Switched to a new branch 'test'
b09b364445 scripts/qemu-binfmt-conf.sh: allow clearing of entries

=== OUTPUT BEGIN ===
Checking PATCH 1/1: scripts/qemu-binfmt-conf.sh: allow clearing of entries...
WARNING: line over 80 characters
#25: FILE: scripts/qemu-binfmt-conf.sh:167:
+                           [--help][--clear][--credential yes|no][--exportdir PATH]

ERROR: line over 90 characters
#56: FILE: scripts/qemu-binfmt-conf.sh:317:
+options=$(getopt -o ds:Q:e:hc: -l debian,systemd:,qemu-path:,exportdir:,help,clear,credential: -- "$@")

total: 1 errors, 1 warnings, 46 lines checked

Your patch 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


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Laurent Vivier July 3, 2018, 4:57 p.m. UTC | #2 
Le 03/07/2018 à 18:00, Alex Bennée a écrit :
> Currently running the script twice will fail with "sh: echo: I/O

> error" as the registration is already complete. Add a new option

> --clear to reset the entries to save the user doing it by hand.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> ---

>  scripts/qemu-binfmt-conf.sh | 16 ++++++++++++++--

>  1 file changed, 14 insertions(+), 2 deletions(-)

> 

> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh

> index d7eefda0b8..13ef4713e6 100755

> --- a/scripts/qemu-binfmt-conf.sh

> +++ b/scripts/qemu-binfmt-conf.sh

> @@ -160,7 +160,7 @@ qemu_get_family() {

>  usage() {

>      cat <<EOF

>  Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]

> -                           [--help][--credential yes|no][--exportdir PATH]

> +                           [--help][--clear][--credential yes|no][--exportdir PATH]

>  

>         Configure binfmt_misc to use qemu interpreter

>  

> @@ -176,6 +176,7 @@ Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]

>                       (default: $SYSTEMDDIR or $DEBIANDIR)

>         --credential: if yes, credential and security tokens are

>                       calculated according to the binary to interpret

> +       --clear:      clear existing qemu binfmt registrations

>  

>      To import templates with update-binfmts, use :

>  

> @@ -249,6 +250,13 @@ qemu_register_interpreter() {

>      qemu_generate_register > /proc/sys/fs/binfmt_misc/register

>  }

>  

> +qemu_clear_interpreter() {

> +    if [ -e /proc/sys/fs/binfmt_misc/qemu-$cpu ]; then


You should use qemu_check_access()

> +        echo "Removing qemu-$cpu as binfmt interpreter for $cpu"

> +        echo -1 > /proc/sys/fs/binfmt_misc/qemu-$cpu

> +    fi

> +}

> +

>  qemu_generate_systemd() {

>      echo "Setting $qemu as binfmt interpreter for $cpu for systemd-binfmt.service"

>      qemu_generate_register > "$EXPORTDIR/qemu-$cpu.conf"

> @@ -302,7 +310,7 @@ DEBIANDIR="/usr/share/binfmts"

>  QEMU_PATH=/usr/local/bin

>  FLAGS=""

>  

> -options=$(getopt -o ds:Q:e:hc: -l debian,systemd:,qemu-path:,exportdir:,help,credential: -- "$@")

> +options=$(getopt -o ds:Q:e:hc: -l debian,systemd:,qemu-path:,exportdir:,help,clear,credential: -- "$@")

>  eval set -- "$options"

>  

>  while true ; do

> @@ -354,6 +362,10 @@ while true ; do

>              FLAGS=""

>          fi

>          ;;

> +    --clear)

> +        shift

> +        BINFMT_SET=qemu_clear_interpreter

> +        ;;

>      *)

>          break

>          ;;

> 


if you use --debian or --systemd, you don't have the problem because
update-binfmts and systemd-binfmt.service update /proc from the
generated files for you.

But you're right there is no command to undo what we have done.
You manage only the /proc case, 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"?

Thanks,
Laurent
Alex Bennée July 4, 2018, 8:26 a.m. UTC | #3 
Laurent Vivier <laurent@vivier.eu> writes:

> Le 03/07/2018 à 18:00, Alex Bennée a écrit:

>> Currently running the script twice will fail with "sh: echo: I/O

>> error" as the registration is already complete. Add a new option

>> --clear to reset the entries to save the user doing it by hand.

>>

>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>> ---

>>  scripts/qemu-binfmt-conf.sh | 16 ++++++++++++++--

>>  1 file changed, 14 insertions(+), 2 deletions(-)

>>

>> diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh

>> index d7eefda0b8..13ef4713e6 100755

>> --- a/scripts/qemu-binfmt-conf.sh

>> +++ b/scripts/qemu-binfmt-conf.sh

>> @@ -160,7 +160,7 @@ qemu_get_family() {

>>  usage() {

>>      cat <<EOF

>>  Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]

>> -                           [--help][--credential yes|no][--exportdir PATH]

>> +                           [--help][--clear][--credential yes|no][--exportdir PATH]

>>

>>         Configure binfmt_misc to use qemu interpreter

>>

>> @@ -176,6 +176,7 @@ Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]

>>                       (default: $SYSTEMDDIR or $DEBIANDIR)

>>         --credential: if yes, credential and security tokens are

>>                       calculated according to the binary to interpret

>> +       --clear:      clear existing qemu binfmt registrations

>>

>>      To import templates with update-binfmts, use :

>>

>> @@ -249,6 +250,13 @@ qemu_register_interpreter() {

>>      qemu_generate_register > /proc/sys/fs/binfmt_misc/register

>>  }

>>

>> +qemu_clear_interpreter() {

>> +    if [ -e /proc/sys/fs/binfmt_misc/qemu-$cpu ]; then

>

> You should use qemu_check_access()

>

>> +        echo "Removing qemu-$cpu as binfmt interpreter for $cpu"

>> +        echo -1 > /proc/sys/fs/binfmt_misc/qemu-$cpu

>> +    fi

>> +}

>> +

>>  qemu_generate_systemd() {

>>      echo "Setting $qemu as binfmt interpreter for $cpu for systemd-binfmt.service"

>>      qemu_generate_register > "$EXPORTDIR/qemu-$cpu.conf"

>> @@ -302,7 +310,7 @@ DEBIANDIR="/usr/share/binfmts"

>>  QEMU_PATH=/usr/local/bin

>>  FLAGS=""

>>

>> -options=$(getopt -o ds:Q:e:hc: -l debian,systemd:,qemu-path:,exportdir:,help,credential: -- "$@")

>> +options=$(getopt -o ds:Q:e:hc: -l debian,systemd:,qemu-path:,exportdir:,help,clear,credential: -- "$@")

>>  eval set -- "$options"

>>

>>  while true ; do

>> @@ -354,6 +362,10 @@ while true ; do

>>              FLAGS=""

>>          fi

>>          ;;

>> +    --clear)

>> +        shift

>> +        BINFMT_SET=qemu_clear_interpreter

>> +        ;;

>>      *)

>>          break

>>          ;;

>>

>

> if you use --debian or --systemd, you don't have the problem because

> update-binfmts and systemd-binfmt.service update /proc from the

> generated files for you.


Usually I just "apt install qemu-user" and be done with it but I was
writing up a blog post and trying to keep it as distro agnostic as
possible.

>

> But you're right there is no command to undo what we have done.

> You manage only the /proc case, 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"?


Sure, I'll look into it.

>

> Thanks,

> Laurent



--
Alex Bennée
diff mbox series

Patch

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index d7eefda0b8..13ef4713e6 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -160,7 +160,7 @@  qemu_get_family() {
 usage() {
     cat <<EOF
 Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
-                           [--help][--credential yes|no][--exportdir PATH]
+                           [--help][--clear][--credential yes|no][--exportdir PATH]
 
        Configure binfmt_misc to use qemu interpreter
 
@@ -176,6 +176,7 @@  Usage: qemu-binfmt-conf.sh [--qemu-path PATH][--debian][--systemd CPU]
                      (default: $SYSTEMDDIR or $DEBIANDIR)
        --credential: if yes, credential and security tokens are
                      calculated according to the binary to interpret
+       --clear:      clear existing qemu binfmt registrations
 
     To import templates with update-binfmts, use :
 
@@ -249,6 +250,13 @@  qemu_register_interpreter() {
     qemu_generate_register > /proc/sys/fs/binfmt_misc/register
 }
 
+qemu_clear_interpreter() {
+    if [ -e /proc/sys/fs/binfmt_misc/qemu-$cpu ]; then
+        echo "Removing qemu-$cpu as binfmt interpreter for $cpu"
+        echo -1 > /proc/sys/fs/binfmt_misc/qemu-$cpu
+    fi
+}
+
 qemu_generate_systemd() {
     echo "Setting $qemu as binfmt interpreter for $cpu for systemd-binfmt.service"
     qemu_generate_register > "$EXPORTDIR/qemu-$cpu.conf"
@@ -302,7 +310,7 @@  DEBIANDIR="/usr/share/binfmts"
 QEMU_PATH=/usr/local/bin
 FLAGS=""
 
-options=$(getopt -o ds:Q:e:hc: -l debian,systemd:,qemu-path:,exportdir:,help,credential: -- "$@")
+options=$(getopt -o ds:Q:e:hc: -l debian,systemd:,qemu-path:,exportdir:,help,clear,credential: -- "$@")
 eval set -- "$options"
 
 while true ; do
@@ -354,6 +362,10 @@  while true ; do
             FLAGS=""
         fi
         ;;
+    --clear)
+        shift
+        BINFMT_SET=qemu_clear_interpreter
+        ;;
     *)
         break
         ;;