Hi Lars,

  Thanks for the review. ;) Below is a little explain about your concerns.

A updated version is attached with following changes:

  . return OCF_ERR_CONFIGURED in clone mode, to indicate it doesn't
support clone mode.
  . validate before start/stop/monitor
  . use check_binary
  . ocf_log debug output from init_script
  . umount rpc_pipefs when unbinding the tree

2008/10/22 Lars Marowsky-Bree <[EMAIL PROTECTED]>:
> On 2008-10-21T10:56:41, Xinwei Hu <[EMAIL PROTECTED]> wrote:
>
> Hi Xinwei,
>
> - How is it stacked with IPaddr? It seems to require the IPaddr to be
>  online before - but if IPaddr is running w/o the server being up yet,
>  there's a time where clients would get connection refused errors.

Yes, the nfsserver have to stack with IPaddr.
For the newly started client, you either have a error of "Program not
registered" when nfsserver not started yet, or a error of "No route to
host" when IPaddr not started yet. I think there's no difference from
the client's point of view ;)
For the already connected clients, it seems the clients will hang
until the server notifies the change.

>
>> #!/bin/sh
>
> Really sh, or bash? (bash is fine, but if so, it should state that.) I
> am hazy on what is bash specific and what isn't, so I thought it's safer
> to ask ;-)

I removed all known bash extensions in the script as Dejan sugguested.
It should be sh, but I'm sure about that. (sh are symlinks to bash in
all my environments).

>> if [ -n "$OCF_DEBUG_LIBRARY" ]; then
>>     . $OCF_DEBUG_LIBRARY
>> else
>>     . ${OCF_ROOT}/resource.d/heartbeat/.ocf-shellfuncs
>> fi
>
> I wonder where that snippet comes from; do we actually have a debug
> library anywhere? I've seen it in one or two RAs, but just curious.

Me don't know either. I use a template to write resource agent, and
these lines are in the template for a long time already. ;)

>> <parameter name="nfs_ip" unique="0" required="1">
>> <longdesc lang="en">
>> The floating IP address used to access the the nfs service
>> </longdesc>
>> <shortdesc lang="en">
>> IP address.
>> </shortdesc>
>> <content type="string" default="*" />
>
> If this was called "ip" instead of "nfs_ip", it could reference the
> IPaddr parameter in the CIB (possible with pacemaker-1.0) and only be
> specified once.
>
> (We probably want to eventually have hints like that for the GUI to
> automatically support this ...)

Interesting. I haven't noticed that change, and thought we must assign
the refid manually. What if there're 2 IPaddr in the group then ?

>> #Adapte LSB status code to OCF return code
>>       if [ $rc -eq 0 ]; then
>>               return $OCF_SUCCESS
>>       elif [ $rc -eq 3 ]; then
>>               return $OCF_NOT_RUNNING
>>       else
>>               return $OCF_ERR_GENERIC
>>       fi
>> }
>
> Should this handle "unused" as a possible return code translated to
> ERR_INSTALLED or ERR_CONFIGURED?

er, isn't "unused" a LSB's term for NOT_RUNNING?

>> is_bound ()
>> {
>>       mount | grep -q "$1 on $2 type none (.*bind)"
>>       return $?
>> }
>
> I have no perfect proof, but this strikes me as fragile, ie if bind is
> not the last option etc.
>
>> prepare_tree ()
>> {
>>       fp="$OCF_RESKEY_nfs_shared_infodir"
>>       if is_bound $fp /var/lib/nfs; then
>>               ocf_log debug "$fp is already bound to /var/lib/nfs"
>>               return 0
>>       fi
>>       mount --bind $fp /var/lib/nfs
>> }
>
> Why a bind mount instead of possibly symlinking the shared space to
> this, or directly mounting the shared fs or something else?
>
> (No criticism; just trying to understand the reasons.)

The reason to use bind mounting is that it won't persist between
reboots, and also preserves the local data.
Directly mounting will require a separated fs for this purposes, while
bind mounting requires a directory only.
It's more clean to use bind mount then use symlink to me ;)

>> nfsserver_start ()
>> {
>>       prepare_directory
>>       prepare_tree
>>
>>       cp -f /var/lib/nfs/sm/* /var/lib/nfs/sm.ha 2>&1 > /dev/null
>
> What about left-over files in sm.ha? Should that directory be cleaned
> out first?

The cleanup code is in the below. But you have the point, I moved that
line up here. ;)

> What happens when this is called twice? Does it pass ocf-tester?

It passed ocf-test -L.

Attachment: nfsserver
Description: Binary data

_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to