On Jul 9, 2012, at 3:36 PM, Myklebust, Trond wrote: > On Mon, 2012-07-09 at 11:43 -0400, Chuck Lever wrote: >> The TEST_STATEID and FREE_STATEID operations can return >> NFS4ERR_BAD_STATEID, NFS4ERR_OLD_STATEID, or NFS4ERR_DEADSESSION. >> These error values should not be passed to nfs4_handle_exception(), >> since that will recursively kick off state recovery, resulting in a >> deadlock. The reason to continue using nfs4_handle_exception() is >> to deal with NFS4ERR_DELAY. >> >> Moreover, specifically when the TEST_STATEID operation returns >> NFS4_OK, res.status can contain one of these errors. >> _nfs41_test_stateid() replaces NFS4_OK with the value in res.status, >> which is then returned to callers. >> >> But res.status is not passed through nfs4_stat_to_errno(), and thus is >> a positive NFS4ERR value. Currently callers are only interested in >> !NFS4_OK, and nfs4_handle_exception() ignores positive values, so this >> oversight hasn't bitten us so far. >> >> Bryan agrees the original intent was to return res.status as a >> negative NFS4ERR value to callers of nfs41_test_stateid(), as long as >> no state ID recovery is attempted. > > None of this describes a cleanup... AFAICS it is all a bugfix IIRC behavior isn't supposed to change, so I don't think this should go to -stable. The subsequent patches rely on the error path being corrected, though. >> As a finishing touch, add appropriate documenting comments and some >> debugging printk's. > > This should be a separate patch so that we can send the bugfix part to > stable@vger if needed... I can redrive as two patches, but we should be careful about back porting, IMO. -- Chuck Lever chuck[dot]lever[at]oracle[dot]com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
