Hi,
On Mon, Mar 01, 2010 at 12:31:57PM +0100, Florian Haas wrote:
> Christian,
>
> I'm generally fine with most of the changes, thanks very much for your
> continued contribution. I have a few comments on your patch, though.
>
> > # Check for mandatory files presence and consistency
> > vmware_validate() {
> > if [ -z "`pidof vmware-hostd`" ]; then
> > - ocf_log warn "vmware-hostd is not running"
> > - return $OCF_ERR_GENERIC
> > + ocf_log err "vmware-hostd is not running: aborting"
> > + exit $OCF_ERR_GENERIC
> > fi
>
> Changing warn to err and return to exit is obviously fine, but you may
> actually want to make this $OCF_ERR_INSTALLED.
Not sure, but since this is a check for the process running,
ERR_GENERIC would be better. The software may be installed, but
only not running.
> > @@ -253,12 +260,20 @@
> > else
> > ocf_log info "Virtual machine $VM is already stopped"
> > fi
> > + # VMware randomly fails to unregister VMs,
> > + # so we send the command twice
> > ocf_log info "Unregistering virtual machine $VM"
> > vmware_unregister_vm $VMID
> > VMID=`vmware_get_vid "$RELVMXPATH"`
> > if [ -n "$VMID" ]; then
> > - ocf_log err "Could not unregister virtual machine $VM: aborting."
> > - exit $OCF_ERR_GENERIC
> > + ocf_log warn "Could not unregister virtual machine $VM: retrying."
> > + sleep 10
> > + vmware_unregister_vm $VMID
> > + VMID=`vmware_get_vid "$RELVMXPATH"`
> > + if [ -n "$VMID" ]; then
> > + ocf_log err "Could not unregister virtual machine $VM: aborting."
> > + exit $OCF_ERR_GENERIC
> > + fi
> > fi
> > ocf_log info "Virtual machine $VM is stopped"
> > return $OCF_SUCCESS
>
> If you are running into VMware "randomly unregistering" VMs, can you be
> confident that it'll work on the second try? Wouldn't it be smarter to
> just keep trying until you are timed out?
Agreed, always better to keep trying (in case it's an intermittent
error) and let the upper layer timeout the action.
[...]
Cheers,
Dejan
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
