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