Skip to content

mds: generate symlink inode with correct mode#63937

Merged
tchaikov merged 1 commit intoceph:mainfrom
tchaikov:wip-mds-inode-valid-test-instances
Jun 18, 2025
Merged

mds: generate symlink inode with correct mode#63937
tchaikov merged 1 commit intoceph:mainfrom
tchaikov:wip-mds-inode-valid-test-instances

Conversation

@tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Jun 15, 2025 •
edited by batrick
Loading

Fix test instance generation for InodeStoreBare and InodeStore to properly set the mode field to S_IFLNK for symlink inodes.

Previously, generated test instances with symlink inodes had unset mode fields, creating inconsistent data. This issue was masked because ceph-dencoder reused existing instances during encode/decode consistency tests, leaving stale values intact.

The problem would surface when check-generated.sh and readable.sh allocate fresh instances for decoding tests, as the missing mode field would cause decode/encode inconsistencies.

This change fixes generate_test_instances() to set the mode field to S_IFLNK for symlink inodes, creating valid InodeStore and InodeStoreBare instances with consistent field values for proper encode/decode testing.

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

@tchaikov
mds: generate symlink inode with correct mode
7b631e7 
Fix test instance generation for InodeStoreBare and InodeStore to
properly set the mode field to S_IFLNK for symlink inodes.

Previously, generated test instances with symlink inodes had unset
mode fields, creating inconsistent data. This issue was masked because
ceph-dencoder reused existing instances during encode/decode consistency
tests, leaving stale values intact.

The problem would surface when check-generated.sh and readable.sh
allocate fresh instances for decoding tests, as the missing mode field
would cause decode/encode inconsistencies.

This change fixes generate_test_instances() to set the mode field to
S_IFLNK for symlink inodes, creating valid InodeStore and InodeStoreBare
instances with consistent field values for proper encode/decode testing.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@github-actions github-actions bot added the cephfs Ceph File System label Jun 15, 2025
@tchaikov tchaikov mentioned this pull request Jun 15, 2025
Merged
14 tasks
@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 17, 2025 •
edited
Loading

@batrick hi Patrick, thank you for your review and approval. but i am wondering if running this change with qa is necessary. because, as explained in the commit message, InodeStore::generate_test_instances() and InodeStoreBare::generate_test_instances() are only used in two cases:

  • used by check-generated.sh to generate test instances, so we can verify that the original value of a certain type is identical to the one after encoding and decoding it, by comparing their json dumps.
  • used by other types in their own generate_test_instances()

hence we don't use generate_test_instances() in our teuthology-based tests.

what do you think?

@batrick
Copy link
Member

batrick commented Jun 17, 2025

@batrick hi Patrick, thank you for your review and approval. but i am wondering if running this change with qa is necessary. because, as explained in the commit message, InodeStore::generate_test_instances() and InodeStoreBare::generate_test_instances() are only used in three cases:

* used by check-generated.sh to generate test instances, so we can verify that the original value of a certain type is identical to the one after encoding and decoding it, by comparing their json dumps.

* used by other types in their own `generate_test_instances()`

hence we don't use generate_test_instances() in our teuthology-based tests.

what do you think?

I had intended to just see it builds for all platforms we build on but if you'd like to merge it now I don't mind.

@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 18, 2025 •
edited
Loading

@batrick sorry that i misread your intention of adding the needs-qa label, i thought needs-qa was added to include this change in a teuthology batch. since this change is not urgent, we can wait until it's proved to build and the result is published on shaman.

just pushed it to ceph-ci: https://shaman.ceph.com/builds/ceph/wip-kefu-pr-63937/

@tchaikov
Copy link
Contributor Author

tchaikov commented Jun 18, 2025

tested at https://shaman.ceph.com/builds/ceph/wip-kefu-pr-63937/

@tchaikov tchaikov merged commit 9884643 into ceph:main Jun 18, 2025
25 of 26 checks passed
@tchaikov tchaikov deleted the wip-mds-inode-valid-test-instances branch June 18, 2025 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@batrick batrick batrick approved these changes

Assignees

No one assigned

Labels

cephfs Ceph File System needs-qa

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tchaikov @batrick