mds: use the whole string as the snapshot long name #45192

lxbsz · 2022-02-28T10:13:35Z

The length of encrypted the snapshot name will be longer than normal
name and we need the whole encrypted name in kclient to do the base64
decode.

The full string of the encrypted long name will be like _${ENCRPTED-NAME}_${INO}.

Signed-off-by: Xiubo Li xiubli@redhat.com

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 retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

ajarr · 2022-02-28T18:55:33Z

looks good!

nmshelke

LGTM

jtlayton

Looks fine otherwise.

src/mds/snap.cc

lxbsz · 2022-03-02T12:39:01Z

jenkins retest this please

luis-henrix · 2022-03-08T15:55:11Z

src/mds/snap.cc

@@ -84,9 +84,9 @@ std::string_view SnapInfo::get_long_name() const
  if (long_name.empty() ||
      long_name.compare(1, name.size(), name) ||
      long_name.find_last_of("_") != name.size() + 1) {
-    char nm[80];


This puzzles me. Does this mean that creating a snapshot with a name length > 80 is currently broken? I guess that bad things can happen if I do an LSSNAP in the .snap directory that has one of these 'long names'.
Any idea why there was such a limitation? And how sure are we that this change won't break something else?

This puzzles me. Does this mean that creating a snapshot with a name length > 80 is currently broken? I guess that bad things can happen if I do an LSSNAP in the .snap directory that has one of these 'long names'. Any idea why there was such a limitation? And how sure are we that this change won't break something else?

I didn't find there has any special limitation for the snapshot name in ceph. The current code is also buggy if the snapshot name is too long, which is larger than 80 - 2 * sizeof('_') + sizeof(<inode#>) == 65 chars, since it will always truncate the long snap name to 80 chars, then if you do LSSNAP the MDS may won't receive the exact long snap name and couldn't parse the correct <inode#>, just as in your kernel mail list mentioned:

The MDS logs:

747 2022-03-09T11:06:01.308+0800 152187fed700 4 mds.0.server handle_client_request client_request(client.4216:16 lookupsnap #0x10000000001//_123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 2022-03-09T11:06:01.308545+0800 caller_uid=0, caller_gid=0{0,}) v4 748 2022-03-09T11:06:01.309+0800 152187fed700 20 mds.0.5 get_session have 0x5593da9c6d00 client.4216 v1:10.72.47.117:0/2358830768 state open 749 2022-03-09T11:06:01.309+0800 152187fed700 15 mds.0.server oldest_client_tid=16 750 2022-03-09T11:06:01.309+0800 152187fed700 7 mds.0.cache request_start request(client.4216:16 nref=2 cr=0x5593daab6840) 751 2022-03-09T11:06:01.309+0800 152187fed700 7 mds.0.server dispatch_client_request client_request(client.4216:16 lookupsnap #0x10000000001//_123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 2022-03-09T11:06:01.308545+0800 caller_uid=0, caller_gid=0{0,}) v4 752 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.server rdlock_path_pin_ref request(client.4216:16 nref=2 cr=0x5593daab6840) #0x10000000001//_123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 753 2022-03-09T11:06:01.309+0800 152187fed700 7 mds.0.cache traverse: opening base ino 0x10000000001 snap head 754 2022-03-09T11:06:01.309+0800 152187fed700 12 mds.0.cache traverse: path seg depth 0 '' snapid head 755 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.cache traverse: snapdir 756 2022-03-09T11:06:01.309+0800 152187fed700 12 mds.0.cache traverse: path seg depth 1 '_123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777' snapid snapdir 757 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.cache.snaprealm(0x10000000000 seq 2 0x5593daaa6000) resolve_snapname '_123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777' in [0,head] 758 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.cache.snaprealm(0x10000000000 seq 2 0x5593daaa6000) _123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 parses to name '123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345' dirino 0x1999999999 759 2022-03-09T11:06:01.309+0800 152187fed700 15 mds.0.cache.snaprealm(0x10000000000 seq 2 0x5593daaa6000) ? snap(2 0x10000000000 '123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345' 2022-03-09T10:52:39.357824+0800) 760 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.cache.snaprealm(0x1 seq 1 0x5593d5c6cc00) resolve_snapname '_123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777' in [2,head] 761 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.cache.snaprealm(0x1 seq 1 0x5593d5c6cc00) _123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 parses to name '123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345' dirino 0x1999999999 762 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.cache traverse: snap _123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 -> 0 763 2022-03-09T11:06:01.309+0800 152187fed700 10 mds.0.server FAIL on error -2 764 2022-03-09T11:06:01.309+0800 152187fed700 7 mds.0.server reply_client_request -2 ((2) No such file or directory) client_request(client.4216:16 lookupsnap #0x10000000001//_123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 2022-03-09T11:06:01.308545+0800 caller_uid=0, caller_gid=0{0,}) v4

Actually the full long snap name:

Should be: 123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_1099511627777 <--- has 81 chars Not: 123456qwertasdfgzxcvb7890yuiophjklnm123456qwertasdfgzxcvb78912345_109951162777 <--- has 80 chars

I haven't come across such a limitation. maybe @gregsfortytwo might know some history...

Nope, saw this from the mailing list post and it was a surprise to me. I thought there were plenty of things generating long names so I'm surprised this hasn't come up before.

vshankar

Otherwise looks good.

dvanders · 2022-04-02T05:11:42Z

@lxbsz thanks, lgtm. Will this be backported to P and Q?

lxbsz · 2022-04-07T14:27:26Z

@lxbsz thanks, lgtm. Will this be backported to P and Q?

Yeah, this will be backported together with the fscrypt feature later.

lxbsz · 2022-04-14T12:19:36Z

jenkins retest this please

The length of encrypted the snapshot name will be longer than normal name and we need the whole encrypted name in kclient to do the base64 decode. Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz · 2022-04-18T04:51:13Z

jenkins retest this please

lxbsz · 2022-04-25T12:55:16Z

jenkins retest this please

vshankar · 2022-05-24T02:18:25Z

jenkins retest this please

vshankar · 2022-10-14T10:52:52Z

jenkins test make check

* refs/pull/45192/head: mds: use the whole string as the snapshot long name Reviewed-by: Jos Collin <jcollin@redhat.com> Reviewed-by: Nikhilkumar Shelke <nshelke@redhat.com> Reviewed-by: Ramana Raja <rraja@redhat.com> Reviewed-by: Jeff Layton <jlayton@redhat.com>

vshankar

https://tracker.ceph.com/projects/cephfs/wiki/Main#08-Dec-2022

vshankar · 2022-12-09T04:36:30Z

jenkins retest this please

* refs/pull/45192/head: mds: use the whole string as the snapshot long name Reviewed-by: Venky Shankar <vshankar@redhat.com> Reviewed-by: Jos Collin <jcollin@redhat.com> Reviewed-by: Nikhilkumar Shelke <nshelke@redhat.com> Reviewed-by: Jeff Layton <jlayton@redhat.com> Reviewed-by: Ramana Raja <rraja@redhat.com>

vshankar · 2022-12-12T13:56:57Z

jenkins render docs

vshankar · 2022-12-12T14:00:20Z

jenkins test docs

lxbsz requested review from vshankar and a team February 28, 2022 10:13

github-actions bot added the cephfs Ceph File System label Feb 28, 2022

lxbsz requested a review from jtlayton February 28, 2022 10:14

lxbsz force-pushed the snapshotlongname branch 2 times, most recently from 12bbe7c to 6a05384 Compare February 28, 2022 11:26

ajarr approved these changes Feb 28, 2022

View reviewed changes

nmshelke approved these changes Mar 1, 2022

View reviewed changes

jtlayton approved these changes Mar 1, 2022

View reviewed changes

src/mds/snap.cc Outdated Show resolved Hide resolved

lxbsz force-pushed the snapshotlongname branch from 6a05384 to 2d26ec2 Compare March 1, 2022 11:09

joscollin approved these changes Mar 2, 2022

View reviewed changes

luis-henrix reviewed Mar 8, 2022

View reviewed changes

lxbsz mentioned this pull request Mar 9, 2022

mds: limit the snapshot names to 240 characters #45312

Merged

vshankar reviewed Mar 11, 2022

View reviewed changes

vshankar added the wip-vshankar-testing1 label Mar 11, 2022

vshankar added wip-vshankar-fscrypt CephFS fscrypt changes and removed wip-vshankar-testing1 labels Mar 24, 2022

mds: use the whole string as the snapshot long name

5511dd7

The length of encrypted the snapshot name will be longer than normal name and we need the whole encrypted name in kclient to do the base64 decode. Signed-off-by: Xiubo Li <xiubli@redhat.com>

lxbsz force-pushed the snapshotlongname branch from 2d26ec2 to 5511dd7 Compare April 14, 2022 12:22

djgalloway changed the base branch from master to main May 25, 2022 20:03

lxbsz mentioned this pull request Oct 27, 2022

mds: prevent clients from exceeding the xattrs key/value limits #46357

Merged

vshankar added wip-vshankar-testing and removed wip-vshankar-fscrypt CephFS fscrypt changes labels Oct 28, 2022

vshankar approved these changes Dec 8, 2022

View reviewed changes

vshankar merged commit aa403a9 into ceph:main Dec 12, 2022

vshankar removed the wip-vshankar-testing label Dec 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mds: use the whole string as the snapshot long name #45192

mds: use the whole string as the snapshot long name #45192

lxbsz commented Feb 28, 2022 •

edited

ajarr commented Feb 28, 2022

nmshelke left a comment

jtlayton left a comment

lxbsz commented Mar 2, 2022

luis-henrix Mar 8, 2022

lxbsz Mar 9, 2022 •

edited

vshankar Mar 11, 2022

gregsfortytwo Apr 7, 2022

vshankar left a comment

dvanders commented Apr 2, 2022

lxbsz commented Apr 7, 2022

lxbsz commented Apr 14, 2022

lxbsz commented Apr 18, 2022

lxbsz commented Apr 25, 2022

vshankar commented May 24, 2022

vshankar commented Oct 14, 2022

vshankar left a comment

vshankar commented Dec 9, 2022

vshankar commented Dec 12, 2022

vshankar commented Dec 12, 2022

mds: use the whole string as the snapshot long name #45192

mds: use the whole string as the snapshot long name #45192

Conversation

lxbsz commented Feb 28, 2022 • edited

Checklist

ajarr commented Feb 28, 2022

nmshelke left a comment

Choose a reason for hiding this comment

jtlayton left a comment

Choose a reason for hiding this comment

lxbsz commented Mar 2, 2022

luis-henrix Mar 8, 2022

Choose a reason for hiding this comment

lxbsz Mar 9, 2022 • edited

Choose a reason for hiding this comment

vshankar Mar 11, 2022

Choose a reason for hiding this comment

gregsfortytwo Apr 7, 2022

Choose a reason for hiding this comment

vshankar left a comment

Choose a reason for hiding this comment

dvanders commented Apr 2, 2022

lxbsz commented Apr 7, 2022

lxbsz commented Apr 14, 2022

lxbsz commented Apr 18, 2022

lxbsz commented Apr 25, 2022

vshankar commented May 24, 2022

vshankar commented Oct 14, 2022

vshankar left a comment

Choose a reason for hiding this comment

vshankar commented Dec 9, 2022

vshankar commented Dec 12, 2022

vshankar commented Dec 12, 2022

lxbsz commented Feb 28, 2022 •

edited

lxbsz Mar 9, 2022 •

edited