On Sat, 1 Mar 2014, Yan, Zheng wrote:
> ceph_fh_to_parent() finds the inode that corresponds to the 'ino' field
> of struct ceph_nfs_confh. This is wrong, it should find the inode that
> corresponds to the 'parent_ino' field.
>
> Signed-off-by: Yan, Zheng <zheng.z.yan@xxxxxxxxx>
It's been a while since I've looked at this, but I'm a bit confused.
If we have a get_parent() operation that will reliably get the parent
directory for an inode, why do we want/need the ceph_nfs_confh? If we
*do* have that struct and look up the parent_ino, we have no guarantee
that it is still the parent if there has been an intervening rename.
Would it be better to always use ceph_nfs_fh, and rely on LOOKUPPARENT
instead?
Thanks-
sage
> ---
> fs/ceph/export.c | 38 +++++---------------------------------
> 1 file changed, 5 insertions(+), 33 deletions(-)
>
> diff --git a/fs/ceph/export.c b/fs/ceph/export.c
> index 905d7f2..017af26 100644
> --- a/fs/ceph/export.c
> +++ b/fs/ceph/export.c
> @@ -122,49 +122,21 @@ static struct dentry *ceph_fh_to_dentry(struct super_block *sb,
> }
>
> /*
> - * get parent, if possible.
> - *
> - * FIXME: we could do better by querying the mds to discover the
> - * parent.
> + * convert regular fh to parent
> */
> static struct dentry *ceph_fh_to_parent(struct super_block *sb,
> - struct fid *fid,
> + struct fid *fid,
> int fh_len, int fh_type)
> {
> struct ceph_nfs_confh *cfh = (void *)fid->raw;
> - struct ceph_vino vino;
> - struct inode *inode;
> - struct dentry *dentry;
> - int err;
>
> - if (fh_type == 1)
> + if (fh_type != FILEID_INO32_GEN_PARENT)
> return ERR_PTR(-ESTALE);
> if (fh_len < sizeof(*cfh) / 4)
> return ERR_PTR(-ESTALE);
>
> - pr_debug("fh_to_parent %llx/%d\n", cfh->parent_ino,
> - cfh->parent_name_hash);
> -
> - vino.ino = cfh->ino;
> - vino.snap = CEPH_NOSNAP;
> - inode = ceph_find_inode(sb, vino);
> - if (!inode)
> - return ERR_PTR(-ESTALE);
> -
> - dentry = d_obtain_alias(inode);
> - if (IS_ERR(dentry)) {
> - pr_err("fh_to_parent %llx -- inode %p but ENOMEM\n",
> - cfh->ino, inode);
> - iput(inode);
> - return dentry;
> - }
> - err = ceph_init_dentry(dentry);
> - if (err < 0) {
> - iput(inode);
> - return ERR_PTR(err);
> - }
> - dout("fh_to_parent %llx %p dentry %p\n", cfh->ino, inode, dentry);
> - return dentry;
> + pr_debug("fh_to_parent %llx\n", cfh->parent_ino);
> + return __fh_to_dentry(sb, cfh->parent_ino);
> }
>
> const struct export_operations ceph_export_ops = {
> --
> 1.8.5.3
>
> --
> 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
>
>
--
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
