Project

General

Profile

Backport #16946

jewel: client: nlink count is not maintained correctly

Added by Nathan Cutler almost 5 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Target version:
Release:
jewel
Crash signature (v1):
Crash signature (v2):


Related issues

Copied from CephFS - Bug #16668: client: nlink count is not maintained correctly Resolved 07/12/2016

History

#1 Updated by Nathan Cutler almost 5 years ago

  • Copied from Bug #16668: client: nlink count is not maintained correctly added

#3 Updated by Loïc Dachary almost 5 years ago

  • Status changed from New to Need More Info
  • Assignee set to Jeff Layton

git cherry-pick -x https://github.com/ceph/ceph/pull/10386/commits/f3605d39e53b3ff777eb64538abfa62a5f98a4f2 which is part of https://github.com/ceph/ceph/pull/10386/commits conflicts with

<<<<<<< variant A
  r = _do_lookup(dir, dname, target, uid, gid);
>>>>>>> variant B
  r = _do_lookup(dir, dname, mask, target, uid, gid);
======= end

do you advise backporting the pull request that changes _do_lookup signature as well ?

#4 Updated by Jeff Layton almost 5 years ago

Yes. I think you'll want the entire patch pile from that PR. These 4 patches at least:

https://github.com/ceph/ceph/commit/14ee7bcbf0bd ceph: don't fudge the ctime in stat() unless it's really older than the mtime
https://github.com/ceph/ceph/commit/9e8476743eef client: only skip querying the MDS in _lookup when we have the necessary caps
https://github.com/ceph/ceph/commit/f3605d39e53b client: plumb a mask argument into _lookup
https://github.com/ceph/ceph/commit/a2ce16f8bfdb client: add mask parameter to _do_lookup

#5 Updated by Jeff Layton almost 5 years ago

  • Status changed from Need More Info to In Progress
  • Assignee changed from Jeff Layton to Loïc Dachary

#6 Updated by Loïc Dachary almost 5 years ago

  • Status changed from In Progress to New
  • Assignee deleted (Loïc Dachary)

This is perfect, thank you !

#7 Updated by Loïc Dachary almost 5 years ago

  • Status changed from New to Need More Info
  • Assignee set to Jeff Layton

Actually, you were right to ask, my question was about something else :-) It's good to know that the four commits are the one that matter, it may help. One of them, however, conflicts when cherry-picked on top of jewel with

git cherry-pick -x f3605d39e53b3ff777eb64538abfa62a5f98a4f2

with the conflict shown above. My question is about the best way to resolve this conflict. It could either be resolved manually or the commit(s) that create the conflict could be backported as well so that there is no more conflict. The problem with the later approach is that we may end up adding to jewel more than what is reasonable. My notion of "reasonable" in the context of fs is not to be relied on, hence my question :-)

#8 Updated by Jeff Layton almost 5 years ago

  • Assignee changed from Jeff Layton to Loïc Dachary

You want the latter approach, and you want to pick them in the order they were originally committed, in case we need to bisect later. Just backporting f3605d39e53b3ff777eb64538abfa62a5f98a4f2 would be problematic, as that patch relies on the earlier changes in that series.

So to be clear, I'd cherry-pick (in this order):

a2ce16f8bfdb client: add mask parameter to _do_lookup
f3605d39e53b client: plumb a mask argument into _lookup
9e8476743eef client: only skip querying the MDS in _lookup when we have the necessary caps

...this commit is not strictly required to fix the problem with nlink, but it does fix a related problem. I'd suggest taking it as well:

14ee7bcbf0bd ceph: don't fudge the ctime in stat() unless it's really older than the mtime

#9 Updated by Jeff Layton almost 5 years ago

  • Status changed from Need More Info to In Progress

#10 Updated by Loïc Dachary almost 5 years ago

@Jeff this is a very unusual situation and I apologize for the noise. It turns out that github does not display the commits in order.

git log --oneline 01cd578^1..01cd578 | cat
01cd578 Merge pull request #10386 from ceph/wip-jlayton-nlink
14ee7bc ceph: don't fudge the ctime in stat() unless it's really older than the mtime
9e84767 client: only skip querying the MDS in _lookup when we have the necessary caps
f3605d3 client: plumb a mask argument into _lookup
a2ce16f client: add mask parameter to _do_lookup
e8a8989 test: add a link count test
d85ba57 doc: add a new document on capabilities
c0bba0f cephfs: remove some unused constants

which is the order you suggested for f3605d3 and a2ce16f. But https://github.com/ceph/ceph/pull/10386/commits shows them inverted. I can't figure out which git --*-order option makes that kind of inversion. And I'm surprised that did not hit me before.

I'm not curious enough to investigate more and the only reason I'm writing that down (except for your entertainment ;-) is to find it later, should it happen again.

#11 Updated by Loïc Dachary almost 5 years ago

  • Description updated (diff)

#12 Updated by Greg Farnum almost 5 years ago

FYI: Github is annoying and does some kind of timestamp sort when displaying commits. I'm not sure if it's the original author date or commit date or something else, but it can make a mess of rebased patches. :(

#13 Updated by Loïc Dachary over 4 years ago

  • Status changed from In Progress to Resolved
  • Target version set to v10.2.4

Also available in: Atom PDF