On 2010-11-24 19:46, Frank Lazzarini wrote: > Hi, > > sure no problem we can name it firebird.
OK, firebird it is. > Packaging it with the firebird > packages I don't really find a good idea, because not every user that > needs firebird 2.5 will use it in a cluster. That's why there would typically be a subpackage RPM named firebird-resource-agent or whatever, and no-one would be obliged to install that. > Therefore I would > appreciate it if it would be packages with the pacemaker package, if > that package includes all the ocf RA's. It's not pacemaker, it's the resource-agents (on Debian: cluster-agents) package, but sure, we can ship it. > Thanks again for the support. A few more issues I noticed: 1. you use sudo, any particular reason why you're not doing "su -c" like all the other RAs do? Besides, you should add a check_binary test (for either sudo or su) to your validate function. 2. Even if starting the daemon fails, you still pretend it succeeded: > # start fbguard with the specified user > sudo -u $OCF_RESKEY_user $OCF_RESKEY_binary -pidfile $OCF_RESKEY_pid > -daemon -forever > > return $OCF_SUCCESS You can either exit on error after sudo (with "|| exit $OCF_ERR_GENERIC"), or spin on monitor after start, or both. 3. This too looks fishy: > > if [ -f $OCF_RESKEY_pid ] > then > kill `cat $OCF_RESKEY_pid` > if [ $? = 0 ]; then > return $OCF_SUCCESS > fi All that $? tells you is whether you were able to _send_ the signal. It says nothing about whether the process did the right thing with the signal, and it _certainly_ does not imply that the process shut down completely. Returning successfully here is premature. Ditch that if block. Because just below: > fi > > while firebird25_monitor; do > ocf_log debug "Resource has not stopped yet, waiting" > sleep 1 > done ... you're doing the right thing and ask your monitor function to see if the daemon is in fact dead (and if it's not, you wait till it is). 4. In monitor, you do this: > pid=`cat $OCF_RESKEY_pid` > kill -s 0 $pid >/dev/null 2>&1 > rc=$? ... and then test based on $rc. Now, kill -s 0 is generally fine to test whether a process with that pid is currently running, but the problem with that approach is that it _will_ succeed on a process that is a zombie, which really is a failed resource. Thus, it's often a good idea to combine this with an additional test, preferably one where you can ask the daemon, or a client, whether the application is really operational. Does Firebird offer such a facility? 5. There's an easier way to do this: > if [ ! -x $OCF_RESKEY_binary ]; then > ocf_log err "fbguard binary $OCF_RESKEY_binary does not exist > or is not executable" > exit $OCF_ERR_INSTALLED > fi You just replace those 4 lines with "check_binary $OCF_RESKEY_binary". 6. Also, > getent passwd $OCF_RESKEY_user >/dev/null 2>&1 > if [ ! $? -eq 0 ]; then can just be replaced with if ! getent passwd $OCF_RESKEY_user >/dev/null 2>&1; then ...for additional brevity. As far as I'm concerned, once you submit a version with these issues fixed, this RA is in good shape to be merged. Lars, Dejan, and the other usual suspects, if you have any objections to pushing this, please let me know. Cheers, Florian
signature.asc
Description: OpenPGP digital signature
_______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
