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.

Sign up for GitHub

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

Jump to bottom

src/mds: increment directory inode's change attr by one #48241

Merged
merged 3 commits into from Oct 17, 2022
Merged

src/mds: increment directory inode's change attr by one #48241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d597100
test/libcephfs: enhance directory's change_attr testing
ajarr Sep 22, 2022
05fdd6c
src/mds: increment directory inode's change attr by one
ajarr Sep 22, 2022
f02b777
cephfs: add additional logging to track change_attr of inodes
ajarr Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/client/Client.cc
View file
Open in desktop
Expand Up @@ -1108,6 +1108,7 @@ Inode * Client::add_update_inode(InodeStat *st, utime_t from,
}

/* always take a newer change attr */
ldout(cct, 12) << __func__ << " client inode change_attr: " << in->change_attr << " , mds inodestat change_attr: " << st->change_attr << dendl;
if (st->change_attr > in->change_attr)
in->change_attr = st->change_attr;

Expand Down Expand Up @@ -8334,7 +8335,7 @@ void Client::fill_statx(Inode *in, unsigned int mask, struct ceph_statx *stx)
{
ldout(cct, 10) << __func__ << " on " << in->ino << " snap/dev" << in->snapid
<< " mode 0" << oct << in->mode << dec
<< " mtime " << in->mtime << " ctime " << in->ctime << dendl;
<< " mtime " << in->mtime << " ctime " << in->ctime << " change_attr " << in->change_attr << dendl;
memset(stx, 0, sizeof(struct ceph_statx));

/*
Expand Down
1 change: 1 addition & 0 deletions src/client/Inode.cc
View file
Open in desktop
Expand Up @@ -56,6 +56,7 @@ ostream& operator<<(ostream &out, const Inode &in)
<< " btime=" << in.btime
<< " mtime=" << in.mtime
<< " ctime=" << in.ctime
<< " change_attr=" << in.change_attr
<< " caps=" << ccap_string(in.caps_issued());
if (!in.caps.empty()) {
out << "(";
Expand Down
7 changes: 5 additions & 2 deletions src/mds/CInode.cc
View file
Open in desktop
Expand Up @@ -2543,7 +2543,8 @@ void CInode::finish_scatter_gather_update(int type, MutationRef& mut)
if (touched_mtime)
pi->mtime = pi->ctime = pi->dirstat.mtime;
if (touched_chattr)
pi->change_attr = pi->dirstat.change_attr;
pi->change_attr++;
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 know what I was thinking here. Your fix seems obviously correct.


dout(20) << " final dirstat " << pi->dirstat << dendl;

if (dirstat_valid && !dirstat.same_sums(pi->dirstat)) {
Expand Down Expand Up @@ -3799,7 +3800,9 @@ int CInode::encode_inodestat(bufferlist& bl, Session *session,
dout(20) << " pfile " << pfile << " pauth " << pauth
<< " plink " << plink << " pxattr " << pxattr
<< " plocal " << plocal
<< " mtime " << any_i->mtime
<< " ctime " << any_i->ctime
<< " change_attr " << any_i->change_attr
<< " valid=" << valid << dendl;

// file
Expand Down Expand Up @@ -4147,7 +4150,7 @@ void CInode::encode_cap_message(const ref_t<MClientCaps> &m, Capability *cap)

dout(20) << __func__ << " pfile " << pfile
<< " pauth " << pauth << " plink " << plink << " pxattr " << pxattr
<< " ctime " << i->ctime << dendl;
<< " mtime " << i->mtime << " ctime " << i->ctime << " change_attr " << i->change_attr << dendl;

i = pfile ? pi:oi;
m->set_layout(i->layout);
Expand Down
4 changes: 2 additions & 2 deletions src/mds/MDCache.cc
View file
Open in desktop
Expand Up @@ -2162,7 +2162,7 @@ void MDCache::predirty_journal_parents(MutationRef mut, EMetaBlob *blob,
if (do_parent_mtime) {
pf->fragstat.mtime = mut->get_op_stamp();
pf->fragstat.change_attr++;
dout(10) << "predirty_journal_parents bumping change_attr to " << pf->fragstat.change_attr << " on " << parent << dendl;
dout(10) << "predirty_journal_parents bumping fragstat change_attr to " << pf->fragstat.change_attr << " on " << parent << dendl;
if (pf->fragstat.mtime > pf->rstat.rctime) {
dout(10) << "predirty_journal_parents updating mtime on " << *parent << dendl;
pf->rstat.rctime = pf->fragstat.mtime;
Expand Down Expand Up @@ -2288,7 +2288,7 @@ void MDCache::predirty_journal_parents(MutationRef mut, EMetaBlob *blob,
if (touched_mtime)
pi.inode->mtime = pi.inode->ctime = pi.inode->dirstat.mtime;
if (touched_chattr)
pi.inode->change_attr = pi.inode->dirstat.change_attr;
pi.inode->change_attr++;
dout(20) << "predirty_journal_parents gives " << pi.inode->dirstat << " on " << *pin << dendl;

if (parent->get_frag() == frag_t()) { // i.e., we are the only frag
Expand Down
4 changes: 3 additions & 1 deletion src/messages/MClientCaps.h
View file
Open in desktop
Expand Up @@ -194,7 +194,9 @@ class MClientCaps final : public SafeMessage {
out << " size " << size << "/" << max_size;
if (truncate_seq)
out << " ts " << truncate_seq << "/" << truncate_size;
out << " mtime " << mtime;
out << " mtime " << mtime
<< " ctime " << ctime
<< " change_attr " << change_attr;
if (time_warp_seq)
out << " tws " << time_warp_seq;

Expand Down
123 changes: 111 additions & 12 deletions src/test/libcephfs/test.cc
View file
Open in desktop
Expand Up @@ -1870,40 +1870,139 @@ TEST(LibCephFS, ChangeAttr) {
ceph_shutdown(cmount);
}

TEST(LibCephFS, DirChangeAttr) {
TEST(LibCephFS, DirChangeAttrCreateFile) {
struct ceph_mount_info *cmount;
ASSERT_EQ(ceph_create(&cmount, NULL), 0);
ASSERT_EQ(ceph_conf_read_file(cmount, NULL), 0);
ASSERT_EQ(0, ceph_conf_parse_env(cmount, NULL));
ASSERT_EQ(ceph_mount(cmount, "/"), 0);

char dirname[32], filename[56];
sprintf(dirname, "/dirchange%x", getpid());
sprintf(filename, "%s/foo", dirname);
char dirpath[32], filepath[56];
sprintf(dirpath, "/dirchange%x", getpid());
sprintf(filepath, "%s/foo", dirpath);

ASSERT_EQ(ceph_mkdir(cmount, dirname, 0755), 0);
ASSERT_EQ(ceph_mkdir(cmount, dirpath, 0755), 0);

struct ceph_statx stx;
ASSERT_EQ(ceph_statx(cmount, dirname, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);

uint64_t old_change_attr = stx.stx_version;

int fd = ceph_open(cmount, filename, O_RDWR|O_CREAT|O_EXCL, 0666);
/* Important: Follow an operation that changes the directory's ctime (setxattr)
* with one that changes the directory's mtime and ctime (create).
* Check that directory's change_attr is updated everytime ctime changes.
*/

/* set xattr on dir, and check whether dir's change_attr is incremented */
ASSERT_EQ(ceph_setxattr(cmount, dirpath, "user.name", (void*)"bob", 3, XATTR_CREATE), 0);
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);
ASSERT_GT(stx.stx_version, old_change_attr);
old_change_attr = stx.stx_version;

