On Mon, Apr 15, 2013 at 3:23 AM, Kuan Kai Chiu <big.chiu@xxxxxxxxxxx> wrote:
> MDS crashes while journaling dirty root inode in handle_client_setxattr
> and handle_client_removexattr. We should use journal_dirty_inode to
> safely log root inode here.
> ---
> src/mds/Server.cc | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/mds/Server.cc b/src/mds/Server.cc
> index 11ab834..1e62dd2 100644
> --- a/src/mds/Server.cc
> +++ b/src/mds/Server.cc
> @@ -3907,8 +3907,7 @@ void Server::handle_client_setxattr(MDRequest *mdr)
> mdlog->start_entry(le);
> le->metablob.add_client_req(req->get_reqid(), req->get_oldest_client_tid());
> mdcache->predirty_journal_parents(mdr, &le->metablob, cur, 0, PREDIRTY_PRIMARY, false);
> - mdcache->journal_cow_inode(mdr, &le->metablob, cur);
> - le->metablob.add_primary_dentry(cur->get_projected_parent_dn(), true, cur);
> + mdcache->journal_dirty_inode(mdr, &le->metablob, cur);
>
> journal_and_reply(mdr, cur, 0, le, new C_MDS_inode_update_finish(mds, mdr, cur));
> }
> @@ -3964,8 +3963,7 @@ void Server::handle_client_removexattr(MDRequest *mdr)
> mdlog->start_entry(le);
> le->metablob.add_client_req(req->get_reqid(), req->get_oldest_client_tid());
> mdcache->predirty_journal_parents(mdr, &le->metablob, cur, 0, PREDIRTY_PRIMARY, false);
> - mdcache->journal_cow_inode(mdr, &le->metablob, cur);
> - le->metablob.add_primary_dentry(cur->get_projected_parent_dn(), true, cur);
> + mdcache->journal_dirty_inode(mdr, &le->metablob, cur);
>
> journal_and_reply(mdr, cur, 0, le, new C_MDS_inode_update_finish(mds, mdr, cur));
> }
This is fine as far as it goes, but we'll need your sign-off for us to
incorporate it into the codebase. Also, have you run any tests with
it? The reason I ask is that when I apply this patch, set an xattr on
the root inode, and then restart the MDS and client, there are no
xattrs on the root any more. I think this should fix that, but there
may be other such issues:
diff --git a/src/mds/events/EMetaBlob.h b/src/mds/events/EMetaBlob.h
index 7065460..439bd78 100644
--- a/src/mds/events/EMetaBlob.h
+++ b/src/mds/events/EMetaBlob.h
@@ -468,7 +468,7 @@ private:
if (!pi) pi = in->get_projected_inode();
if (!pdft) pdft = &in->dirfragtree;
- if (!px) px = &in->xattrs;
+ if (!px) px = in->get_projected_xattrs();
bufferlist snapbl;
if (psnapbl)
You've fallen victim to this new setup, incidentally — in the past the
root inode wasn't allowed to get any of these modifications because
it's not quite real in the way the rest of them are. We opened that up
when we made the virtual xattr interface, but we weren't very careful
about it so apparently we missed some side effects.
-Greg
Software Engineer #42 @ http://inktank.com | http://ceph.com
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
