mds: Return ceph.dir.subvolume vxattr by edwinzrodriguez · Pull Request #65104 · ceph/ceph

edwinzrodriguez · 2025-08-18T20:23:31Z

This introduces handling for the "ceph.dir.subvolume" virtual xattr on directories.

Returns ASCII "1" when the directory is a subvolume root, "0" otherwise.
Uses srnode->is_subvolume() for the determination.
Returns -ENOTDIR for non-directory targets.
Uses a lightweight rdlock on the directory’s snaplock to guard the read-only check.
If the rdlock cannot be acquired immediately, returns -EBUSY to avoid blocking; callers can retry.
Caches success of rd-only checks within the request via mdr->more()->rdonly_checks to prevent re-acquiring locks in the same op.
Drops the rdlock once the check is complete.

QA:

Extend CephFS subvolume tests to validate vxattr retrieval and behavior around setting/removal.

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)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins test classic perf Jenkins Job | Jenkins Job Definition
jenkins test crimson perf Jenkins Job | Jenkins Job Definition
jenkins test signed Jenkins Job | Jenkins Job Definition
jenkins test make check Jenkins Job | Jenkins Job Definition
jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
jenkins test submodules Jenkins Job | Jenkins Job Definition
jenkins test dashboard Jenkins Job | Jenkins Job Definition
jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
jenkins test api Jenkins Job | Jenkins Job Definition
jenkins test docs ReadTheDocs | Github Workflow Definition
jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
jenkins test windows Jenkins Job | Jenkins Job Definition
jenkins test rook e2e Jenkins Job | Jenkins Job Definition

edwinzrodriguez · 2025-08-19T12:47:08Z

jenkins test make check

chrisphoffman · 2025-08-19T19:45:47Z

src/mds/Server.cc

