From patchwork Tue Apr 16 20:06:28 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gregory Farnum X-Patchwork-Id: 2450951 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 3CBDEDF230 for ; Tue, 16 Apr 2013 20:06:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935924Ab3DPUG3 (ORCPT ); Tue, 16 Apr 2013 16:06:29 -0400 Received: from mail-qe0-f51.google.com ([209.85.128.51]:36470 "EHLO mail-qe0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935907Ab3DPUG3 convert rfc822-to-8bit (ORCPT ); Tue, 16 Apr 2013 16:06:29 -0400 Received: by mail-qe0-f51.google.com with SMTP id 1so509881qec.38 for ; Tue, 16 Apr 2013 13:06:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:content-transfer-encoding :x-gm-message-state; bh=ha8da9Kz8k3SecGZMwj95NvO3vCsIEAT/C9mfmjN1K0=; b=ngLN5ZGBDbAged/7KgP9psFOhYUKirxyW+2s2tVErajXnURwtuv9nXHwbWQy8JiCJv W+Qy8xjWwijiRp0wAXmt8g4E7oo/7aZ+8d2YmPDxxVBrFIreSsfHaPHFnHilMRpsHTOw k/02thxEsm6bkXe7lrEx4uqDEf7N1NgGcOhSME9FKeYjL9zxdpkiNJdqjz+owQRgVrHl K4h5XQkxDGkPg6Yll3E6wbAlnG6BuNXsEHyIVD3cjBmpyuC/SEe9lP+CCCTcxqOtptcg OVMrNtF42dujy8NME9ORSJi7i/fmSrVsJcWat7oVeJDb3GhlqaDTYrLHjetVEAi7wbiD JGEg== MIME-Version: 1.0 X-Received: by 10.224.137.70 with SMTP id v6mr4368114qat.91.1366142788225; Tue, 16 Apr 2013 13:06:28 -0700 (PDT) Received: by 10.229.103.105 with HTTP; Tue, 16 Apr 2013 13:06:28 -0700 (PDT) In-Reply-To: <1366021404-5812-2-git-send-email-big.chiu@bigtera.com> References: <1366021404-5812-1-git-send-email-big.chiu@bigtera.com> <1366021404-5812-2-git-send-email-big.chiu@bigtera.com> Date: Tue, 16 Apr 2013 13:06:28 -0700 Message-ID: Subject: Re: [PATCH] mds: fix setting/removing xattrs on root From: Gregory Farnum To: Kuan Kai Chiu Cc: "ceph-devel@vger.kernel.org" , henry@bigtera.com X-Gm-Message-State: ALoCoQkh0Pabd7R5i9/MlBSJr3vEiCoOKoixxDTJ56ywOGCGlbpbhGQNgKHjiWV7A/gc+8sHty0c Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org On Mon, Apr 15, 2013 at 3:23 AM, Kuan Kai Chiu 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: 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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)