On Thu, 19 Apr 2007, Xinwei Hu wrote:
> As I asked on the IRC channel last night, there's no real blocker if we have
> a standalone pingd.sh (without ha.cf involved anymore).
>
> So here attached my first bash version of pingd. It works well for a day :)
>
> The known issue is that I don't know how to daemonize in bash,
> so the pingd RA needs a little tweak also.
>
> Comments are welcome.
OK. Here are some portability considerations, based on a quick glance
through the code.
This might seem negative. It is not intended that way! Keep up the
good work.
But they are details that will need attention at some point as your
project progresses.
1. Your email above says "bash". Your file starts "#!/bin/sh", which is
Bourne shell. On a few systems, such as Linux, Bourne and bash might just
happen to be equivalent, but in the world of portable programming they are
not: bash ("Bourne Again SHell") is Bourne with extensions.
Portability strongly urges true Bourne shell, rather than bash. For
portable software, such as heartbeat, it is necessary to keep to true
Bourne (i.e. "sh"), avoiding the bash extensions.
2. "export LC_ALL=C" (and likewise "export LANG=C"). That's a bash-ism.
In Bourne:
LC_ALL=C; export LC_ALL
LANG=C; export LANG=C
3. "attrd_updater=/usr/sbin/attrd_updater": "attrd_updater" gets
installed in different places in different systems. The mechanism for
handling this in autoconf projects (such as heartbeat) is routine. When
your "pingd.sh" is close to being ready for installing into the heartbeat
source tree, it will need to be abstrated into a "pingd.sh.in" file (and a
corresponding adjustment made to "configure.in").
4. "TEMP=`getopt ...`. There is "getopt", and there is "getopts". The
heartbeat source currently includes one of each. Their more obscure
details can vary from system to system. So keep it simple; don't try to
do too much with the "getopt{,s}" call. The "-l ..." and "-n ..." are
not available on some systems. At least on Solaris "getopts" (with the
"s") is preferred. Also this same "getopts" (with the "s") then makes
argument parsing easier, with its "OPTARG" and "OPTIND".
5. "ping -q -c 1 $ping_host". The options for "ping" are notoriously
variable from system to system. Keep it simple. (For example my system
doesn't have a "-q" option; and it says that "-c <n>" is for a thing
called "traffic class", only valid on IPv6.) If they are not necessary,
leave them out. If they are necessary, then for those of us who come
along later to maintain code, especially on other operating systems, it is
worth adding comments about your intentions, such as:
# -q: to do foo
# -c <n> to do bar
6. "date +%s": My OS doesn't support the "+%s" option. Could you clarify
your intention?
There are probably some others that I've missed on this quick glance.
Another: 'pid_file="/tmp/pingd.pid': I suspect that this file should
probably be in one of the heartbeat runtime locations rather than /tmp.
If someone can identify the best location, then the autoconfiscation of
your code at the appropriate time will be able to handle this.
Sorry for that list! Please don't take it too negatively. They are
details that are better addressed before the code is committed (which I
hope it will be some time!) and while it is still fresh in the mind.
Best wishes.
--
: David Lee I.T. Service :
: Senior Systems Programmer Computer Centre :
: UNIX Team Leader Durham University :
: South Road :
: http://www.dur.ac.uk/t.d.lee/ Durham DH1 3LE :
: Phone: +44 191 334 2752 U.K. :
_______________________________________________
Linux-HA mailing list
[email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha
See also: http://linux-ha.org/ReportingProblems
