diff mbox

mds: Use stray dir as base inode for stray reintegration

Message ID alpine.DEB.2.00.1210030811350.21194@cobra.newdream.net (mailing list archive)
State New, archived
Headers show

Commit Message

Sage Weil Oct. 3, 2012, 3:17 p.m. UTC 
On Wed, 3 Oct 2012, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> Server::handle_client_rename() only skips common ancestor check
> if source path's base inode is stray directory, but source path's
> base inode is mdsdir in the stray reintegration case.
> 
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>

Hmm, I think this is a problem because path_traverse() does not know how 
to get started with a stray dir it doesn't already have.  It might make 
more sense to use the full path here, and make the handle_client_rename() 
check smarter.

I'm testing this:



Thanks!
sage


> ---
>  src/mds/MDCache.cc | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
> index 8b02b8b..32c9e36 100644
> --- a/src/mds/MDCache.cc
> +++ b/src/mds/MDCache.cc
> @@ -8288,8 +8288,7 @@ void MDCache::reintegrate_stray(CDentry *straydn, CDentry *rdn)
>    dout(10) << "reintegrate_stray " << *straydn << " into " << *rdn << dendl;
>    
>    // rename it to another mds.
> -  filepath src;
> -  straydn->make_path(src);
> +  filepath src(straydn->get_name(), straydn->get_dir()->get_inode()->ino());
>    filepath dst;
>    rdn->make_path(dst);
>  
> -- 
> 1.7.11.4
> 
> 
--
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

Comments

Yan, Zheng Oct. 3, 2012, 4:01 p.m. UTC | #1 
On 10/03/2012 11:17 PM, Sage Weil wrote:
> On Wed, 3 Oct 2012, Yan, Zheng wrote:
>> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>>
>> Server::handle_client_rename() only skips common ancestor check
>> if source path's base inode is stray directory, but source path's
>> base inode is mdsdir in the stray reintegration case.
>>
>> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
> 
> Hmm, I think this is a problem because path_traverse() does not know how 
> to get started with a stray dir it doesn't already have.  It might make 
> more sense to use the full path here, and make the handle_client_rename() 
> check smarter.
> 
> I'm testing this:
> 
> diff --git a/src/mds/Server.cc b/src/mds/Server.cc
> index b706b5a..0c3ff96 100644
> --- a/src/mds/Server.cc
> +++ b/src/mds/Server.cc
> @@ -5066,7 +5066,8 @@ void Server::handle_client_rename(MDRequest *mdr)
>  
>    // src+dest traces _must_ share a common ancestor for locking to prevent orphans
>    if (destpath.get_ino() != srcpath.get_ino() &&
> -      !MDS_INO_IS_STRAY(srcpath.get_ino())) {  // <-- mds 'rename' out of stray dir is ok!
> +      !(req->get_source()->is_mds() &&
> +       MDS_INO_IS_MDSDIR(srcpath.get_ino()))) {  // <-- mds 'rename' out of stray dir is ok!
>      // do traces share a dentry?
>      CDentry *common = 0;
>      for (unsigned i=0; i < srctrace.size(); i++) {
> 
>
FYI: MDCache::migrate_stray() uses stray directory as base inode, it requires an update.
--
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 mbox

Patch

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index b706b5a..0c3ff96 100644
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -5066,7 +5066,8 @@  void Server::handle_client_rename(MDRequest *mdr)
 
   // src+dest traces _must_ share a common ancestor for locking to prevent orphans
   if (destpath.get_ino() != srcpath.get_ino() &&
-      !MDS_INO_IS_STRAY(srcpath.get_ino())) {  // <-- mds 'rename' out of stray dir is ok!
+      !(req->get_source()->is_mds() &&
+       MDS_INO_IS_MDSDIR(srcpath.get_ino()))) {  // <-- mds 'rename' out of stray dir is ok!
     // do traces share a dentry?
     CDentry *common = 0;
     for (unsigned i=0; i < srctrace.size(); i++) {