Skip to content

Comments

client: correctly unwrap getcwd dentries#62228

Merged
batrick merged 6 commits intoceph:mainfrom
batrick:client-path_walk-getcwd
Mar 14, 2025
Merged

client: correctly unwrap getcwd dentries#62228
batrick merged 6 commits intoceph:mainfrom
batrick:client-path_walk-getcwd

Conversation

@batrick
Copy link
Member

@batrick batrick commented Mar 11, 2025 •
edited
Loading

Mostly this is a fix for unwrapping dentry names for getcwd. I've also included a second fix for mkdir and recursive path walks needed for userspace encryption (see #61137).

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

@batrick batrick requested a review from chrisphoffman March 11, 2025 16:26
@github-actions github-actions bot added cephfs Ceph File System tests labels Mar 11, 2025
@batrick batrick mentioned this pull request Mar 11, 2025
Merged
@batrick batrick force-pushed the client-path_walk-getcwd branch 2 times, most recently from 542a2aa to 7009eef Compare March 11, 2025 19:08
@batrick
Copy link
Member Author

batrick commented Mar 11, 2025

jenkins test make check arm64

@batrick
Copy link
Member Author

batrick commented Mar 11, 2025

This PR is under test in https://tracker.ceph.com/issues/70407.

@batrick
Copy link
Member Author

batrick commented Mar 11, 2025

jenkins test make check

@batrick
Copy link
Member Author

batrick commented Mar 11, 2025

jenkins test windows

@batrick batrick force-pushed the client-path_walk-getcwd branch from 7009eef to 98beac3 Compare March 12, 2025 14:34
@batrick
Copy link
Member Author

batrick commented Mar 12, 2025

Previous code caused:

2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:=================================== FAILURES ===================================
2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:_______________________________ test_delete_cwd ________________________________
2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:testdir = None
2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:    def test_delete_cwd(testdir):
2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:        assert_equal(b"/", cephfs.getcwd())
2025-03-12T10:49:57.566 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:        cephfs.mkdir(b"/temp-directory", 0o755)
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:        cephfs.chdir(b"/temp-directory")
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:        cephfs.rmdir(b"/temp-directory")
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:        # getcwd gives you something stale here: it remembers the path string
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:        # even when things are unlinked.  It's up to the caller to find out
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:        # whether it really still exists
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:>       assert_equal(b"/temp-directory", cephfs.getcwd())
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:../../../clone.client.0/src/test/pybind/test_cephfs.py:314:
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:a = b'/temp-directory', b = b'/'
2025-03-12T10:49:57.567 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:    def assert_equal(a, b):
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:>       assert a == b
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:E       AssertionError
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:../../../clone.client.0/src/test/pybind/assertions.py:2: AssertionError
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:---------------------------- Captured stdout setup -----------------------------
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:b'Purge /'
2025-03-12T10:49:57.568 INFO:tasks.workunit.client.0.smithi055.stdout:ret_buf=b''

/teuthology/pdonnell-2025-03-12_03:15:00-fs-wip-pdonnell-testing-20250312.000233-debug-distro-default-smithi/8182736/teuthology.log

@batrick batrick force-pushed the client-path_walk-getcwd branch 3 times, most recently from afdc259 to 8b7e190 Compare March 12, 2025 17:45
@batrick
Copy link
Member Author

batrick commented Mar 12, 2025

This PR is under test in https://tracker.ceph.com/issues/70426.

@batrick batrick requested a review from a team March 12, 2025 19:54
@batrick
Copy link
Member Author

batrick commented Mar 13, 2025

jenkins test windows

vshankar
vshankar reviewed Mar 13, 2025
View reviewed changes
vshankar
vshankar reviewed Mar 13, 2025
View reviewed changes
vshankar
vshankar reviewed Mar 13, 2025
View reviewed changes
batrick added 6 commits March 13, 2025 10:07
@batrick
include/filepath: add iterators for components
a33faa7 
Signed-off-by: Patrick Donnelly <[email protected]>
@batrick
test/libcephfs: refactor ManyNestedDirs
d62a379 
Signed-off-by: Patrick Donnelly <[email protected]>
@batrick
test/libcephfs: test getcwd with case insensitive dir
28bdb8e 
Unfortunately, it's not easy to refactor this test into a shared method without
setting up an explicit test class which has been avoided up to this point. So
I'm going to just copy the code. Sorry.

Signed-off-by: Patrick Donnelly <[email protected]>
@batrick
client: refactor / optimize chdir
3bfd614 
In particular: there's no reason to do a getcwd after chdir.

Signed-off-by: Patrick Donnelly <[email protected]>
@batrick
client: unwrap dentries for getcwd
cda2a6e 
This was missed in the path_walk refactor. readdir is not the only way to "get"
dentry names.

Signed-off-by: Patrick Donnelly <[email protected]>
@batrick
libcephfs: getcwd after chdir for API constraint
c7c5089 
libcephfs semantics require that the old path to the cwd be returned when
getcwd is encounters an unlinked directory in the current working directory.

Signed-off-by: Patrick Donnelly <[email protected]>
@batrick batrick force-pushed the client-path_walk-getcwd branch from 8b7e190 to c7c5089 Compare March 13, 2025 14:07
@batrick batrick requested a review from vshankar March 13, 2025 21:55
@batrick batrick merged commit 4cbf549 into ceph:main Mar 14, 2025
11 checks passed
@batrick batrick deleted the client-path_walk-getcwd branch March 14, 2025 12:29
chrisphoffman
chrisphoffman reviewed Mar 14, 2025
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chrisphoffman chrisphoffman chrisphoffman left review comments

@vshankar vshankar vshankar approved these changes

Assignees

No one assigned

Labels

cephfs Ceph File System tests wip-pdonnell-testing2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@batrick @vshankar @chrisphoffman