[Qemu-devel] [PATCH] scripts/qemu-binfmt-conf.sh: allow clearing of entries

[Qemu-devel] [PATCH] scripts/qemu-binfmt-conf.sh: allow clearing of entries

[Alex Bennée]

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
+        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
         ;;
-- 
2.17.1

Re: [Qemu-devel] [PATCH] scripts/qemu-binfmt-conf.sh: allow clearing of entries

[no-reply]

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

Re: [Qemu-devel] [PATCH] scripts/qemu-binfmt-conf.sh: allow clearing of entries

[Laurent Vivier]

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

Re: [Qemu-devel] [PATCH] scripts/qemu-binfmt-conf.sh: allow clearing of entries

[Alex Bennée]

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