/* create a file within dir, and check whether dir's change_attr is incremented */
int fd = ceph_open(cmount, filepath, O_RDWR|O_CREAT|O_EXCL, 0666);
ASSERT_LT(0, fd);
ceph_close(cmount, fd);
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);
ASSERT_GT(stx.stx_version, old_change_attr);

ASSERT_EQ(0, ceph_unlink(cmount, filepath));
ASSERT_EQ(0, ceph_rmdir(cmount, dirpath));
ceph_shutdown(cmount);
}

TEST(LibCephFS, DirChangeAttrRenameFile) {
struct ceph_mount_info *cmount;
ASSERT_EQ(ceph_create(&cmount, NULL), 0);
ASSERT_EQ(ceph_conf_read_file(cmount, NULL), 0);
ASSERT_EQ(0, ceph_conf_parse_env(cmount, NULL));
ASSERT_EQ(ceph_mount(cmount, "/"), 0);

char dirpath[32], filepath[56], newfilepath[56];
sprintf(dirpath, "/dirchange%x", getpid());
sprintf(filepath, "%s/foo", dirpath);

ASSERT_EQ(ceph_mkdir(cmount, dirpath, 0755), 0);

int fd = ceph_open(cmount, filepath, O_RDWR|O_CREAT|O_EXCL, 0666);
ASSERT_LT(0, fd);
ceph_close(cmount, fd);

ASSERT_EQ(ceph_statx(cmount, dirname, &stx, CEPH_STATX_VERSION, 0), 0);
/* Important: Follow an operation that changes the directory's ctime (setattr)
* with one that changes the directory's mtime and ctime (rename).
* Check that directory's change_attr is updated everytime ctime changes.
*/
struct ceph_statx stx;
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);
ASSERT_NE(stx.stx_version, old_change_attr);

