Skip to content

mgr/vol: keep and show clone source info#63214

Merged
vshankar merged 5 commits intoceph:mainfrom
rishabh-d-dave:vols-clone-src-info-in-meta
Jul 21, 2025
Merged

mgr/vol: keep and show clone source info#63214
vshankar merged 5 commits intoceph:mainfrom
rishabh-d-dave:vols-clone-src-info-in-meta

Conversation

@rishabh-d-dave
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave commented May 9, 2025

Fixes: https://tracker.ceph.com/issues/71266

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

@rishabh-d-dave rishabh-d-dave requested a review from a team May 9, 2025 16:53
@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from 69989a5 to c27692b Compare May 12, 2025 05:28
@vshankar
Copy link
Contributor

vshankar commented May 12, 2025

@neesingh-rh @kotreshhr PTAL

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 12, 2025

jenkins test make check arm64

vshankar
vshankar requested changes May 13, 2025
View reviewed changes
@kotreshhr
Copy link
Contributor

kotreshhr commented May 14, 2025

This is a good information to have.

But if the source subvolume is deleted and recreated again with the same name ? Won't it be confusing for the user if the source information points to just the subvolume name? Can we differentiate this somehow ? One way - If the source subvolume is deleted, can we update the same in subvolume info output ?

@vshankar
Copy link
Contributor

vshankar commented May 14, 2025

This is a good information to have.

But if the source subvolume is deleted and recreated again with the same name ? Won't it be confusing for the user if the source information points to just the subvolume name? Can we differentiate this somehow ? One way - If the source subvolume is deleted, can we update the same in subvolume info output ?

Would subvolume name+path be enough to distinguish for subvolume incarnations?

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 14, 2025 •
edited
Loading

This is a good information to have.
But if the source subvolume is deleted and recreated again with the same name ? Won't it be confusing for the user if the source information points to just the subvolume name? Can we differentiate this somehow ? One way - If the source subvolume is deleted, can we update the same in subvolume info output ?

Would subvolume name+path be enough to distinguish for subvolume incarnations?

We can write UUID of the subvolume incarnation in .meta file and print it in the output of subvolume info command.

We can also save the state of source subvolume in .meta file of cloned subvolume. This will tell users if/when the source subvolume gets deleted. But this probably is undesriable since change of subvolume state will trigger an update for .meta file for every clone.

Therefore, writing only UUID in .meta file and printing it in subvolume info is a better option. Users can figure out whether or not the source subvolume was deleted by comparing the UUIDs.

@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from c27692b to 4d558b5 Compare May 14, 2025 10:38
@vshankar
Copy link
Contributor

vshankar commented May 14, 2025

This is a good information to have.
But if the source subvolume is deleted and recreated again with the same name ? Won't it be confusing for the user if the source information points to just the subvolume name? Can we differentiate this somehow ? One way - If the source subvolume is deleted, can we update the same in subvolume info output ?

Would subvolume name+path be enough to distinguish for subvolume incarnations?

We can write UUID of the subvolume incarnation in .meta file and print it in the output of subvolume info command.

We can also save the state of source subvolume in .meta file of cloned subvolume. This will tell users if/when the source subvolume gets deleted. But this probably is undesriable since change of subvolume state will trigger an update for .meta file for every clone.

Therefore, writing only UUID in .meta file and printing it in subvolume info is a better option. Users can figure out whether or not the source subvolume was deleted by comparing the UUIDs.

That's an extra step the user has to perform. Dumping source state seems like a good option.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 14, 2025

This is a good information to have.
But if the source subvolume is deleted and recreated again with the same name ? Won't it be confusing for the user if the source information points to just the subvolume name? Can we differentiate this somehow ? One way - If the source subvolume is deleted, can we update the same in subvolume info output ?

Would subvolume name+path be enough to distinguish for subvolume incarnations?

We can write UUID of the subvolume incarnation in .meta file and print it in the output of subvolume info command.
We can also save the state of source subvolume in .meta file of cloned subvolume. This will tell users if/when the source subvolume gets deleted. But this probably is undesriable since change of subvolume state will trigger an update for .meta file for every clone.
Therefore, writing only UUID in .meta file and printing it in subvolume info is a better option. Users can figure out whether or not the source subvolume was deleted by comparing the UUIDs.

That's an extra step the user has to perform. Dumping source state seems like a good option.

Let's say there are 100 different clones of a subvolume, a change in state of this subvolume will trigger 100 writes immediately to .meta files of each of 100 clones. This is why I though just storing UUID is a better idea. WDYT?

@vshankar
Copy link
Contributor

vshankar commented May 14, 2025

This is a good information to have.
But if the source subvolume is deleted and recreated again with the same name ? Won't it be confusing for the user if the source information points to just the subvolume name? Can we differentiate this somehow ? One way - If the source subvolume is deleted, can we update the same in subvolume info output ?

Would subvolume name+path be enough to distinguish for subvolume incarnations?

We can write UUID of the subvolume incarnation in .meta file and print it in the output of subvolume info command.
We can also save the state of source subvolume in .meta file of cloned subvolume. This will tell users if/when the source subvolume gets deleted. But this probably is undesriable since change of subvolume state will trigger an update for .meta file for every clone.
Therefore, writing only UUID in .meta file and printing it in subvolume info is a better option. Users can figure out whether or not the source subvolume was deleted by comparing the UUIDs.

That's an extra step the user has to perform. Dumping source state seems like a good option.

