* [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED @ 2017-01-22 19:04 Chuck Lever 2017-01-23 15:01 ` Chuck Lever 0 siblings, 1 reply; 14+ messages in thread From: Chuck Lever @ 2017-01-22 19:04 UTC (permalink / raw) To: linux-nfs Xuan Qi reports that the Linux NFSv4 client failed to lock a file that was migrated. The steps he observed on the wire: 1. The client sent a LOCK request 2. The server replied NFS4ERR_MOVED 3. The client switched to the destination server 4. The client sent the LOCK request again with a bumped lock sequence ID 5. The server rejected the LOCK request with NFS4ERR_BAD_SEQID RFC 3530 section 8.1.5 provides a list of NFS errors which do not bump a lock sequence ID. However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 section 9.1.7, this list has been updated by the addition of NFS4ERR_MOVED. Reported-by: Xuan Qi <xuan.qi@oracle.com> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Cc: stable@vger.kernel.org # v3.7+ --- include/linux/nfs4.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h index bca5363..1b1ca04 100644 --- a/include/linux/nfs4.h +++ b/include/linux/nfs4.h @@ -282,7 +282,7 @@ enum nfsstat4 { static inline bool seqid_mutating_err(u32 err) { - /* rfc 3530 section 8.1.5: */ + /* See RFC 7530, section 9.1.7 */ switch (err) { case NFS4ERR_STALE_CLIENTID: case NFS4ERR_STALE_STATEID: @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) case NFS4ERR_BADXDR: case NFS4ERR_RESOURCE: case NFS4ERR_NOFILEHANDLE: + case NFS4ERR_MOVED: return false; }; return true; ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-22 19:04 [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED Chuck Lever @ 2017-01-23 15:01 ` Chuck Lever 2017-01-23 16:49 ` J. Bruce Fields 0 siblings, 1 reply; 14+ messages in thread From: Chuck Lever @ 2017-01-23 15:01 UTC (permalink / raw) To: Linux NFS Mailing List > On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > Xuan Qi reports that the Linux NFSv4 client failed to lock a file > that was migrated. The steps he observed on the wire: > > 1. The client sent a LOCK request > 2. The server replied NFS4ERR_MOVED > 3. The client switched to the destination server > 4. The client sent the LOCK request again with a bumped > lock sequence ID > 5. The server rejected the LOCK request with NFS4ERR_BAD_SEQID The list of steps could be more clear: 1. The client sent a LOCK request to the source server 2. The source server replied NFS4ERR_MOVED 3. The client switched to the destination server 4. The client sent the same LOCK request to the destination server with a bumped lock sequence ID 5. The destination server rejected the LOCK request with NFS4ERR_BAD_SEQID > RFC 3530 section 8.1.5 provides a list of NFS errors which do not > bump a lock sequence ID. > > However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 section > 9.1.7, this list has been updated by the addition of NFS4ERR_MOVED. > > Reported-by: Xuan Qi <xuan.qi@oracle.com> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > Cc: stable@vger.kernel.org # v3.7+ > --- > include/linux/nfs4.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > index bca5363..1b1ca04 100644 > --- a/include/linux/nfs4.h > +++ b/include/linux/nfs4.h > @@ -282,7 +282,7 @@ enum nfsstat4 { > > static inline bool seqid_mutating_err(u32 err) > { > - /* rfc 3530 section 8.1.5: */ > + /* See RFC 7530, section 9.1.7 */ > switch (err) { > case NFS4ERR_STALE_CLIENTID: > case NFS4ERR_STALE_STATEID: > @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) > case NFS4ERR_BADXDR: > case NFS4ERR_RESOURCE: > case NFS4ERR_NOFILEHANDLE: > + case NFS4ERR_MOVED: > return false; > }; > return true; > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-23 15:01 ` Chuck Lever @ 2017-01-23 16:49 ` J. Bruce Fields 2017-01-24 19:06 ` Chuck Lever 0 siblings, 1 reply; 14+ messages in thread From: J. Bruce Fields @ 2017-01-23 16:49 UTC (permalink / raw) To: Chuck Lever; +Cc: Linux NFS Mailing List On Mon, Jan 23, 2017 at 10:01:27AM -0500, Chuck Lever wrote: > > > On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > > > Xuan Qi reports that the Linux NFSv4 client failed to lock a file > > that was migrated. The steps he observed on the wire: > > > > 1. The client sent a LOCK request > > 2. The server replied NFS4ERR_MOVED > > 3. The client switched to the destination server > > 4. The client sent the LOCK request again with a bumped > > lock sequence ID > > 5. The server rejected the LOCK request with NFS4ERR_BAD_SEQID > > The list of steps could be more clear: > > 1. The client sent a LOCK request to the source server > 2. The source server replied NFS4ERR_MOVED > 3. The client switched to the destination server > 4. The client sent the same LOCK request to the destination > server with a bumped lock sequence ID > 5. The destination server rejected the LOCK request with > NFS4ERR_BAD_SEQID > > > > RFC 3530 section 8.1.5 provides a list of NFS errors which do not > > bump a lock sequence ID. > > > > However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 section > > 9.1.7, this list has been updated by the addition of NFS4ERR_MOVED. I guess we figured the backwards-incompatible change was OK since essentially the Solaris server is the first we know of to be making real use of NFS4ERR_MOVED? And probably it's required for the their implementation because the old server no longer has the ability to update the state once it's reached the point of returning ERR_MOVED. OK, makes sense to me, I think. --b. > > > > Reported-by: Xuan Qi <xuan.qi@oracle.com> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > > Cc: stable@vger.kernel.org # v3.7+ > > --- > > include/linux/nfs4.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > > index bca5363..1b1ca04 100644 > > --- a/include/linux/nfs4.h > > +++ b/include/linux/nfs4.h > > @@ -282,7 +282,7 @@ enum nfsstat4 { > > > > static inline bool seqid_mutating_err(u32 err) > > { > > - /* rfc 3530 section 8.1.5: */ > > + /* See RFC 7530, section 9.1.7 */ > > switch (err) { > > case NFS4ERR_STALE_CLIENTID: > > case NFS4ERR_STALE_STATEID: > > @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) > > case NFS4ERR_BADXDR: > > case NFS4ERR_RESOURCE: > > case NFS4ERR_NOFILEHANDLE: > > + case NFS4ERR_MOVED: > > return false; > > }; > > return true; > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > Chuck Lever > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-23 16:49 ` J. Bruce Fields @ 2017-01-24 19:06 ` Chuck Lever 2017-01-24 19:15 ` J. Bruce Fields 0 siblings, 1 reply; 14+ messages in thread From: Chuck Lever @ 2017-01-24 19:06 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Linux NFS Mailing List > On Jan 23, 2017, at 11:49 AM, bfields@fieldses.org wrote: > > On Mon, Jan 23, 2017 at 10:01:27AM -0500, Chuck Lever wrote: >> >>> On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>> >>> Xuan Qi reports that the Linux NFSv4 client failed to lock a file >>> that was migrated. The steps he observed on the wire: >>> >>> 1. The client sent a LOCK request >>> 2. The server replied NFS4ERR_MOVED >>> 3. The client switched to the destination server >>> 4. The client sent the LOCK request again with a bumped >>> lock sequence ID >>> 5. The server rejected the LOCK request with NFS4ERR_BAD_SEQID >> >> The list of steps could be more clear: >> >> 1. The client sent a LOCK request to the source server >> 2. The source server replied NFS4ERR_MOVED >> 3. The client switched to the destination server >> 4. The client sent the same LOCK request to the destination >> server with a bumped lock sequence ID >> 5. The destination server rejected the LOCK request with >> NFS4ERR_BAD_SEQID >> >> >>> RFC 3530 section 8.1.5 provides a list of NFS errors which do not >>> bump a lock sequence ID. >>> >>> However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 section >>> 9.1.7, this list has been updated by the addition of NFS4ERR_MOVED. > > I guess we figured the backwards-incompatible change was OK since > essentially the Solaris server is the first we know of to be making real > use of NFS4ERR_MOVED? > > And probably it's required for the their implementation because the old > server no longer has the ability to update the state once it's reached > the point of returning ERR_MOVED. > > OK, makes sense to me, I think. Hi Bruce- Does this mean you will take this patch, or should I just add your Reviewed-by: ? > --b. > >>> >>> Reported-by: Xuan Qi <xuan.qi@oracle.com> >>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >>> Cc: stable@vger.kernel.org # v3.7+ >>> --- >>> include/linux/nfs4.h | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h >>> index bca5363..1b1ca04 100644 >>> --- a/include/linux/nfs4.h >>> +++ b/include/linux/nfs4.h >>> @@ -282,7 +282,7 @@ enum nfsstat4 { >>> >>> static inline bool seqid_mutating_err(u32 err) >>> { >>> - /* rfc 3530 section 8.1.5: */ >>> + /* See RFC 7530, section 9.1.7 */ >>> switch (err) { >>> case NFS4ERR_STALE_CLIENTID: >>> case NFS4ERR_STALE_STATEID: >>> @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) >>> case NFS4ERR_BADXDR: >>> case NFS4ERR_RESOURCE: >>> case NFS4ERR_NOFILEHANDLE: >>> + case NFS4ERR_MOVED: >>> return false; >>> }; >>> return true; >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> -- >> Chuck Lever >> >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 19:06 ` Chuck Lever @ 2017-01-24 19:15 ` J. Bruce Fields 2017-01-24 19:31 ` Chuck Lever 2017-01-24 20:23 ` Trond Myklebust 0 siblings, 2 replies; 14+ messages in thread From: J. Bruce Fields @ 2017-01-24 19:15 UTC (permalink / raw) To: Chuck Lever; +Cc: Linux NFS Mailing List On Tue, Jan 24, 2017 at 02:06:16PM -0500, Chuck Lever wrote: > > > On Jan 23, 2017, at 11:49 AM, bfields@fieldses.org wrote: > > > > On Mon, Jan 23, 2017 at 10:01:27AM -0500, Chuck Lever wrote: > >> > >>> On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > >>> > >>> Xuan Qi reports that the Linux NFSv4 client failed to lock a file > >>> that was migrated. The steps he observed on the wire: > >>> > >>> 1. The client sent a LOCK request > >>> 2. The server replied NFS4ERR_MOVED > >>> 3. The client switched to the destination server > >>> 4. The client sent the LOCK request again with a bumped > >>> lock sequence ID > >>> 5. The server rejected the LOCK request with NFS4ERR_BAD_SEQID > >> > >> The list of steps could be more clear: > >> > >> 1. The client sent a LOCK request to the source server > >> 2. The source server replied NFS4ERR_MOVED > >> 3. The client switched to the destination server > >> 4. The client sent the same LOCK request to the destination > >> server with a bumped lock sequence ID > >> 5. The destination server rejected the LOCK request with > >> NFS4ERR_BAD_SEQID > >> > >> > >>> RFC 3530 section 8.1.5 provides a list of NFS errors which do not > >>> bump a lock sequence ID. > >>> > >>> However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 section > >>> 9.1.7, this list has been updated by the addition of NFS4ERR_MOVED. > > > > I guess we figured the backwards-incompatible change was OK since > > essentially the Solaris server is the first we know of to be making real > > use of NFS4ERR_MOVED? > > > > And probably it's required for the their implementation because the old > > server no longer has the ability to update the state once it's reached > > the point of returning ERR_MOVED. > > > > OK, makes sense to me, I think. > > Hi Bruce- > > Does this mean you will take this patch, or should > I just add your Reviewed-by: ? I can take it if nobody objects. Mind if I append the above to the changelog? (Just want to document why we think the apparently backwards-incompatible change is OK.) --b. > > > > --b. > > > >>> > >>> Reported-by: Xuan Qi <xuan.qi@oracle.com> > >>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > >>> Cc: stable@vger.kernel.org # v3.7+ > >>> --- > >>> include/linux/nfs4.h | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > >>> index bca5363..1b1ca04 100644 > >>> --- a/include/linux/nfs4.h > >>> +++ b/include/linux/nfs4.h > >>> @@ -282,7 +282,7 @@ enum nfsstat4 { > >>> > >>> static inline bool seqid_mutating_err(u32 err) > >>> { > >>> - /* rfc 3530 section 8.1.5: */ > >>> + /* See RFC 7530, section 9.1.7 */ > >>> switch (err) { > >>> case NFS4ERR_STALE_CLIENTID: > >>> case NFS4ERR_STALE_STATEID: > >>> @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) > >>> case NFS4ERR_BADXDR: > >>> case NFS4ERR_RESOURCE: > >>> case NFS4ERR_NOFILEHANDLE: > >>> + case NFS4ERR_MOVED: > >>> return false; > >>> }; > >>> return true; > >>> > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > >>> the body of a message to majordomo@vger.kernel.org > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> > >> -- > >> Chuck Lever > >> > >> > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > Chuck Lever > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 19:15 ` J. Bruce Fields @ 2017-01-24 19:31 ` Chuck Lever 2017-01-24 19:41 ` J. Bruce Fields 2017-01-24 20:23 ` Trond Myklebust 1 sibling, 1 reply; 14+ messages in thread From: Chuck Lever @ 2017-01-24 19:31 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Linux NFS Mailing List > On Jan 24, 2017, at 2:15 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > On Tue, Jan 24, 2017 at 02:06:16PM -0500, Chuck Lever wrote: >> >>> On Jan 23, 2017, at 11:49 AM, bfields@fieldses.org wrote: >>> >>> On Mon, Jan 23, 2017 at 10:01:27AM -0500, Chuck Lever wrote: >>>> >>>>> On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>>>> >>>>> Xuan Qi reports that the Linux NFSv4 client failed to lock a file >>>>> that was migrated. The steps he observed on the wire: >>>>> >>>>> 1. The client sent a LOCK request >>>>> 2. The server replied NFS4ERR_MOVED >>>>> 3. The client switched to the destination server >>>>> 4. The client sent the LOCK request again with a bumped >>>>> lock sequence ID >>>>> 5. The server rejected the LOCK request with NFS4ERR_BAD_SEQID >>>> >>>> The list of steps could be more clear: >>>> >>>> 1. The client sent a LOCK request to the source server >>>> 2. The source server replied NFS4ERR_MOVED >>>> 3. The client switched to the destination server >>>> 4. The client sent the same LOCK request to the destination >>>> server with a bumped lock sequence ID >>>> 5. The destination server rejected the LOCK request with >>>> NFS4ERR_BAD_SEQID >>>> >>>> >>>>> RFC 3530 section 8.1.5 provides a list of NFS errors which do not >>>>> bump a lock sequence ID. >>>>> >>>>> However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 section >>>>> 9.1.7, this list has been updated by the addition of NFS4ERR_MOVED. >>> >>> I guess we figured the backwards-incompatible change was OK since >>> essentially the Solaris server is the first we know of to be making real >>> use of NFS4ERR_MOVED? >>> >>> And probably it's required for the their implementation because the old >>> server no longer has the ability to update the state once it's reached >>> the point of returning ERR_MOVED. >>> >>> OK, makes sense to me, I think. >> >> Hi Bruce- >> >> Does this mean you will take this patch, or should >> I just add your Reviewed-by: ? > > I can take it if nobody objects. Mind if I append the above to the > changelog? (Just want to document why we think the apparently > backwards-incompatible change is OK.) Adding a justification is OK with me, and please replace the list of steps with my updated list above. However, your explanation implies that Solaris is the only server that might need this fix. Actually _any_ server that supports transparent state migration needs clients to get this fix. Lock operations on a file that has moved are not able to update the sequence ID on the destination server. This backwards-compatible change is OK because: - No servers in the wild support migration yet, thus NFS4ERR_MOVED is never returned by existing servers - Clients that do not support migration should never receive NFS4ERR_MOVED on a state-mutating operation In other words, this change is necessary only for clients that support TSM. Salt to taste. > --b. > >> >> >>> --b. >>> >>>>> >>>>> Reported-by: Xuan Qi <xuan.qi@oracle.com> >>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >>>>> Cc: stable@vger.kernel.org # v3.7+ >>>>> --- >>>>> include/linux/nfs4.h | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h >>>>> index bca5363..1b1ca04 100644 >>>>> --- a/include/linux/nfs4.h >>>>> +++ b/include/linux/nfs4.h >>>>> @@ -282,7 +282,7 @@ enum nfsstat4 { >>>>> >>>>> static inline bool seqid_mutating_err(u32 err) >>>>> { >>>>> - /* rfc 3530 section 8.1.5: */ >>>>> + /* See RFC 7530, section 9.1.7 */ >>>>> switch (err) { >>>>> case NFS4ERR_STALE_CLIENTID: >>>>> case NFS4ERR_STALE_STATEID: >>>>> @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) >>>>> case NFS4ERR_BADXDR: >>>>> case NFS4ERR_RESOURCE: >>>>> case NFS4ERR_NOFILEHANDLE: >>>>> + case NFS4ERR_MOVED: >>>>> return false; >>>>> }; >>>>> return true; >>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>>> -- >>>> Chuck Lever >>>> >>>> >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> -- >> Chuck Lever >> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 19:31 ` Chuck Lever @ 2017-01-24 19:41 ` J. Bruce Fields 2017-01-24 19:53 ` J. Bruce Fields 2017-01-24 19:54 ` Chuck Lever 0 siblings, 2 replies; 14+ messages in thread From: J. Bruce Fields @ 2017-01-24 19:41 UTC (permalink / raw) To: Chuck Lever; +Cc: Linux NFS Mailing List On Tue, Jan 24, 2017 at 02:31:37PM -0500, Chuck Lever wrote: > Adding a justification is OK with me, and please replace the > list of steps with my updated list above. > > However, your explanation implies that Solaris is the only server > that might need this fix. Actually _any_ server that supports > transparent state migration needs clients to get this fix. Lock > operations on a file that has moved are not able to update the > sequence ID on the destination server. Are you sure? Couldn't an implementation include a server-to-server protocol that allowed the source and destination server to share stateid information? But even if that's possible, it may be unnecessarily complicated, so I agree I shouldn't be claiming it's a Solaris-specific issue (though it may be worth documenting that's who first hit this). --b. > This backwards-compatible change is OK because: > > - No servers in the wild support migration yet, thus > NFS4ERR_MOVED is never returned by existing servers > > - Clients that do not support migration should never receive > NFS4ERR_MOVED on a state-mutating operation > > In other words, this change is necessary only for clients that > support TSM. > > Salt to taste. > > > > --b. > > > >> > >> > >>> --b. > >>> > >>>>> > >>>>> Reported-by: Xuan Qi <xuan.qi@oracle.com> > >>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > >>>>> Cc: stable@vger.kernel.org # v3.7+ > >>>>> --- > >>>>> include/linux/nfs4.h | 3 ++- > >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > >>>>> index bca5363..1b1ca04 100644 > >>>>> --- a/include/linux/nfs4.h > >>>>> +++ b/include/linux/nfs4.h > >>>>> @@ -282,7 +282,7 @@ enum nfsstat4 { > >>>>> > >>>>> static inline bool seqid_mutating_err(u32 err) > >>>>> { > >>>>> - /* rfc 3530 section 8.1.5: */ > >>>>> + /* See RFC 7530, section 9.1.7 */ > >>>>> switch (err) { > >>>>> case NFS4ERR_STALE_CLIENTID: > >>>>> case NFS4ERR_STALE_STATEID: > >>>>> @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) > >>>>> case NFS4ERR_BADXDR: > >>>>> case NFS4ERR_RESOURCE: > >>>>> case NFS4ERR_NOFILEHANDLE: > >>>>> + case NFS4ERR_MOVED: > >>>>> return false; > >>>>> }; > >>>>> return true; > >>>>> > >>>>> -- > >>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > >>>>> the body of a message to majordomo@vger.kernel.org > >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >>>> > >>>> -- > >>>> Chuck Lever > >>>> > >>>> > >>>> > >>>> -- > >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > >>>> the body of a message to majordomo@vger.kernel.org > >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > >>> the body of a message to majordomo@vger.kernel.org > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> > >> -- > >> Chuck Lever > >> > >> > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > Chuck Lever > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 19:41 ` J. Bruce Fields @ 2017-01-24 19:53 ` J. Bruce Fields 2017-01-24 19:58 ` Chuck Lever 2017-01-24 19:54 ` Chuck Lever 1 sibling, 1 reply; 14+ messages in thread From: J. Bruce Fields @ 2017-01-24 19:53 UTC (permalink / raw) To: Chuck Lever; +Cc: Linux NFS Mailing List On Tue, Jan 24, 2017 at 02:41:40PM -0500, J. Bruce Fields wrote: > On Tue, Jan 24, 2017 at 02:31:37PM -0500, Chuck Lever wrote: > > Adding a justification is OK with me, and please replace the > > list of steps with my updated list above. > > > > However, your explanation implies that Solaris is the only server > > that might need this fix. Actually _any_ server that supports > > transparent state migration needs clients to get this fix. Lock > > operations on a file that has moved are not able to update the > > sequence ID on the destination server. > > Are you sure? Couldn't an implementation include a server-to-server > protocol that allowed the source and destination server to share stateid > information? > > But even if that's possible, it may be unnecessarily complicated, so I > agree I shouldn't be claiming it's a Solaris-specific issue (though it > may be worth documenting that's who first hit this). > > --b. > > > This backwards-compatible change is OK because: > > > > - No servers in the wild support migration yet, thus > > NFS4ERR_MOVED is never returned by existing servers I think you mean "transparent state migration" there? A server supporting non-transparent state migration could return NFS4ERR_MOVED on a LOCK operation, but the client won't be able to use that stateid afterwards in that case anyway. > > - Clients that do not support migration should never receive > > NFS4ERR_MOVED on a state-mutating operation I didn't think there was a way for clients to advertise non-support for migration? But such clients could never recover from MOVED anyway, so we're not making things worse for them. --b. > > > > In other words, this change is necessary only for clients that > > support TSM. > > > > Salt to taste. > > > > > > > --b. > > > > > >> > > >> > > >>> --b. > > >>> > > >>>>> > > >>>>> Reported-by: Xuan Qi <xuan.qi@oracle.com> > > >>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > > >>>>> Cc: stable@vger.kernel.org # v3.7+ > > >>>>> --- > > >>>>> include/linux/nfs4.h | 3 ++- > > >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) > > >>>>> > > >>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > > >>>>> index bca5363..1b1ca04 100644 > > >>>>> --- a/include/linux/nfs4.h > > >>>>> +++ b/include/linux/nfs4.h > > >>>>> @@ -282,7 +282,7 @@ enum nfsstat4 { > > >>>>> > > >>>>> static inline bool seqid_mutating_err(u32 err) > > >>>>> { > > >>>>> - /* rfc 3530 section 8.1.5: */ > > >>>>> + /* See RFC 7530, section 9.1.7 */ > > >>>>> switch (err) { > > >>>>> case NFS4ERR_STALE_CLIENTID: > > >>>>> case NFS4ERR_STALE_STATEID: > > >>>>> @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) > > >>>>> case NFS4ERR_BADXDR: > > >>>>> case NFS4ERR_RESOURCE: > > >>>>> case NFS4ERR_NOFILEHANDLE: > > >>>>> + case NFS4ERR_MOVED: > > >>>>> return false; > > >>>>> }; > > >>>>> return true; > > >>>>> > > >>>>> -- > > >>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > >>>>> the body of a message to majordomo@vger.kernel.org > > >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > >>>> > > >>>> -- > > >>>> Chuck Lever > > >>>> > > >>>> > > >>>> > > >>>> -- > > >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > >>>> the body of a message to majordomo@vger.kernel.org > > >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > >>> -- > > >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > >>> the body of a message to majordomo@vger.kernel.org > > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > >> > > >> -- > > >> Chuck Lever > > >> > > >> > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > > Chuck Lever > > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 19:53 ` J. Bruce Fields @ 2017-01-24 19:58 ` Chuck Lever 0 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2017-01-24 19:58 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Linux NFS Mailing List > On Jan 24, 2017, at 2:53 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > On Tue, Jan 24, 2017 at 02:41:40PM -0500, J. Bruce Fields wrote: >> On Tue, Jan 24, 2017 at 02:31:37PM -0500, Chuck Lever wrote: >>> Adding a justification is OK with me, and please replace the >>> list of steps with my updated list above. >>> >>> However, your explanation implies that Solaris is the only server >>> that might need this fix. Actually _any_ server that supports >>> transparent state migration needs clients to get this fix. Lock >>> operations on a file that has moved are not able to update the >>> sequence ID on the destination server. >> >> Are you sure? Couldn't an implementation include a server-to-server >> protocol that allowed the source and destination server to share stateid >> information? >> >> But even if that's possible, it may be unnecessarily complicated, so I >> agree I shouldn't be claiming it's a Solaris-specific issue (though it >> may be worth documenting that's who first hit this). >> >> --b. >> >>> This backwards-compatible change is OK because: >>> >>> - No servers in the wild support migration yet, thus >>> NFS4ERR_MOVED is never returned by existing servers > > I think you mean "transparent state migration" there? Yes, though I'm not aware of any public implementations that support plain migration either. NFS4ERR_MOVED can also be returned in cases where servers want to advertise replicas, but we don't expect that status on seqid-mutating operations like LOCK. > A server supporting non-transparent state migration could return > NFS4ERR_MOVED on a LOCK operation, but the client won't be able to use > that stateid afterwards in that case anyway. > >>> - Clients that do not support migration should never receive >>> NFS4ERR_MOVED on a state-mutating operation > > I didn't think there was a way for clients to advertise non-support for > migration? Not in NFSv4.0, but NFSv4.1 has an EXCHANGE_ID flag that signifies client support for migration. > But such clients could never recover from MOVED anyway, so we're not > making things worse for them. Exactly. > --b. > >>> >>> In other words, this change is necessary only for clients that >>> support TSM. >>> >>> Salt to taste. >>> >>> >>>> --b. >>>> >>>>> >>>>> >>>>>> --b. >>>>>> >>>>>>>> >>>>>>>> Reported-by: Xuan Qi <xuan.qi@oracle.com> >>>>>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >>>>>>>> Cc: stable@vger.kernel.org # v3.7+ >>>>>>>> --- >>>>>>>> include/linux/nfs4.h | 3 ++- >>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h >>>>>>>> index bca5363..1b1ca04 100644 >>>>>>>> --- a/include/linux/nfs4.h >>>>>>>> +++ b/include/linux/nfs4.h >>>>>>>> @@ -282,7 +282,7 @@ enum nfsstat4 { >>>>>>>> >>>>>>>> static inline bool seqid_mutating_err(u32 err) >>>>>>>> { >>>>>>>> - /* rfc 3530 section 8.1.5: */ >>>>>>>> + /* See RFC 7530, section 9.1.7 */ >>>>>>>> switch (err) { >>>>>>>> case NFS4ERR_STALE_CLIENTID: >>>>>>>> case NFS4ERR_STALE_STATEID: >>>>>>>> @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) >>>>>>>> case NFS4ERR_BADXDR: >>>>>>>> case NFS4ERR_RESOURCE: >>>>>>>> case NFS4ERR_NOFILEHANDLE: >>>>>>>> + case NFS4ERR_MOVED: >>>>>>>> return false; >>>>>>>> }; >>>>>>>> return true; >>>>>>>> >>>>>>>> -- >>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>>>>> the body of a message to majordomo@vger.kernel.org >>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>>> >>>>>>> -- >>>>>>> Chuck Lever >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>>>> the body of a message to majordomo@vger.kernel.org >>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> -- >>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>>> the body of a message to majordomo@vger.kernel.org >>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>> >>>>> -- >>>>> Chuck Lever >>>>> >>>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> -- >>> Chuck Lever >>> >>> > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 19:41 ` J. Bruce Fields 2017-01-24 19:53 ` J. Bruce Fields @ 2017-01-24 19:54 ` Chuck Lever 1 sibling, 0 replies; 14+ messages in thread From: Chuck Lever @ 2017-01-24 19:54 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Linux NFS Mailing List > On Jan 24, 2017, at 2:41 PM, J. Bruce Fields <bfields@fieldses.org> wrote: > > On Tue, Jan 24, 2017 at 02:31:37PM -0500, Chuck Lever wrote: >> Adding a justification is OK with me, and please replace the >> list of steps with my updated list above. >> >> However, your explanation implies that Solaris is the only server >> that might need this fix. Actually _any_ server that supports >> transparent state migration needs clients to get this fix. Lock >> operations on a file that has moved are not able to update the >> sequence ID on the destination server. > > Are you sure? I'm pretty sure. > Couldn't an implementation include a server-to-server > protocol that allowed the source and destination server to share stateid > information? "Migration" means that the filesystem's name space and data content has moved, and is no longer accessible on this server. "Transparent State Migration" means that the filesystem's state was moved with its name space and data content. NFS4ERR_MOVED here means specifically that file and its state is no longer managed or accessible at the local server. It kind of implies categorically that the above situation is not in play. What you are describing is something that is not Transparent State Migration. Sounds more like replication. In which case, the server would report NFS4ERR_MOVED on a LOOKUP at the root of the filesystem, and not on a LOCK request. A client can recognize the difference between these two and react accordingly. > But even if that's possible, it may be unnecessarily complicated, so I > agree I shouldn't be claiming it's a Solaris-specific issue (though it > may be worth documenting that's who first hit this). > > --b. > >> This backwards-compatible change is OK because: >> >> - No servers in the wild support migration yet, thus >> NFS4ERR_MOVED is never returned by existing servers >> >> - Clients that do not support migration should never receive >> NFS4ERR_MOVED on a state-mutating operation >> >> In other words, this change is necessary only for clients that >> support TSM. >> >> Salt to taste. >> >> >>> --b. >>> >>>> >>>> >>>>> --b. >>>>> >>>>>>> >>>>>>> Reported-by: Xuan Qi <xuan.qi@oracle.com> >>>>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >>>>>>> Cc: stable@vger.kernel.org # v3.7+ >>>>>>> --- >>>>>>> include/linux/nfs4.h | 3 ++- >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h >>>>>>> index bca5363..1b1ca04 100644 >>>>>>> --- a/include/linux/nfs4.h >>>>>>> +++ b/include/linux/nfs4.h >>>>>>> @@ -282,7 +282,7 @@ enum nfsstat4 { >>>>>>> >>>>>>> static inline bool seqid_mutating_err(u32 err) >>>>>>> { >>>>>>> - /* rfc 3530 section 8.1.5: */ >>>>>>> + /* See RFC 7530, section 9.1.7 */ >>>>>>> switch (err) { >>>>>>> case NFS4ERR_STALE_CLIENTID: >>>>>>> case NFS4ERR_STALE_STATEID: >>>>>>> @@ -291,6 +291,7 @@ static inline bool seqid_mutating_err(u32 err) >>>>>>> case NFS4ERR_BADXDR: >>>>>>> case NFS4ERR_RESOURCE: >>>>>>> case NFS4ERR_NOFILEHANDLE: >>>>>>> + case NFS4ERR_MOVED: >>>>>>> return false; >>>>>>> }; >>>>>>> return true; >>>>>>> >>>>>>> -- >>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>>>> the body of a message to majordomo@vger.kernel.org >>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> >>>>>> -- >>>>>> Chuck Lever >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>>> the body of a message to majordomo@vger.kernel.org >>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>>> -- >>>> Chuck Lever >>>> >>>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> -- >> Chuck Lever >> >> -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 19:15 ` J. Bruce Fields 2017-01-24 19:31 ` Chuck Lever @ 2017-01-24 20:23 ` Trond Myklebust 2017-01-24 20:31 ` bfields 2017-01-25 19:58 ` Chuck Lever 1 sibling, 2 replies; 14+ messages in thread From: Trond Myklebust @ 2017-01-24 20:23 UTC (permalink / raw) To: bfields@fieldses.org, chuck.lever@oracle.com; +Cc: linux-nfs@vger.kernel.org T24gVHVlLCAyMDE3LTAxLTI0IGF0IDE0OjE1IC0wNTAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6 DQo+IE9uIFR1ZSwgSmFuIDI0LCAyMDE3IGF0IDAyOjA2OjE2UE0gLTA1MDAsIENodWNrIExldmVy IHdyb3RlOg0KPiA+IA0KPiA+ID4gT24gSmFuIDIzLCAyMDE3LCBhdCAxMTo0OSBBTSwgYmZpZWxk c0BmaWVsZHNlcy5vcmcgd3JvdGU6DQo+ID4gPiANCj4gPiA+IE9uIE1vbiwgSmFuIDIzLCAyMDE3 IGF0IDEwOjAxOjI3QU0gLTA1MDAsIENodWNrIExldmVyIHdyb3RlOg0KPiA+ID4gPiANCj4gPiA+ ID4gPiBPbiBKYW4gMjIsIDIwMTcsIGF0IDI6MDQgUE0sIENodWNrIExldmVyIDxjaHVjay5sZXZl ckBvcmFjbGUuDQo+ID4gPiA+ID4gY29tPiB3cm90ZToNCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBY dWFuIFFpIHJlcG9ydHMgdGhhdCB0aGUgTGludXggTkZTdjQgY2xpZW50IGZhaWxlZCB0byBsb2Nr IGENCj4gPiA+ID4gPiBmaWxlDQo+ID4gPiA+ID4gdGhhdCB3YXMgbWlncmF0ZWQuIFRoZSBzdGVw cyBoZSBvYnNlcnZlZCBvbiB0aGUgd2lyZToNCj4gPiA+ID4gPiANCj4gPiA+ID4gPiAxLiBUaGUg Y2xpZW50IHNlbnQgYSBMT0NLIHJlcXVlc3QNCj4gPiA+ID4gPiAyLiBUaGUgc2VydmVyIHJlcGxp ZWQgTkZTNEVSUl9NT1ZFRA0KPiA+ID4gPiA+IDMuIFRoZSBjbGllbnQgc3dpdGNoZWQgdG8gdGhl IGRlc3RpbmF0aW9uIHNlcnZlcg0KPiA+ID4gPiA+IDQuIFRoZSBjbGllbnQgc2VudCB0aGUgTE9D SyByZXF1ZXN0IGFnYWluIHdpdGggYSBidW1wZWQNCj4gPiA+ID4gPiDCoGxvY2sgc2VxdWVuY2Ug SUQNCj4gPiA+ID4gPiA1LiBUaGUgc2VydmVyIHJlamVjdGVkIHRoZSBMT0NLIHJlcXVlc3Qgd2l0 aA0KPiA+ID4gPiA+IE5GUzRFUlJfQkFEX1NFUUlEDQo+ID4gPiA+IA0KPiA+ID4gPiBUaGUgbGlz dCBvZiBzdGVwcyBjb3VsZCBiZSBtb3JlIGNsZWFyOg0KPiA+ID4gPiANCj4gPiA+ID4gMS4gVGhl IGNsaWVudCBzZW50IGEgTE9DSyByZXF1ZXN0IHRvIHRoZSBzb3VyY2Ugc2VydmVyDQo+ID4gPiA+ IDIuIFRoZSBzb3VyY2Ugc2VydmVyIHJlcGxpZWQgTkZTNEVSUl9NT1ZFRA0KPiA+ID4gPiAzLiBU aGUgY2xpZW50IHN3aXRjaGVkIHRvIHRoZSBkZXN0aW5hdGlvbiBzZXJ2ZXINCj4gPiA+ID4gNC4g VGhlIGNsaWVudCBzZW50IHRoZSBzYW1lIExPQ0sgcmVxdWVzdCB0byB0aGUgZGVzdGluYXRpb24N Cj4gPiA+ID4gwqAgc2VydmVyIHdpdGggYSBidW1wZWQgbG9jayBzZXF1ZW5jZSBJRA0KPiA+ID4g PiA1LiBUaGUgZGVzdGluYXRpb24gc2VydmVyIHJlamVjdGVkIHRoZSBMT0NLIHJlcXVlc3Qgd2l0 aA0KPiA+ID4gPiDCoCBORlM0RVJSX0JBRF9TRVFJRA0KPiA+ID4gPiANCj4gPiA+ID4gDQo+ID4g PiA+ID4gUkZDIDM1MzAgc2VjdGlvbiA4LjEuNSBwcm92aWRlcyBhIGxpc3Qgb2YgTkZTIGVycm9y cyB3aGljaCBkbw0KPiA+ID4gPiA+IG5vdA0KPiA+ID4gPiA+IGJ1bXAgYSBsb2NrIHNlcXVlbmNl IElELg0KPiA+ID4gPiA+IA0KPiA+ID4gPiA+IEhvd2V2ZXIsIFJGQyAzNTMwIGlzIG5vdyBvYnNv bGV0ZWQgYnkgUkZDIDc1MzAuIEluIFJGQyA3NTMwDQo+ID4gPiA+ID4gc2VjdGlvbg0KPiA+ID4g PiA+IDkuMS43LCB0aGlzIGxpc3QgaGFzIGJlZW4gdXBkYXRlZCBieSB0aGUgYWRkaXRpb24gb2YN Cj4gPiA+ID4gPiBORlM0RVJSX01PVkVELg0KPiA+ID4gDQo+ID4gPiBJIGd1ZXNzIHdlIGZpZ3Vy ZWQgdGhlIGJhY2t3YXJkcy1pbmNvbXBhdGlibGUgY2hhbmdlIHdhcyBPSyBzaW5jZQ0KPiA+ID4g ZXNzZW50aWFsbHkgdGhlIFNvbGFyaXMgc2VydmVyIGlzIHRoZSBmaXJzdCB3ZSBrbm93IG9mIHRv IGJlDQo+ID4gPiBtYWtpbmcgcmVhbA0KPiA+ID4gdXNlIG9mIE5GUzRFUlJfTU9WRUQ/DQo+ID4g PiANCj4gPiA+IEFuZCBwcm9iYWJseSBpdCdzIHJlcXVpcmVkIGZvciB0aGUgdGhlaXIgaW1wbGVt ZW50YXRpb24gYmVjYXVzZQ0KPiA+ID4gdGhlIG9sZA0KPiA+ID4gc2VydmVyIG5vIGxvbmdlciBo YXMgdGhlIGFiaWxpdHkgdG8gdXBkYXRlIHRoZSBzdGF0ZSBvbmNlIGl0J3MNCj4gPiA+IHJlYWNo ZWQNCj4gPiA+IHRoZSBwb2ludCBvZiByZXR1cm5pbmcgRVJSX01PVkVELg0KPiA+ID4gDQo+ID4g PiBPSywgbWFrZXMgc2Vuc2UgdG8gbWUsIEkgdGhpbmsuDQo+ID4gDQo+ID4gSGkgQnJ1Y2UtDQo+ ID4gDQo+ID4gRG9lcyB0aGlzIG1lYW4geW91IHdpbGwgdGFrZSB0aGlzIHBhdGNoLCBvciBzaG91 bGQNCj4gPiBJIGp1c3QgYWRkIHlvdXIgUmV2aWV3ZWQtYnk6ID8NCj4gDQo+IEkgY2FuIHRha2Ug aXQgaWYgbm9ib2R5IG9iamVjdHMuwqDCoE1pbmQgaWYgSSBhcHBlbmQgdGhlIGFib3ZlIHRvIHRo ZQ0KPiBjaGFuZ2Vsb2c/wqDCoChKdXN0IHdhbnQgdG8gZG9jdW1lbnQgd2h5IHdlIHRoaW5rIHRo ZSBhcHBhcmVudGx5DQo+IGJhY2t3YXJkcy1pbmNvbXBhdGlibGUgY2hhbmdlIGlzIE9LLikNCj4g DQpJJ3ZlIGFscmVhZHkgYWRkZWQgaXQgdG8gbXkgbGludXgtbmV4dCBicmFuY2ggYXMgYSBzdGFi bGUgcGF0Y2guDQoNCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFp bnRhaW5lciwgUHJpbWFyeURhdGENCnRyb25kLm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20NCg== ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 20:23 ` Trond Myklebust @ 2017-01-24 20:31 ` bfields 2017-01-25 19:58 ` Chuck Lever 1 sibling, 0 replies; 14+ messages in thread From: bfields @ 2017-01-24 20:31 UTC (permalink / raw) To: Trond Myklebust; +Cc: chuck.lever@oracle.com, linux-nfs@vger.kernel.org On Tue, Jan 24, 2017 at 08:23:36PM +0000, Trond Myklebust wrote: > On Tue, 2017-01-24 at 14:15 -0500, J. Bruce Fields wrote: > > On Tue, Jan 24, 2017 at 02:06:16PM -0500, Chuck Lever wrote: > > > > > > > On Jan 23, 2017, at 11:49 AM, bfields@fieldses.org wrote: > > > > > > > > On Mon, Jan 23, 2017 at 10:01:27AM -0500, Chuck Lever wrote: > > > > > > > > > > > On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle. > > > > > > com> wrote: > > > > > > > > > > > > Xuan Qi reports that the Linux NFSv4 client failed to lock a > > > > > > file > > > > > > that was migrated. The steps he observed on the wire: > > > > > > > > > > > > 1. The client sent a LOCK request > > > > > > 2. The server replied NFS4ERR_MOVED > > > > > > 3. The client switched to the destination server > > > > > > 4. The client sent the LOCK request again with a bumped > > > > > > lock sequence ID > > > > > > 5. The server rejected the LOCK request with > > > > > > NFS4ERR_BAD_SEQID > > > > > > > > > > The list of steps could be more clear: > > > > > > > > > > 1. The client sent a LOCK request to the source server > > > > > 2. The source server replied NFS4ERR_MOVED > > > > > 3. The client switched to the destination server > > > > > 4. The client sent the same LOCK request to the destination > > > > > server with a bumped lock sequence ID > > > > > 5. The destination server rejected the LOCK request with > > > > > NFS4ERR_BAD_SEQID > > > > > > > > > > > > > > > > RFC 3530 section 8.1.5 provides a list of NFS errors which do > > > > > > not > > > > > > bump a lock sequence ID. > > > > > > > > > > > > However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 > > > > > > section > > > > > > 9.1.7, this list has been updated by the addition of > > > > > > NFS4ERR_MOVED. > > > > > > > > I guess we figured the backwards-incompatible change was OK since > > > > essentially the Solaris server is the first we know of to be > > > > making real > > > > use of NFS4ERR_MOVED? > > > > > > > > And probably it's required for the their implementation because > > > > the old > > > > server no longer has the ability to update the state once it's > > > > reached > > > > the point of returning ERR_MOVED. > > > > > > > > OK, makes sense to me, I think. > > > > > > Hi Bruce- > > > > > > Does this mean you will take this patch, or should > > > I just add your Reviewed-by: ? > > > > I can take it if nobody objects. Mind if I append the above to the > > changelog? (Just want to document why we think the apparently > > backwards-incompatible change is OK.) > > > I've already added it to my linux-next branch as a stable patch. OK, fine by me, dropping.--b. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-24 20:23 ` Trond Myklebust 2017-01-24 20:31 ` bfields @ 2017-01-25 19:58 ` Chuck Lever 2017-01-25 20:08 ` Chuck Lever 1 sibling, 1 reply; 14+ messages in thread From: Chuck Lever @ 2017-01-25 19:58 UTC (permalink / raw) To: Trond Myklebust; +Cc: J. Bruce Fields, Linux NFS Mailing List > On Jan 24, 2017, at 3:23 PM, Trond Myklebust <trondmy@primarydata.com> wrote: > > On Tue, 2017-01-24 at 14:15 -0500, J. Bruce Fields wrote: >> On Tue, Jan 24, 2017 at 02:06:16PM -0500, Chuck Lever wrote: >>> >>>> On Jan 23, 2017, at 11:49 AM, bfields@fieldses.org wrote: >>>> >>>> On Mon, Jan 23, 2017 at 10:01:27AM -0500, Chuck Lever wrote: >>>>> >>>>>> On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle. >>>>>> com> wrote: >>>>>> >>>>>> Xuan Qi reports that the Linux NFSv4 client failed to lock a >>>>>> file >>>>>> that was migrated. The steps he observed on the wire: >>>>>> >>>>>> 1. The client sent a LOCK request >>>>>> 2. The server replied NFS4ERR_MOVED >>>>>> 3. The client switched to the destination server >>>>>> 4. The client sent the LOCK request again with a bumped >>>>>> ย lock sequence ID >>>>>> 5. The server rejected the LOCK request with >>>>>> NFS4ERR_BAD_SEQID >>>>> >>>>> The list of steps could be more clear: >>>>> >>>>> 1. The client sent a LOCK request to the source server >>>>> 2. The source server replied NFS4ERR_MOVED >>>>> 3. The client switched to the destination server >>>>> 4. The client sent the same LOCK request to the destination >>>>> ย server with a bumped lock sequence ID >>>>> 5. The destination server rejected the LOCK request with >>>>> ย NFS4ERR_BAD_SEQID >>>>> >>>>> >>>>>> RFC 3530 section 8.1.5 provides a list of NFS errors which do >>>>>> not >>>>>> bump a lock sequence ID. >>>>>> >>>>>> However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 >>>>>> section >>>>>> 9.1.7, this list has been updated by the addition of >>>>>> NFS4ERR_MOVED. >>>> >>>> I guess we figured the backwards-incompatible change was OK since >>>> essentially the Solaris server is the first we know of to be >>>> making real >>>> use of NFS4ERR_MOVED? >>>> >>>> And probably it's required for the their implementation because >>>> the old >>>> server no longer has the ability to update the state once it's >>>> reached >>>> the point of returning ERR_MOVED. >>>> >>>> OK, makes sense to me, I think. >>> >>> Hi Bruce- >>> >>> Does this mean you will take this patch, or should >>> I just add your Reviewed-by: ? >> >> I can take it if nobody objects.ย ย Mind if I append the above to the >> changelog?ย ย (Just want to document why we think the apparently >> backwards-incompatible change is OK.) >> > I've already added it to my linux-next branch as a stable patch. This patch alone might not be enough. Our test results show that even with this patch applied, the Linux client still increments the lock sequence ID after NFS4ERR_MOVED. -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED 2017-01-25 19:58 ` Chuck Lever @ 2017-01-25 20:08 ` Chuck Lever 0 siblings, 0 replies; 14+ messages in thread From: Chuck Lever @ 2017-01-25 20:08 UTC (permalink / raw) To: Trond Myklebust; +Cc: J. Bruce Fields, Linux NFS Mailing List > On Jan 25, 2017, at 2:58 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > >> >> On Jan 24, 2017, at 3:23 PM, Trond Myklebust <trondmy@primarydata.com> wrote: >> >> On Tue, 2017-01-24 at 14:15 -0500, J. Bruce Fields wrote: >>> On Tue, Jan 24, 2017 at 02:06:16PM -0500, Chuck Lever wrote: >>>> >>>>> On Jan 23, 2017, at 11:49 AM, bfields@fieldses.org wrote: >>>>> >>>>> On Mon, Jan 23, 2017 at 10:01:27AM -0500, Chuck Lever wrote: >>>>>> >>>>>>> On Jan 22, 2017, at 2:04 PM, Chuck Lever <chuck.lever@oracle. >>>>>>> com> wrote: >>>>>>> >>>>>>> Xuan Qi reports that the Linux NFSv4 client failed to lock a >>>>>>> file >>>>>>> that was migrated. The steps he observed on the wire: >>>>>>> >>>>>>> 1. The client sent a LOCK request >>>>>>> 2. The server replied NFS4ERR_MOVED >>>>>>> 3. The client switched to the destination server >>>>>>> 4. The client sent the LOCK request again with a bumped >>>>>>> ย lock sequence ID >>>>>>> 5. The server rejected the LOCK request with >>>>>>> NFS4ERR_BAD_SEQID >>>>>> >>>>>> The list of steps could be more clear: >>>>>> >>>>>> 1. The client sent a LOCK request to the source server >>>>>> 2. The source server replied NFS4ERR_MOVED >>>>>> 3. The client switched to the destination server >>>>>> 4. The client sent the same LOCK request to the destination >>>>>> ย server with a bumped lock sequence ID >>>>>> 5. The destination server rejected the LOCK request with >>>>>> ย NFS4ERR_BAD_SEQID >>>>>> >>>>>> >>>>>>> RFC 3530 section 8.1.5 provides a list of NFS errors which do >>>>>>> not >>>>>>> bump a lock sequence ID. >>>>>>> >>>>>>> However, RFC 3530 is now obsoleted by RFC 7530. In RFC 7530 >>>>>>> section >>>>>>> 9.1.7, this list has been updated by the addition of >>>>>>> NFS4ERR_MOVED. >>>>> >>>>> I guess we figured the backwards-incompatible change was OK since >>>>> essentially the Solaris server is the first we know of to be >>>>> making real >>>>> use of NFS4ERR_MOVED? >>>>> >>>>> And probably it's required for the their implementation because >>>>> the old >>>>> server no longer has the ability to update the state once it's >>>>> reached >>>>> the point of returning ERR_MOVED. >>>>> >>>>> OK, makes sense to me, I think. >>>> >>>> Hi Bruce- >>>> >>>> Does this mean you will take this patch, or should >>>> I just add your Reviewed-by: ? >>> >>> I can take it if nobody objects.ย ย Mind if I append the above to the >>> changelog?ย ย (Just want to document why we think the apparently >>> backwards-incompatible change is OK.) >>> >> I've already added it to my linux-next branch as a stable patch. > > This patch alone might not be enough. > > Our test results show that even with this patch applied, the Linux > client still increments the lock sequence ID after NFS4ERR_MOVED. Looks like nfs_increment_seqid() also needs to be updated? -- Chuck Lever ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-01-25 20:08 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-22 19:04 [PATCH v1] nfs: Don't increment lock sequence ID after NFS4ERR_MOVED Chuck Lever 2017-01-23 15:01 ` Chuck Lever 2017-01-23 16:49 ` J. Bruce Fields 2017-01-24 19:06 ` Chuck Lever 2017-01-24 19:15 ` J. Bruce Fields 2017-01-24 19:31 ` Chuck Lever 2017-01-24 19:41 ` J. Bruce Fields 2017-01-24 19:53 ` J. Bruce Fields 2017-01-24 19:58 ` Chuck Lever 2017-01-24 19:54 ` Chuck Lever 2017-01-24 20:23 ` Trond Myklebust 2017-01-24 20:31 ` bfields 2017-01-25 19:58 ` Chuck Lever 2017-01-25 20:08 ` Chuck Lever
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.