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

cephfs: support WORM(Write Once Read Many) feature #26691

Closed
wants to merge 8 commits into from

Conversation

weiqiaomiao
Copy link
Contributor

@weiqiaomiao weiqiaomiao commented Feb 28, 2019

Write Once Read Many (WORM) technology is used in many applications for data integrity reasons and because of the accepted legal admissibility of stored files that use the technology. And we can see some distribute filesystem such as glusterfs support WORM feature. Driven by the needs of our customer , we are working on Cephfs support WORM(write once read only) feature.
In our implementation, the worm feature is disabled default, and can enabled on a exist empty dir . A dir which enable worm feature called a worm root dir. When a subdir or a file create under worm root dir, it auto inherit the worm attibute from worm root dir. A file which enable worm feature called a worm file. The worm file has three state:
(1) normal state: the initial state of a file. in this state, the worm file is mutable and deletable。
(2) retained state: in this state, the worm file is immutable or undeletable.
(3) expired state: in this state, the worm like as the normal file, it is mutable and deletable (the worm file is immutable but deletable in some other storage).

A file makes transition between these 3 states. Methods of making WORM/Retention transition:
(1) manual commit: Using chattr +i command to make the worm file through into retained state immediately.
(2) automatic commit: Automatic transition between the different states of a file. Dormant Files (files which are untouched for a long time) will be converted into WORM files based on auto-commit period .

Retention Profiles/Policies: Configurable Policies that guide the WORM/Retention behavior. Contains,
(1) Default Retention Period: default time period till which a WORM file should be retained(undeletable)
(2) Auto-commit Period: a worm file get into retained state if it is untouched in auto commit period.
(3) minimum retention period: minimum time period for users config on a worm file
(4) max retention period: max time period for user config on a worm file

Cephfs Modification:
(1) add worm state and retention policies into inode per file
(2) worm feature can enable on any empty dir, but once it enabled, it can't disabled. we can enable worm feature by setfattr like:

setfattr -n ceph.worm -v "enable=1 auto_commit_period=120 min_retention_period=240 max_retention_period=7200 retention_period=360" dir1

or

setfattr -n ceph.worm.enable -v 1 dir1
setfattr -n ceph.worm.enable -v 120 dir1
setfattr -n ceph.worm.min_retention_period -v 240 dir1
setfattr -n ceph.worm.max_retention_period -v 7200 dir1
setfattr -n ceph.worm.retention_period -v 360 dir1

and get worm state and policies by getfattr like:

 getfattr -n ceph.worm dir1

(3) Lazy Auto-commit: IO Triggered Using timeouts for untouched files. The modify IO(write、truncate、unlink.etc) will cause the transition.

(4) Using the file atime save WORM file protection expiration time.
manual commit : atime = mtime + retention_period
atomatic commit: atime = mtime + auto_commit_period + retention_period
user can change the worm file retention period ( must between minimum retention period ~ max retention period) through change file Atime.

(5) hard link don't support if worm feature enable.

Signed-off-by: Wei Qiaomiao wei.qiaomiao@zte.com.cn

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

@xiexingguo xiexingguo added feature cephfs Ceph File System labels Feb 28, 2019
.set_default(120)
.set_description("default auto commit period for worm feature"),

