Re: [PATCH] tools: fix oom setting of xenstored

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.10.2021 09:31, Juergen Gross wrote:
> On 19.10.21 08:54, Jan Beulich wrote:
>> On 19.10.2021 06:41, Juergen Gross wrote:
>>> --- a/tools/hotplug/Linux/launch-xenstore.in
>>> +++ b/tools/hotplug/Linux/launch-xenstore.in
>>> @@ -60,7 +60,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . 
>>> @CONFIG_DIR@/@CONFIG_LEAF
>>>             echo "No xenstored found"
>>>             exit 1
>>>     }
>>> -   [ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] || XENSTORED_OOM_MEM_THRESHOLD=50
>>> +   [ -z "$XENSTORED_OOM_MEM_THRESHOLD" ] && XENSTORED_OOM_MEM_THRESHOLD=50
>>
>> Is resilience against "set -e" being in effect of interest? If so I
>> think this would want to be
> 
> I don't think set -e would have a negative effect on above line. The
> bash man-page tells me that:
> 
>    The shell does not exit if the command that fails is part of the
>    command list immediately following a while or until keyword, part of
>    the test following the if, ...
> 
> And I believe that "[ ... ]" is treated like an "if".

I don't think so - "[ ... ]" is an equivalent of "test ...", i.e.
unrelated to whether that's an operand of "if". The question is
what effect && has, i.e. the behavior is due to what you've
hidden by using ... in your quotation: "..., and is not a part of
an AND or OR list, ...".

I think I recall constructs like the one you use not working with
"set -e" on at least some bash versions, though. Apparently this
was due to a bash bug then (or I'm misremembering, but that's not
overly likely since some of my long used scripts specifically
avoid using && in such situations).

> A short test
> showed that bash indeed does not exit in this case:
> 
>    ( set -e; [ -z "" ] && xx=okay; echo $xx; )
> 
> This will print "okay", so bash didn't exit.

Of course, because the left side of && succeeds. You'd need

   ( set -e; [ -z "xxx" ] && xx=okay; echo xx=$xx; )

and observe "xx=" getting printed. Which indeed I do observe on
the one bash version I've tried to double check. But that one's
surely newer than what I think I saw such problems on.

Jan