Let's say there are 100 different clones of a subvolume, a change in state of this subvolume will trigger 100 writes immediately to .meta files of each of 100 clones. This is why I though just storing UUID is a better idea. WDYT?

Why not do it the other way -- while fetching source information, fetch the source subvolume state. I think this only needs to be done when dumping clone subvolume info, yes?

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 14, 2025

Why not do it the other way -- while fetching source information, fetch the source subvolume state. I think this only needs to be done when dumping clone subvolume info, yes?

Right. So, we never store the source subvolume's state in .meta file?

@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from 4d558b5 to 3be68a0 Compare May 14, 2025 11:46
@vshankar
Copy link
Contributor

vshankar commented May 14, 2025

Why not do it the other way -- while fetching source information, fetch the source subvolume state. I think this only needs to be done when dumping clone subvolume info, yes?

Right. So, we never store the source subvolume's state in .meta file?

The source subvolume state in the clone subvolume metadata? No. I think we can fetch that on-the-fly.

@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from 3be68a0 to 5cc1e13 Compare May 14, 2025 14:10
@rishabh-d-dave rishabh-d-dave requested a review from vshankar May 14, 2025 14:10
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 14, 2025

@vshankar PTAL

@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from 5cc1e13 to 9accbbb Compare May 14, 2025 16:23
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 15, 2025

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 15, 2025

jenkins test make check arm64

1 similar comment
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented May 15, 2025

jenkins test make check arm64

@rishabh-d-dave
mgr/vol: keep clone source info even after cloning is finished
bbacfda 
Instead of removing the information regarding source of a cloned
subvolume from the .meta file after the cloning has finished, keep it as
it is as the user may find it useful.

Fixes: https://tracker.ceph.com/issues/71266
Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch 3 times, most recently from 06f6cc7 to af1dfb6 Compare June 12, 2025 14:38
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Jun 12, 2025

@kotreshhr @vshankar I've made the last set of requested changes. If it looks fine, can I get an approval? I'll put it through QA.

@rishabh-d-dave rishabh-d-dave added the wip-rishabh-testing Rishabh's testing label label Jun 13, 2025
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Jun 13, 2025

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

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Jun 16, 2025

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

A large number of jobs failed to an unrelated error, deferring QA run until it is fixed.

@rishabh-d-dave rishabh-d-dave removed the wip-rishabh-testing Rishabh's testing label label Jun 16, 2025
neesingh-rh
neesingh-rh reviewed Jun 17, 2025
View reviewed changes
@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from af1dfb6 to e92decf Compare June 17, 2025 11:40
kotreshhr
kotreshhr reviewed Jun 19, 2025
View reviewed changes
Copy link
Contributor

@kotreshhr kotreshhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rishabh-d-dave Could you add a testcase for the source not being part of subvolume info for subvolume (not clone) ? Adding a validating line in existing subvolume info test is fine.

lgtm otherwise

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Jun 20, 2025

@rishabh-d-dave Could you add a testcase for the source not being part of subvolume info for subvolume (not clone) ? Adding a validating line in existing subvolume info test is fine.

lgtm otherwise

It's already done - https://github.com/ceph/ceph/pull/63214/files#diff-d3a7e3f3f24fff510b4d2a562b2093257b20b3908748c3432d14e460c449186bR2869

@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from e92decf to 7bd4ae9 Compare June 20, 2025 11:42
@rishabh-d-dave
mgr/vol: show clone source info in "subvolume info" cmd output
0ef6da6 
Include clone source information in output of "ceph fs subvolume info"
command so that users can access this information conveniently.

Fixes: https://tracker.ceph.com/issues/71266
Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
qa/cephfs: add test to check clone source info's present in...
44cacaf 
.meta file of a cloned subvolume after cloning is finished and in the
output of "ceph fs subvolume info" command.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
doc/cephfs: update docs since "subvolume info" cmd output can also...
0a02914 
contain source field in it.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
release note: add a note that "subvolume info" cmd output can also...
37244a7 
contain "source field" in it.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave rishabh-d-dave force-pushed the vols-clone-src-info-in-meta branch from 7bd4ae9 to 37244a7 Compare June 20, 2025 15:27
neesingh-rh
neesingh-rh reviewed Jul 1, 2025
View reviewed changes
Copy link
Contributor

@neesingh-rh neesingh-rh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vshankar
Copy link
Contributor

vshankar commented Jul 10, 2025

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

vshankar
vshankar approved these changes Jul 21, 2025
View reviewed changes
Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://tracker.ceph.com/projects/cephfs/wiki/QA_main_2025#wip-vshankar-testing-20250714111344-debug

@vshankar vshankar merged commit 37bd496 into ceph:main Jul 21, 2025
13 checks passed
This was referenced Jul 23, 2025
Merged
Merged
@rishabh-d-dave rishabh-d-dave deleted the vols-clone-src-info-in-meta branch July 31, 2025 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anthonyeleven anthonyeleven anthonyeleven left review comments

@vshankar vshankar vshankar approved these changes

@kotreshhr kotreshhr kotreshhr approved these changes

@neesingh-rh neesingh-rh neesingh-rh approved these changes

Assignees

@kotreshhr kotreshhr

@neesingh-rh neesingh-rh

Labels

cephfs Ceph File System documentation needs-review pybind tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rishabh-d-dave @vshankar @kotreshhr @anthonyeleven @neesingh-rh