Option("mds_worm_retention_period", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
Copy link
Member

Choose a reason for hiding this comment

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

Seems these three options are all sharing the same default value. Did you do that deliberately?

Also I've observed you silently changed the perm of all files you touched from 100644->100755, which I guess should definitely need to be addressed first..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These options had been redesigned, I think mds_worm_retention_period may be equal to mds_worm_min_retention_period and set them to "1h", mds_worm_max_retention_period may be larger than 'mds_worm_min_retention_period` and set it to "30 years", What do you think?

the modified file mode already restore to 100644, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

You might switch to the inline .set_min & .set_max helpers to perform the boundary checking, then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it,thanks

.set_description("rate of decay in seconds for calculating request load average"),

Option("mds_worm_commit_period", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
.set_default(7200)
Copy link
Member

Choose a reason for hiding this comment

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

s/7200/2_hr/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it,thanks

.set_description("default auto commit period for worm feature"),

Option("mds_worm_retention_period", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
.set_default(3600)
Copy link
Member

Choose a reason for hiding this comment

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

s/3600/1_hr/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it,thanks

OPTION(mds_worm_commit_period, OPT_U64)
OPTION(mds_worm_retention_period, OPT_U64)
OPTION(mds_worm_min_retention_period, OPT_U64)
OPTION(mds_worm_max_retention_period, OPT_U64)
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to add options to legacy_config_opts.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,fixed it,thanks

ENCODE_FINISH(bl);
}
void decode(bufferlist::iterator& p) {
DECODE_START_LEGACY_COMPAT_LEN(1, 1, 1, p);
Copy link
Contributor

Choose a reason for hiding this comment

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

DECODE_START()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it,thanks

@@ -3687,6 +3687,9 @@ int CInode::encode_inodestat(bufferlist& bl, Session *session,
}
}

mempool_inode *policy_i = ppolicy ? pi : oi;
::encode(policy_i->worm, bl);

Copy link
Contributor

Choose a reason for hiding this comment

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

needs to bump encoding version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it,thanks

@@ -254,6 +256,8 @@ struct InodeStat {
change_attr = 0;
}
}

::decode(worm, p);
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to check encoding version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it,thanks

@weiqiaomiao weiqiaomiao force-pushed the wqm-wip-worm branch 5 times, most recently from 51ba90b to bcfb8f0 Compare March 2, 2019 08:32
@xiexingguo xiexingguo requested a review from jtlayton March 4, 2019 06:52
@weiqiaomiao
Copy link
Contributor Author

@ukernel @batrick @jtlayton I reorganized all the commit, and makes the code more readable now, can you take a look it?

req->head.args.setattr.mode = stx->stx_mode;
req->inode_drop |= CEPH_CAP_AUTH_SHARED;

if (in->worm.is_enable() && !in->worm.is_retain() && !(stx->stx_mode & 00222)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I really like treating a mode with no write bits as special.

What about POSIX ACLs here? Suppose there are no write bits in the mode but there is a POSIX ACL set that explicitly allows write access for another user? Should that trigger this transition?

A better mechanism might be to wire up the appropriate ioctl so that you could use "chattr -i" here. That would make it a privileged operation, but that seems reasonable for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion, jtlaytion!

My original intention was to find a simple way to trigger the conversion of the state and removing the Write property in mode was just one of the methods, and I didn't take into account the POSIX ACL scene.

I think your suggestion is more reasonable, but I'm not familiar with chattr, so I think I need a little time to learn how to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically, you'd just need to wire up FS_IOC_GETFLAGS and FS_IOC_SETFLAGS and fix them up to handle the IMMUTABLE flag. FUSE has an ioctl handler, so it should be possible to do this for both the kernel and fuse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm very sorry for delay in reply. I add ll_set_flags() function to handle IMMUTABLE flag (make
the worm state transition in it) and ll_get_flags() function to get the flags . Can you take a look at it?

@renhwztetecs
Copy link
Contributor

As a document backup application scenario, WORM is an important function. It can protect files from being deleted and modified easily.

if (mask & CEPH_SETATTR_ATIME)
in->atime = utime_t(stx->stx_atime);
if (mask & CEPH_SETATTR_ATIME) {
uint32_t min_atime_sec = in->mtime.sec()+ in->worm.min_retention_period;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the acquisition time comes from the local, there is a modification of the local time to cause the folder to enter the protection period in advance. It may be provided by providing an external standard clock source interface or in the document: "Synchronize the time before setting the WORM function"

What do you think? @ukernel @batrick @jtlayton

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be better to not overload the atime field. While it's not terribly common, some applications do use it. Why not store this in a separate field in the inode and present it via xattr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Already drop to use atime and add a new field in worm info struct insteed.

@weiqiaomiao weiqiaomiao force-pushed the wqm-wip-worm branch 2 times, most recently from 9d111d9 to 607134b Compare March 16, 2019 06:36
@batrick
Copy link
Member

batrick commented Mar 19, 2019

This looks interesting but before I dig into it I'd like you to cleanup the commit history. For example, 58fd92a needs rewritten to remove the addition to src/common/legacy_config_opts.h. Use git rebase -i and edit the commits as necessary. Please also make sure you don't add unnecessary whitespace changes. I also noticed a lot of trailing whitespace in your patches.

@weiqiaomiao weiqiaomiao force-pushed the wqm-wip-worm branch 5 times, most recently from fecc6a8 to 32e2560 Compare April 2, 2019 12:30
@weiqiaomiao
Copy link
Contributor Author

jenkins,retest this please

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.

I think you mean "auto-inherit" in the title.

@@ -8736,14 +8744,21 @@ int Client::_open(Inode *in, int flags, mode_t mode, Fh **fhp,
return -EROFS;
}

int result = 0;
if (in->is_file() && (flags & (O_WRONLY | O_RDWR | O_TRUNC | O_APPEND))&& in->worm.is_enable()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: please fix the whitespace above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it,thanks

@@ -347,6 +347,8 @@ class Server {
void handle_conf_change(const ConfigProxy& conf,
const std::set <std::string> &changed);

// int worm_state_transition(Inode *in, int op);

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look related to the rest of the patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

already deleted

@@ -11690,6 +11705,62 @@ int Client::ll_removexattr(Inode *in, const char *name, const UserPerm& perms)
return _removexattr(in, name, perms);
}

int Client::worm_state_transition(Inode *in, const UserPerm& perms, int op)
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment header would be nice here. What, specifically, is this function intended to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

if (*flags & (~FS_IMMUTABLE_FL)) {
fuse_reply_err(req, EOPNOTSUPP);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little concerned with this. Suppose I accidentally set this on the wrong file. How do I fix it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think may be only wait for file protection time expires 。any suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think waiting will be acceptable -- we will need to be able to fix this in some fashion, as we can't just throw away the filesystem and start over (as you would with a CD-R or something).

Maybe this could only be allowed from certain clients using a new cephfs client capability flag, and those clients would be able to undo this as well?

http://docs.ceph.com/docs/master/cephfs/client-auth/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK ! Already add a new cap flag "i" and "chattr -i"" to expired a file which is in retained state.

if (dir->worm.is_enable() || in->worm.is_enable()) {
ldout(cct, 8) << "_link" << "worm feature doesn't support link " << dendl;
return -EPERM;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We expect all files in the worm directory to be controlled,but it hardlink is supported,if a non-worm file linked to a file under worm directory, it can't controlled by worm.

parse_worm_vxattr(mdr, cur, "worm.enable", "1", worm);
}

if (q == 0xffffffff) { // retetion period ulimit
Copy link
Contributor

Choose a reason for hiding this comment

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

Use UINT_MAX instead of 0xffffffff (here and below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

#define WORM_EXPIRE 8 // the worm file is in expired state
#define WORM_RETEN_PERIOD_UNLIMIT 16 //the worm file retention period is unlimit
#define MAX_MIN_RETEN_PERIOD 946080000 // 30 years
#define MIN_MAX_RETEN_PERIOD 2207520000 // 70 years
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these max/min values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our customers require a minimum retention period that can be set from 0 to 30 years, and a maximum retention period from 0 to 70 years. And to avoid other people being confused like you, I will remove them and put the restrictions on the configuration of our own management system.

if (*flags & (~FS_IMMUTABLE_FL)) {
fuse_reply_err(req, EOPNOTSUPP);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think waiting will be acceptable -- we will need to be able to fix this in some fashion, as we can't just throw away the filesystem and start over (as you would with a CD-R or something).

Maybe this could only be allowed from certain clients using a new cephfs client capability flag, and those clients would be able to undo this as well?

http://docs.ceph.com/docs/master/cephfs/client-auth/

@stale
Copy link

stale bot commented Aug 7, 2019

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Aug 7, 2019
@batrick batrick removed their assignment Aug 7, 2019
@stale stale bot removed the stale label Aug 7, 2019
@weiqiaomiao weiqiaomiao force-pushed the wqm-wip-worm branch 5 times, most recently from cacf095 to 830fea1 Compare August 23, 2019 02:50
@weiqiaomiao
Copy link
Contributor Author

jenkins retest this please

Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
we can set multiple worm attribute at once:
`setfattr -n ceph.worm -v "enable=1 auto_commit_period=60 retention_period=120 min_retention_period=120 max_retention_period=240" dir1`
or set one worm attribute at a time like:

`
setfattr -n ceph.worm.enable -v 1 dir1
setfattr -n ceph.worm.auto_commit_period -v 60 dir1
setfattr -n ceph.worm.min_retention_period -v 120 dir1
setfattr -n ceph.worm.max_retention_period -v 240 dir1
setfattr -n ceph.worm.retention_period -v 120 dir1
`

we can get all file worm  attibute:
`getfattr -n ceph.worm dir1`

or get one file attribute at a time.
`getfattr -n ceph.state file1`

Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
…nd worm state transition

Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
Signed-off-by: Wei Qiaomiao <wei.qiaomiao@zte.com.cn>
@stale
Copy link

stale bot commented Nov 1, 2019

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Nov 1, 2019
@stale
Copy link

stale bot commented Jan 30, 2020

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@stale stale bot closed this Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants