[Cluster-devel] [PATCH] dlm: fix cross-platform error values

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Tue, 2008-01-15 at 07:15 +0100, Fabio M. Di Nitto wrote:
> Hi guys,
> 
> On Mon, 14 Jan 2008, Patrick Caulfeld wrote:
> 
> > Some errno values differ across platforms. So if we return things like
> > -EINPROGRESS from one node it can get misinterpreted or rejected on
> > another one.
> >
> > This patch fixes up the errno values passed on the wire so that they
> > match the x86 ones (so as not to break the protocol), and re-instates
> > the platform-specific ones at the other end.
> >
> > Many thanks to Fabio for testing this patch.
> >
> > Signed-Off-By: Patrick Caulfield <pcaulfie@redhat.com
> > Signed-off-by: Fabio M. Di Nitto <fabbione@ubuntu.com>
> >
> 
> while doing some more testing in strange situations, i noticed that we had 
> more values going down the pipe.
> 
> This patch (based on Patrick's one) makes absolutely sure that all the -E 
> we use around fs/dlm/* are converted before hitting the wire. Mostlikely 
> not even half of them will go that far, but it's easier to catch them all 
> than finding them one at a time.
> 
> values and switch/case are also sorted in ascending order for pure 
> consistency paranoia.
> 
> Signed-off-by: Fabio M. Di Nitto <fabbione@ubuntu.com>
> 
> Thanks
> Fabio
> 
Perhaps it would be worth creating a translation table rather than
listing each value in a switch statement? Also I'd suggest to add a
"default" so that you can catch any values that you didn't expect and
BUG(); or something. That way if someone adds a new error value in the
future, you'll catch it before it hits the wire,

Steve.

> diff --git a/fs/dlm/dlm_internal.h b/fs/dlm/dlm_internal.h
> index ec61bba..d0de852 100644
> --- a/fs/dlm/dlm_internal.h
> +++ b/fs/dlm/dlm_internal.h
> @@ -220,6 +220,27 @@ struct dlm_args {
>   #define DLM_IFL_USER		0x00000001
>   #define DLM_IFL_ORPHAN		0x00000002
> 
> +
> +/* arch-independant errno values */
> +
> +#define DLM_ERRNO_EDEADLK		 35
> +#define DLM_ERRNO_EBADE			 52
> +#define DLM_ERRNO_EBADR			 53
> +#define DLM_ERRNO_EBADSLT		 57
> +#define DLM_ERRNO_EPROTO		 71
> +#define DLM_ERRNO_EBADMSG		 74
> +#define DLM_ERRNO_EPROTONOSUPPORT	 93
> +#define DLM_ERRNO_EOPNOTSUPP		 95
> +#define DLM_ERRNO_EADDRINUSE		 98
> +#define DLM_ERRNO_ENETDOWN		100
> +#define DLM_ERRNO_ENETUNREACH		101
> +#define DLM_ERRNO_ECONNABORTED		103
> +#define DLM_ERRNO_ENOBUFS		105
> +#define DLM_ERRNO_ENOTCONN		107
> +#define DLM_ERRNO_ETIMEDOUT		110
> +#define DLM_ERRNO_EHOSTUNREACH		113
> +#define DLM_ERRNO_EINPROGRESS		115
> +
>   struct dlm_lkb {
>   	struct dlm_rsb		*lkb_resource;	/* the rsb */
>   	struct kref		lkb_ref;
> diff --git a/fs/dlm/util.c b/fs/dlm/util.c
> index 963889c..e0f0ad4 100644
> --- a/fs/dlm/util.c
> +++ b/fs/dlm/util.c
> @@ -36,6 +36,61 @@ void dlm_message_out(struct dlm_message *ms)
> 
>   	header_out(hd);
> 
> +	/* Use arch-independant errno values on the wire */
> +	switch (ms->m_result) {
> +	case -EDEADLK:
> +		ms->m_result = -DLM_ERRNO_EDEADLK;
> +		break;
> +	case -EBADE:
> +		ms->m_result = -DLM_ERRNO_EBADE;
> +		break;
> +	case -EBADR:
> +		ms->m_result = -DLM_ERRNO_EBADR;
> +		break;
> +	case -EBADSLT:
> +		ms->m_result = -DLM_ERRNO_EBADSLT;
> +		break;
> +	case -EPROTO:
> +		ms->m_result = -DLM_ERRNO_EPROTO;
> +		break;
> +	case -EBADMSG:
> +		ms->m_result = -DLM_ERRNO_EBADMSG;
> +		break;
> +	case -EPROTONOSUPPORT:
> +		ms->m_result = -DLM_ERRNO_EPROTONOSUPPORT;
> +		break;
> +	case -EOPNOTSUPP:
> +		ms->m_result = -DLM_ERRNO_EOPNOTSUPP;
> +		break;
> +	case -EADDRINUSE:
> +		ms->m_result = -DLM_ERRNO_EADDRINUSE;
> +		break;
> +	case -ENETDOWN:
> +		ms->m_result = -DLM_ERRNO_ENETDOWN;
> +		break;
> +	case -ENETUNREACH:
> +		ms->m_result = -DLM_ERRNO_ENETUNREACH;
> +		break;
> +	case -ECONNABORTED:
> +		ms->m_result = -DLM_ERRNO_ECONNABORTED;
> +		break;
> +	case -ENOBUFS:
> +		ms->m_result = -DLM_ERRNO_ENOBUFS;
> +		break;
> +	case -ENOTCONN:
> +		ms->m_result = -DLM_ERRNO_ENOTCONN;
> +		break;
> +	case -ETIMEDOUT:
> +		ms->m_result = -DLM_ERRNO_ETIMEDOUT;
> +		break;
> +	case -EHOSTUNREACH:
> +		ms->m_result = -DLM_ERRNO_EHOSTUNREACH;
> +		break;
> +	case -EINPROGRESS:
> +		ms->m_result = -DLM_ERRNO_EINPROGRESS;
> +		break;
> +	}
> +
>   	ms->m_type		= cpu_to_le32(ms->m_type);
>   	ms->m_nodeid		= cpu_to_le32(ms->m_nodeid);
>   	ms->m_pid		= cpu_to_le32(ms->m_pid);
> @@ -80,6 +135,60 @@ void dlm_message_in(struct dlm_message *ms)
>   	ms->m_bastmode		= le32_to_cpu(ms->m_bastmode);
>   	ms->m_asts		= le32_to_cpu(ms->m_asts);
>   	ms->m_result		= le32_to_cpu(ms->m_result);
> +
> +	switch (ms->m_result) {
> +	case -DLM_ERRNO_EDEADLK:
> +		ms->m_result = -EDEADLK;
> +		break;
> +	case -DLM_ERRNO_EBADE:
> +		ms->m_result = -EBADE;
> +		break;
> +	case -DLM_ERRNO_EBADR:
> +		ms->m_result = -EBADR;
> +		break;
> +	case -DLM_ERRNO_EBADSLT:
> +		ms->m_result = -EBADSLT;
> +		break;
> +	case -DLM_ERRNO_EPROTO:
> +		ms->m_result = -EPROTO;
> +		break;
> +	case -DLM_ERRNO_EBADMSG:
> +		ms->m_result = -EBADMSG;
> +		break;
> +	case -DLM_ERRNO_EPROTONOSUPPORT:
> +		ms->m_result = -EPROTONOSUPPORT;
> +		break;
> +	case -DLM_ERRNO_EOPNOTSUPP:
> +		ms->m_result = -EOPNOTSUPP;
> +		break;
> +	case -DLM_ERRNO_EADDRINUSE:
> +		ms->m_result = -EADDRINUSE;
> +		break;
> +	case -DLM_ERRNO_ENETDOWN:
> +		ms->m_result = -ENETDOWN;
> +		break;
> +	case -DLM_ERRNO_ENETUNREACH:
> +		ms->m_result = -ENETUNREACH;
> +		break;
> +	case -DLM_ERRNO_ECONNABORTED:
> +		ms->m_result = -ECONNABORTED;
> +		break;
> +	case -DLM_ERRNO_ENOBUFS:
> +		ms->m_result = -ENOBUFS;
> +		break;
> +	case -DLM_ERRNO_ENOTCONN:
> +		ms->m_result = -ENOTCONN;
> +		break;
> +	case -DLM_ERRNO_ETIMEDOUT:
> +		ms->m_result = -ETIMEDOUT;
> +		break;
> +	case -DLM_ERRNO_EHOSTUNREACH:
> +		ms->m_result = -EHOSTUNREACH;
> +		break;
> +	case -DLM_ERRNO_EINPROGRESS:
> +		ms->m_result = -EINPROGRESS;
> +		break;
> +	}
>   }
> 
>   static void rcom_lock_out(struct rcom_lock *rl)
> 
> --
> I'm going to make him an offer he can't refuse.