Updated: http://pastebin.com/VC03VXvi
See answers inline
On Thu, Sep 30, 2010 at 9:18 AM, Florian Haas <florian.h...@linbit.com> wrote:
> Hello Jonathan,
>
> A few other comments:
>
> - As stated before, numeric comparisons ought to be numeric comparisons:
>
>> if [ $? = $OCF_SUCCESS ]; then
>
> Use -eq.
>
> - You still have a bunch like these around:
>
>> # shorten kernel conntrack timers to remove the zombie entries.
>> $CONNTRACKD -t
>> if [ $? -eq 1 ]
>> then
>> return $OCF_ERR_GENERIC
>> fi
Missed that one.
>
> Use ocf_run.
>
> - conntrackd_start doesn't produce the correct error code if the binary
> isn't installed. Easily fixed with "check_binary ${OCF_RESKEY_binary}".
Added to conntrackd_validate
>
> - What's the "asd" doing here?
>
>> # flush the internal and the external caches
>> ocf_run $CONNTRACKD -f || exit "asd".$OCF_ERR_GENERIC
>
Typo from debug, fixed.
> - I don't understand how the conntrack daemon is actually started.
> Shouldn't there be some invocation of $CONNTRACKD -d in conntrackd_start?
The OCF resource doesn't really handle whether the daemon should start
or not (see earlier discussions in this on the thread), I would
probably add the daemon as a LSB resource rather.
>
> - You're still advertising migrate_from and migrate_to through RA
> metadata and the usage message, please remove those.
Fixed.
>
> - All your parameter definitions come with "contect" in place of
> "content", please fix that typo.
Fixed
>
> - Please run your RA through ocf-tester.
deb-fw1:~# ocf-tester -L -n conntrackd
/usr/lib/ocf/resource.d/heartbeat/conntrackd
Using lrmd/lrmadmin for all tests
Beginning tests for /usr/lib/ocf/resource.d/heartbeat/conntrackd...
* rc=0: Monitoring a stopped resource should return 7
* Your agent does not support the notify action (optional)
* Your agent does not support the demote action (optional)
* Your agent does not support the promote action (optional)
* Your agent does not support master/slave (optional)
* rc=0: Monitoring a stopped resource should return 7
* rc=0: Monitoring a stopped resource should return 7
* rc=0: Monitoring a stopped resource should return 7
Tests failed: /usr/lib/ocf/resource.d/heartbeat/conntrackd failed 4 tests
Not 100% sure codewise what I should change.
Maybe it would make sense to add master/slave to handle the
state-transition and have start/stop handle whether the daemon is
alive or not. However I'm not entirely sure how the crm configuration
statement for this should look given that the daemon should be started
on both nodes running in either master or slave mode, do you have an
example for this?
>
> Cheers,
> Florian
>
>
>
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>
>
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
