Re: [PATCH] ceph: decoding error in ceph_update_snap_realm should return -EIO

[Ilya Dryomov <idryomov@gmail.com>]

On Thu, Jun 3, 2021 at 3:39 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> Currently ceph_update_snap_realm returns -EINVAL when it hits a decoding
> error, which is the wrong error code. -EINVAL implies that the user gave
> us a bogus argument to a syscall or something similar. -EIO is more
> descriptive when we hit a decoding error.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/snap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
> index d07c1c6ac8fb..f8cac2abab3f 100644
> --- a/fs/ceph/snap.c
> +++ b/fs/ceph/snap.c
> @@ -807,7 +807,7 @@ int ceph_update_snap_trace(struct ceph_mds_client *mdsc,
>         return 0;
>
>  bad:
> -       err = -EINVAL;
> +       err = -EIO;
>  fail:
>         if (realm && !IS_ERR(realm))
>                 ceph_put_snap_realm(mdsc, realm);

Hi Jeff,

Is this error code propagated anywhere important?

The vast majority of functions that have something to do with decoding
use EINVAL as a default (usually out-of-bounds) error.  I agree that it
is totally ambiguous, but EIO doesn't seem to be any better to me.  If
there is a desire to separate these errors, I think we need to pick
something much more distinctive.

Thanks,

                Ilya