Trond, we had a discussion today in Austin about the COMPOUND result header
status.  Looking at fs/nfs/nfs4xdr.c we saw that some decoding routines
return an error based on hdr.status after all the compound operations
decoding completed successfully.

For example:
static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, __be32 *p, 
struct nfs_fsinfo *fsinfo)
{
        struct xdr_stream xdr;
        struct compound_hdr hdr;
        int status;

        xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
        status = decode_compound_hdr(&xdr, &hdr);
        if (!status)
                status = decode_setclientid_confirm(&xdr);
        if (!status)
                status = decode_putrootfh(&xdr);
        if (!status)
                status = decode_fsinfo(&xdr, fsinfo);
        if (!status)
                status = -nfs4_stat_to_errno(hdr.status);
                                             ^^^^^^^^^^
        return status;
}

It seems like this is unneeded since hdr.status must be NFS_OK if all operations
in the compound succeeded (and assuming we decode the results from all ops we 
sent).
The first error will be found by decode_op_hdr called by any of the decoding
routines for the individual ops.
If hdr.status contains an error in this case, the server must be broken, isn't 
it?

Also, nfs4_stat_to_errno is not handling the success case very efficiently as
it looks for it in its mapping table like any other error code.

One more thing, all use sites for nfs4_stat_to_errno negate its return value
so it might make sense to return a negative error from nfs4_stat_to_errno rather
than negating its return value everywhere.

Benny

-
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to