+    //   can retry later.
+    // - On success, write ASCII "1" if the directory is a subvolume root, or "0"
+    //   otherwise, using the srnode->is_subvolume() call.
+    if (!cur->is_dir()) {


IIUC, this is not needed since this xattr is added to is_ceph_dir_vxattr, the check at the top of this function should handle non-dir case.

This section:

ceph/src/mds/Server.cc

Lines 7120 to 7124 in 558e59c

if (is_ceph_dir_vxattr(xattr_name)) {

if (!cur->is_dir()) {

respond_to_request(mdr, -ENODATA);

return;

}

dparmar18 · 2025-08-20T11:08:28Z

src/mds/Server.cc

+      r = -ENOTDIR;
+    } else {
+      // Acquire a lighter weight rdlock
+      if (!mdr->more()->rdonly_checks) {


you can skip checking and acquiring snaplock, the inode should have the snaplock from CInode *cur = rdlock_path_pin_ref(mdr, true, false);

mchangir · 2025-08-20T11:10:45Z

@vshankar I have a commit for adding ceph.dir.subvolume support here as well 1419a4c

dparmar18 · 2025-08-20T11:11:46Z

src/mds/Server.cc

+
+      if (mdr->more()->rdonly_checks) {
+        const auto srnode = cur->get_projected_srnode();
+        *css << (srnode->is_subvolume() ? "1"sv : "0"sv);


i think just srnode->is_subvolume() should suffice

Sounds good.

edwinzrodriguez · 2025-09-03T13:07:28Z

jenkins test make check arm64

edwinzrodriguez · 2025-09-03T13:07:34Z

jenkins test make check

edwinzrodriguez · 2025-09-04T12:37:01Z

jenkins test api

github-actions · 2025-09-12T05:05:45Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

chrisphoffman · 2025-09-15T18:31:39Z

src/mds/Server.cc

      // since we only handle ceph vxattrs here
      r = -ENODATA; // no such attribute
    }
+  } else if (xattr_name.substr(0, 18) == "ceph.dir.subvolume"sv) {


Why substr vs explicitly matching?

Copied from the other code for example at line 7275

I think xattr_name == "ceph.dir.subvolume"sv should suffice.

Sure. It seems this function mixes the 2 styles and I couldn't come up with any rationalization why.

It looks like one uses substr to drop into a set of common xattr names and then matches them one by one. It seems like an organizational thing.

qa/tasks/cephfs/test_subvolume.py

edwinzrodriguez · 2025-09-16T02:43:32Z

jenkins test api

mchangir · 2025-09-16T07:04:25Z

@vshankar I have a commit for adding ceph.dir.subvolume support here as well 1419a4c

@vshankar do I drop my commit in favor of Edwin's PR ?

vshankar · 2025-09-16T07:27:24Z

@vshankar do I drop my commit in favor of Edwin's PR ?

Yes please.

edwinzrodriguez · 2025-09-16T12:24:23Z

jenkins test windows

dparmar18 · 2025-09-16T12:41:09Z

LGTM, just one thing - i'm not sure if it's also needed to add a test case in src/test/libcephfs/vxattr.cc? ping @vshankar?

edwinzrodriguez · 2025-09-16T12:51:06Z

A test beyond what I added in qa/tasks/cephfs/test_subvolume.py ?

edwinzrodriguez · 2025-09-16T12:52:22Z

jenkins test api

edwinzrodriguez · 2025-09-16T12:52:44Z

jenkins test make check arm64

dparmar18 · 2025-09-16T13:03:23Z

A test beyond what I added in qa/tasks/cephfs/test_subvolume.py ?

i think it's sufficient, yes but i'm not sure if we're maintaining src/test/libcephfs/vxattr.cc with every vxattr change or is just okay to not have it.

chrisphoffman

All of my comments were addressed.

LGTM

edwinzrodriguez · 2025-09-16T13:21:46Z

jenkins test api

edwinzrodriguez · 2025-09-16T15:10:04Z

jenkins test make check arm64

edwinzrodriguez · 2025-09-16T15:10:16Z

jenkins test api

edwinzrodriguez · 2025-09-16T15:10:36Z

jenkins test windows

edwinzrodriguez · 2025-09-16T19:09:31Z

jenkins test make check arm64

rishabh-d-dave · 2025-09-17T05:02:22Z

@edwinzrodriguez If some many tests are failing you can comment jenkins restest this please to run all CI jobs again.

rishabh-d-dave · 2025-09-17T05:17:54Z

qa/tasks/cephfs/test_subvolume.py

        # cleanup
        self.mount_a.run_shell(['rm', '-rf', 'group/subvol3'])

+


I would recommend to remove this newline since in Python we reserve 2 sequential newlines for special purpose.

PEP 8 style guide specifies that top-level function and class definitions should be surrounded by two blank lines.

rishabh-d-dave

LGTM

This introduces handling for the "ceph.dir.subvolume" virtual xattr on directories. - Returns ASCII "1" when the directory is a subvolume root, "0" otherwise. QA: - Extend CephFS subvolume tests to validate vxattr retrieval and behavior around setting/removal. Fixes: https://tracker.ceph.com/issues/72556 Signed-off-by: Edwin Rodriguez <edwin.rodriguez1@ibm.com>

rishabh-d-dave · 2025-09-29T18:22:05Z

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

rishabh-d-dave

QA run was successful - https://tracker.ceph.com/projects/cephfs/wiki/QA_main_2025#wip-rishabh-testing-20250929182116

github-actions · 2025-10-03T11:26:04Z

This is an automated message by src/script/redmine-upkeep.py.

I found one or more Fixes: tags in the commit messages in

git log fa93d68191b31c417381fa4cea3f87eb7840be4a^..fa93d68191b31c417381fa4cea3f87eb7840be4a

The referenced tickets are:

https://tracker.ceph.com/issues/72556

Those tickets do not reference this merged Pull Request. If this Pull Request merge resolves any of those tickets, please update the "Pull Request ID" field on each ticket. A future run of this script will appropriately update them.

Update Log: https://github.com/ceph/ceph/actions/runs/18220936287

github-actions bot added cephfs Ceph File System tests labels Aug 18, 2025

chrisphoffman reviewed Aug 19, 2025

View reviewed changes

vshankar requested a review from a team August 20, 2025 11:00

dparmar18 reviewed Aug 20, 2025

View reviewed changes

edwinzrodriguez force-pushed the ceph-wip-72556 branch 2 times, most recently from c0b3f6f to 65e31c9 Compare August 25, 2025 13:00

edwinzrodriguez force-pushed the ceph-wip-72556 branch from 65e31c9 to 6990903 Compare September 3, 2025 18:24

github-actions bot added the needs-rebase label Sep 12, 2025

edwinzrodriguez force-pushed the ceph-wip-72556 branch from 6990903 to 1ac6301 Compare September 12, 2025 13:36

github-actions bot removed the needs-rebase label Sep 12, 2025

chrisphoffman reviewed Sep 15, 2025

View reviewed changes

edwinzrodriguez force-pushed the ceph-wip-72556 branch from 1ac6301 to cce65a8 Compare September 15, 2025 20:07

vshankar requested a review from a team September 16, 2025 07:27

vshankar assigned chrisphoffman Sep 16, 2025

dparmar18 self-assigned this Sep 16, 2025

mchangir mentioned this pull request Sep 16, 2025

mgr/volumes: add validation to arguments #60534

Open

14 tasks

edwinzrodriguez force-pushed the ceph-wip-72556 branch from cce65a8 to f497ad4 Compare September 16, 2025 12:33

chrisphoffman approved these changes Sep 16, 2025

View reviewed changes

edwinzrodriguez force-pushed the ceph-wip-72556 branch from f497ad4 to 03f752c Compare September 16, 2025 16:57

rishabh-d-dave reviewed Sep 17, 2025

View reviewed changes

rishabh-d-dave approved these changes Sep 17, 2025

View reviewed changes

edwinzrodriguez force-pushed the ceph-wip-72556 branch from 03f752c to 51fb4b6 Compare September 17, 2025 11:57

batrick approved these changes Sep 22, 2025

View reviewed changes

batrick added the needs-qa label Sep 22, 2025

rishabh-d-dave added the wip-rishabh-testing Rishabh's testing label label Sep 29, 2025

rishabh-d-dave reviewed Oct 3, 2025

View reviewed changes

rishabh-d-dave merged commit fa93d68 into ceph:main Oct 3, 2025
13 checks passed

edwinzrodriguez deleted the ceph-wip-72556 branch October 3, 2025 13:59

This was referenced Oct 4, 2025

tentacle: mds: Return ceph.dir.subvolume vxattr #65779

Merged

squid: mds: Return ceph.dir.subvolume vxattr #65820

Open

	if (is_ceph_dir_vxattr(xattr_name)) {
	if (!cur->is_dir()) {
	respond_to_request(mdr, -ENODATA);
	return;
	}

		# cleanup
		self.mount_a.run_shell(['rm', '-rf', 'group/subvol3'])

Conversation

edwinzrodriguez commented Aug 18, 2025

Contribution Guidelines

Checklist

Uh oh!

edwinzrodriguez commented Aug 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mchangir commented Aug 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edwinzrodriguez commented Sep 3, 2025

Uh oh!

edwinzrodriguez commented Sep 3, 2025

Uh oh!

edwinzrodriguez commented Sep 4, 2025

Uh oh!

github-actions bot commented Sep 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

mchangir commented Sep 16, 2025

Uh oh!

vshankar commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

dparmar18 commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

dparmar18 commented Sep 16, 2025

Uh oh!

chrisphoffman left a comment

Choose a reason for hiding this comment

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

edwinzrodriguez commented Sep 16, 2025

Uh oh!

rishabh-d-dave commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave left a comment

Choose a reason for hiding this comment

Uh oh!

rishabh-d-dave commented Sep 29, 2025

Uh oh!

rishabh-d-dave left a comment

Choose a reason for hiding this comment