Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mds: add truncate size handling support for fscrypt #43588

Merged
merged 18 commits into from Feb 15, 2022

Conversation

lxbsz
Copy link
Member

@lxbsz lxbsz commented Oct 19, 2021

This PR is to support the size handling(truncate size) feature for file cryption, and this is based on Jeff's #41284 PR.

In the kclient side, only when truncating smaller size and the new size is not aligned to CEPH_FSCRYPT_BLOCK_SIZE will the truncate request pass the encrypted last block contents to MDS. And the MDS will help update the last block when truncating the file size.

More detail please see the commit comments.

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

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

@lxbsz
Copy link
Member Author

lxbsz commented Oct 20, 2021

jenkins test make check

@lxbsz lxbsz marked this pull request as draft October 20, 2021 05:51
src/mds/MDCache.cc Outdated Show resolved Hide resolved
src/mds/mdstypes.h Show resolved Hide resolved
src/mds/journal.cc Outdated Show resolved Hide resolved
@lxbsz
Copy link
Member Author

lxbsz commented Nov 1, 2021

@kotreshhr Could help review the mds: add truncate size handling support for fscrypt patch in this PR, maybe we need to fix the journal log related code as Jeff mentioned above.

Thanks.

src/mds/MDCache.cc Outdated Show resolved Hide resolved
@lxbsz lxbsz force-pushed the fsize_support branch 2 times, most recently from 424debf to 34eafd9 Compare November 17, 2021 12:41
@lxbsz
Copy link
Member Author

lxbsz commented Nov 17, 2021

Have removed the mds: bump truncate seq when fscrypt_file changes commit as discussed in #41284 (comment).

src/mds/CInode.cc Outdated Show resolved Hide resolved
@lxbsz lxbsz force-pushed the fsize_support branch 2 times, most recently from ca2c6cf to d818cdc Compare November 25, 2021 03:08
@lxbsz lxbsz marked this pull request as ready for review November 25, 2021 03:10
@lxbsz
Copy link
Member Author

lxbsz commented Nov 25, 2021

Cleaned up patches and added more comments, and also split the big patch into several small ones. Since Jeff and I have passed all the xfstests tool related tests in kclient with this PR, I have dropped the Draft flag.

@lxbsz
Copy link
Member Author

lxbsz commented Jan 12, 2022

Rebased it to the upstream and only changed the commit mds: add truncate size handling support for fscrypt by switching the object version to change_attr to gate the operation. At the same time I will post the kclient related patch today.

@lxbsz lxbsz requested a review from jtlayton January 12, 2022 01:11
@vshankar
Copy link
Contributor

jenkins test make check

@vshankar vshankar self-assigned this Jan 13, 2022
@github-actions
Copy link

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

jtlayton and others added 18 commits January 13, 2022 21:08
Signed-off-by: Jeff Layton <jlayton@redhat.com>
A flag isn't sufficient as we can't reasonably use an xattr to store the
context. Switch the fscrypt fields to two vectors of opaque bytes, one
governed by AUTH caps and the other by FILE.

Also remove the special handling for encryption.ctx xattr, since we
won't be using that going forward anyway.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Add fscrypt_auth and fscrypt_file attributes to the inode_t encoding.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
...and update those fields when the appropriate caps are
acquired/released/flushed.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
...and update the fscrypt_file field on setattr changes for size.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Add new mask bits for those fields and update them if a setattr request
comes in with them.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
We don't really expect the userland client to use these (at least not
initially) but plumb in new vxattrs for fetching and setting these
fields. The new vxattr just fetches whatever is in the in-core inode,
and setting it issues a setattr under the hood.

Also, ensure that we update these fields on cap updates and client
requests.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Create a file and set both fscrypt_auth and fscrypt_file on it. Verify
that they are set, remount and verify it again.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Encryption is currently only supported on files/directories with layouts
where stripe_count=1.  Forbid changing layouts when encryption is involved.

Signed-off-by: Luis Henriques <lhenriques@suse.de>
The clients will trust and need fscrypt_file field to truncate the
pagecaches and update the i_size.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
The maximum size of the last block without the header along with
a truncate request when the fscrypt is enabled. Default value is
4KB.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Xiubo Li <xiubli@redhat.com>
For now the userland clients hasn't support the fscrypt yet, they
will be only allowed to read the encrypted files.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
The kclient will only send truncate requests with the modified and
encrypted last block contents when new size is smaller and is not
aligned to CEPH_FSCRYPT_BLOCK_SIZE, which is 4KB for now.

Or if the Fx caps is issued and the new size is larger the kclient
will buffer the truncating. Or it will send truncate requests wihtout
the last block filled.

When the fscrypt is enabled and when truncating with a smaller size,
both the old size and new size in the truncate request will always
be rounded up to CEPH_FSCRYPT_BLOCK_SIZE, which is 4K for now, in
kclient. For example if truncating a file size from 3KB to 2KB, the
MDS will always get old_size == new_size == 4KB. So we need to check
whether there has last block data passed together with the truncate
request to make sure whether truncating to a smaller size.

The kclinet will send it's 'change_attr' along with the truncate req,
and the MDS will compare it with the one in CInode just after the MDS
successfully xlockes the CInode's filelock, if they are different
that means it's possibly some clients have update the file or have
dirty caps just before MDS xlockes the CInode's filelock. We will let
the kclient retry it by returning a -EAGAIN errno.

Then the MDS will write the last block to OSD and then truncate
the size as normal.

Currently the last block contents will be journaled together with
the project inode only and it will be cleared after the truncate
being finished, and won't make it persistent together with the
CInode:inode_t in the metadata pool.

Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
Copy link
Member Author

lxbsz commented Jan 13, 2022

@jtlayton

The conflicts are in the commit client: add support for fscrypt_auth and fscrypt_file fields in _do_setattr(), please take a look.
Thanks.

Copy link
Contributor

@jtlayton jtlayton left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Nice work.

@vshankar vshankar merged commit 1a76289 into ceph:master Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants