Re: [Xen-devel] [PATCH v2 6/6] RFC: tools/dm_restrict: Enable QEMU sandboxing

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

George Dunlap writes ("Re: [PATCH v2 6/6] RFC: tools/dm_restrict: Enable QEMU 
sandboxing"):
> On 09/24/2018 12:21 PM, Ian Jackson wrote:
> > Just noticed this, but: OMG no `set -e'.
> > You probably want `set -o pipefail' too.
> 
> `set -e` never made any sense to me -- that's not the way I code in any
> other language; why would scripting be any different?  What's the
> advantage of doing that in the current script?

?

Many modern languages throw exceptions.  set -e is a bit like that.
If you don't say set -e then you need to wrap every everything in your
entire script with an error check.

For example, you write

> >> dmpid=$(xenstore-read /local/domain/$domid/image/device-model-pid 
> >> 2>/dev/null)
> >> if [[ -z "$dmpid" ]] ; then
> >>     echo "xenstore-read failed"
> >>     exit 1
> >> fi
> > 
> > Why do you throw away the stderr from xenstore-read ?
> 
> That's left over from a previous version of the script, where I didn't
> check to see whether $1 was numeric, but rather tried to interpret it as
> numeric and if it failed, then ran `xl domid` on it.  I can take that out.

but with set -e you can write only

    dmpid=$(xenstore-read /local/domain/$domid/image/device-model-pid)

and the subsequent if is not needed.


> The question is: should the script handle the case where
> `xen-qemuuser-shared` is defined instead of `xen-qemuuser-range-base`?

I think it is fine if it doesn't.  If someone wants that feature it is
easy to add it.

> >> function check_rlimit() {
> >>     limit_name=$1
> >>     limit_string=$2
> >>     tgt=$3
> >>
> >>     echo -n "rlimit $limit_name: "
> >>     input=$(grep "^$limit_string" /proc/$dmpid/limits)
> > ...
> >>     if [[ "$input" =~ 
> >> ^$limit_string[[:space:]]*([^[:space:]]+)[[:space:]]*([^[:space:]]+)[[:space:]]*[^[:space:]]+
> >>  ]] ; then
> > 
> > Because of the unfortunate format of /proc/PID/limits, you do can't
> > just do the
> >     fields=($input)
> > trick but
> >     fields=(${input#*  })
> 
> What will this do?

The expression
  ${input#*  }
is the contents of input with the shortest prefix matching `*  '
stripped off the front.  That will eat all the words at the start,
which are separated by one space each, and find the first pair of
adjacent spaces.  So if input is
  'Max processes             63603                63603                
processes 
then "${input#*  }" is
  '           63603                63603                processes 

Expanding it without the surrounding " " causes it to be word-split in
the usual way, producing
  63603 63603 processes
and then this is assigned to a new bash array variable `fields'
which can then be indexed to find the values.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel