[ovs-dev] [PATCH v2] Make pid_exists() more robust against empty pid argument

Ilya Maximets i.maximets at samsung.com
Fri Aug 30 12:19:10 UTC 2019


On 29.08.2019 17:46, Ben Pfaff wrote:
> On Tue, Aug 27, 2019 at 02:43:21PM +0300, Ilya Maximets wrote:
>> On 14.08.2019 18:47, Michele Baldessari wrote:
>>> In some of our destructive testing of ovn-dbs inside containers managed
>>> by pacemaker we reached a situation where /var/run/openvswitch had
>>> empty .pid files. The current code does not deal well with them
>>> and pidfile_is_running() returns true in such a case and this confuses
>>> the OCF resource agent.
>>>
>>> - Before this change:
>>> Inside a container run:
>>>   killall ovsdb-server;
>>>   echo -n '' > /var/run/openvswitch/ovnnb_db.pid; echo -n '' > /var/run/openvswitch/ovnsb_db.pid
>>
>> What about whitespaces?
>> I mean, if you'll  write ' ' instead of '', the check 'test -n "$1"'
>> will succeed and the test will fail.
> 
> I think that is OK, because test -d "/proc/ " will also fail.

Oh, I see. Good point.

> 
>> To handle this case we need to trim off whitespaces by the 'tr' utility
>> or change the proc checker to something like 'test -f /proc/"$1"/status'.
> 
> I guess to be absolutely certain we'd need something like
> 
> case $1 in
>   '')       false ;;    # Reject empty string
>   *[!0-9]*) false ;;    # Reject anything with non-digits
>   *)        test -d /proc/$1 ;;
> esac
> 
> Anyway, I think that this is an improvement, so I applied it to master
> and backported it.

Agree. Thanks!

Best regards, Ilya Maximets.


More information about the dev mailing list