LOOKUPPARENT

LOOKUPPARENT

[Sage Weil]

Hey Zheng,

The recent patch that changed LOOKUPPARENT probably broke getcwd() in 
Client.cc:

void Client::getcwd(string& dir)
{
  filepath path;
  ldout(cct, 10) << "getcwd " << *cwd << dendl;

  Inode *in = cwd;
  while (in != root) {
    assert(in->dn_set.size() < 2); // dirs can't be hard-linked
    Dentry *dn = in->get_first_parent();
    if (!dn) {
      // look it up
      ldout(cct, 10) << "getcwd looking up parent for " << *in << dendl;
      MetaRequest *req = new MetaRequest(CEPH_MDS_OP_LOOKUPPARENT);
      filepath path(in->ino);
      req->set_filepath(path);
      req->set_inode(in);
      int res = make_request(req, -1, -1);
      if (res < 0)
	break;

      // start over
      path = filepath();
      in = cwd;
      continue;
    }
    path.push_front_dentry(dn->name);
    in = dn->dir->parent_inode;
  }
  dir = "/";
  dir += path.get_path();
}

The old version would return the diri + dentry + in, the new version just 
returns dir.   I don't see an old user in the kernel code, but I wonder if 
we should 

- keep the old semantics (diri + dentry + in) and make the kernel 
essentially ignore this information, or

- change getcwd to use LOOKUPINO + WANT_DENTRY to make it work.

?
sage

Re: LOOKUPPARENT

[Yan, Zheng]

On 03/07/2014 06:44 AM, Sage Weil wrote:
> Hey Zheng,
> 
> The recent patch that changed LOOKUPPARENT probably broke getcwd() in 
> Client.cc:
> 
> void Client::getcwd(string& dir)
> {
>   filepath path;
>   ldout(cct, 10) << "getcwd " << *cwd << dendl;
> 
>   Inode *in = cwd;
>   while (in != root) {
>     assert(in->dn_set.size() < 2); // dirs can't be hard-linked
>     Dentry *dn = in->get_first_parent();
>     if (!dn) {
>       // look it up
>       ldout(cct, 10) << "getcwd looking up parent for " << *in << dendl;
>       MetaRequest *req = new MetaRequest(CEPH_MDS_OP_LOOKUPPARENT);
>       filepath path(in->ino);
>       req->set_filepath(path);
>       req->set_inode(in);
>       int res = make_request(req, -1, -1);
>       if (res < 0)
> 	break;
> 
>       // start over
>       path = filepath();
>       in = cwd;
>       continue;
>     }
>     path.push_front_dentry(dn->name);
>     in = dn->dir->parent_inode;
>   }
>   dir = "/";
>   dir += path.get_path();
> }
> 
> The old version would return the diri + dentry + in, the new version just 
> returns dir.   I don't see an old user in the kernel code, but I wonder if 
> we should 
> 
> - keep the old semantics (diri + dentry + in) and make the kernel 
> essentially ignore this information, or
> 
> - change getcwd to use LOOKUPINO + WANT_DENTRY to make it work.

I prefer to change getcwd. The problem of old LOOKUPPARENT semantic is that
client can't lock the parent inode in advance, This breaks VFS locking rules
for lookup.

I added the getcwd fix to https://github.com/ceph/ceph/pull/1385

Regards
Yan, Zheng

> 
> ?
> sage
>