uint64_t old_change_attr = stx.stx_version;

/* chmod dir, and check whether dir's change_attr is incremented */
ASSERT_EQ(ceph_chmod(cmount, dirpath, 0777), 0);
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);
ASSERT_GT(stx.stx_version, old_change_attr);
old_change_attr = stx.stx_version;

ASSERT_EQ(ceph_unlink(cmount, filename), 0);
ASSERT_EQ(ceph_statx(cmount, dirname, &stx, CEPH_STATX_VERSION, 0), 0);
/* rename a file within dir, and check whether dir's change_attr is incremented */
sprintf(newfilepath, "%s/bar", dirpath);
ASSERT_EQ(ceph_rename(cmount, filepath, newfilepath), 0);
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);
ASSERT_NE(stx.stx_version, old_change_attr);
ASSERT_GT(stx.stx_version, old_change_attr);

ASSERT_EQ(0, ceph_unlink(cmount, newfilepath));
ASSERT_EQ(0, ceph_rmdir(cmount, dirpath));
ceph_shutdown(cmount);
}

TEST(LibCephFS, DirChangeAttrRemoveFile) {
struct ceph_mount_info *cmount;
ASSERT_EQ(ceph_create(&cmount, NULL), 0);
ASSERT_EQ(ceph_conf_read_file(cmount, NULL), 0);
ASSERT_EQ(0, ceph_conf_parse_env(cmount, NULL));
ASSERT_EQ(ceph_mount(cmount, "/"), 0);

char dirpath[32], filepath[56];
sprintf(dirpath, "/dirchange%x", getpid());
sprintf(filepath, "%s/foo", dirpath);

ASSERT_EQ(ceph_mkdir(cmount, dirpath, 0755), 0);

ASSERT_EQ(ceph_setxattr(cmount, dirpath, "user.name", (void*)"bob", 3, XATTR_CREATE), 0);

int fd = ceph_open(cmount, filepath, O_RDWR|O_CREAT|O_EXCL, 0666);
ASSERT_LT(0, fd);
ceph_close(cmount, fd);

/* Important: Follow an operation that changes the directory's ctime (removexattr)
* with one that changes the directory's mtime and ctime (remove a file).
* Check that directory's change_attr is updated everytime ctime changes.
*/
struct ceph_statx stx;
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);

uint64_t old_change_attr = stx.stx_version;

/* remove xattr, and check whether dir's change_attr is incremented */
ASSERT_EQ(ceph_removexattr(cmount, dirpath, "user.name"), 0);
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);
ASSERT_GT(stx.stx_version, old_change_attr);
old_change_attr = stx.stx_version;

/* remove a file within dir, and check whether dir's change_attr is incremented */
ASSERT_EQ(0, ceph_unlink(cmount, filepath));
ASSERT_EQ(ceph_statx(cmount, dirpath, &stx, CEPH_STATX_VERSION, 0), 0);
ASSERT_TRUE(stx.stx_mask & CEPH_STATX_VERSION);
ASSERT_GT(stx.stx_version, old_change_attr);

ASSERT_EQ(0, ceph_rmdir(cmount, dirpath));
ceph_shutdown(cmount);
}

Expand Down