From: Jeff Layton <jlayton@redhat.com>
Date: Mon, 24 Oct 2016 14:03:00 +0000 (-0400)
Subject: client: convert ceph_ll_link to UserPerm and remove struct stat parameter
X-Git-Tag: v11.1.0~383^2~12
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9725a33d40271166eb0333b49b6039adacd931c3;p=ceph.git

client: convert ceph_ll_link to UserPerm and remove struct stat parameter

The main user of this API (ganesha) doesn't do anything with the
returned attributes, so there's no real point in returning them
there.

Also, we're not guaranteed to have any caps on the target inode
after the link operation, so in the case of FUSE (which does
require the post-op attributes) we should really do a getattr
to get the latest attributes.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---

diff --git a/src/client/Client.cc b/src/client/Client.cc
index c8db94bef4e7..12cd46471728 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -11532,7 +11532,7 @@ int Client::_link(Inode *in, Inode *dir, const char *newname, const UserPerm& pe
 }
 
 int Client::ll_link(Inode *in, Inode *newparent, const char *newname,
-		    struct stat *attr, const UserPerm& perm)
+		    const UserPerm& perm)
 {
   Mutex::Locker lock(client_lock);
 
@@ -11550,25 +11550,19 @@ int Client::ll_link(Inode *in, Inode *newparent, const char *newname,
   InodeRef target;
 
   if (!cct->_conf->fuse_default_permissions) {
-    if (S_ISDIR(in->mode)) {
-      r = -EPERM;
-      goto out;
-    }
+    if (S_ISDIR(in->mode))
+      return -EPERM;
+
     r = may_hardlink(in, perm);
     if (r < 0)
-      goto out;
+      return r;
+
     r = may_create(newparent, perm);
     if (r < 0)
-      goto out;
+      return r;
   }
 
-  r = _link(in, newparent, newname, perm, &target);
-  if (r == 0) {
-    assert(target);
-    fill_stat(target, attr);
-  }
-out:
-  return r;
+  return _link(in, newparent, newname, perm, &target);
 }
 
 int Client::ll_num_osds(void)
diff --git a/src/client/Client.h b/src/client/Client.h
index 91badbfc45de..cbe7b1fb32f8 100644
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -1154,7 +1154,7 @@ public:
   int ll_rename(Inode *parent, const char *name, Inode *newparent,
 		const char *newname, const UserPerm& perm);
   int ll_link(Inode *in, Inode *newparent, const char *newname,
-	      struct stat *attr, const UserPerm& perm);
+	      const UserPerm& perm);
   int ll_open(Inode *in, int flags, Fh **fh, const UserPerm& perms);
   int _ll_create(Inode *parent, const char *name, mode_t mode,
 	      int flags, InodeRef *in, int caps, Fh **fhp,
diff --git a/src/client/SyntheticClient.cc b/src/client/SyntheticClient.cc
index 9c44d7a334b0..14e2c567a7ea 100644
--- a/src/client/SyntheticClient.cc
+++ b/src/client/SyntheticClient.cc
@@ -1344,12 +1344,11 @@ int SyntheticClient::play_trace(Trace& t, string& prefix, bool metadata_only)
       int64_t i = t.get_int();
       int64_t ni = t.get_int();
       const char *nn = t.get_string(buf, p);
-      struct stat attr;
       if (ll_inos.count(i) &&
 	  ll_inos.count(ni)) {
 	i1 = client->ll_get_inode(vinodeno_t(ll_inos[i],CEPH_NOSNAP));
 	i2 = client->ll_get_inode(vinodeno_t(ll_inos[ni],CEPH_NOSNAP));
-	client->ll_link(i1, i2, nn, &attr, perms);
+	client->ll_link(i1, i2, nn, perms);
 	client->ll_put(i1);
 	client->ll_put(i2);
       }
diff --git a/src/client/fuse_ll.cc b/src/client/fuse_ll.cc
index 97bca93935a2..f63f9274af6b 100644
--- a/src/client/fuse_ll.cc
+++ b/src/client/fuse_ll.cc
@@ -515,14 +515,22 @@ static void fuse_ll_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t newparent,
   UserPerm perm(ctx->uid, ctx->gid);
   GET_GROUPS(perm, req);
   
-  int r = cfuse->client->ll_link(in, nin, newname, &fe.attr, perm);
+  /*
+   * Note that we could successfully link, but then fail the subsequent
+   * getattr and return an error. Perhaps we should ignore getattr errors,
+   * but then how do we tell FUSE that the attrs are bogus?
+   */
+  int r = cfuse->client->ll_link(in, nin, newname, perm);
   if (r == 0) {
-    fe.ino = cfuse->make_fake_ino(fe.attr.st_ino, fe.attr.st_dev);
-    fe.attr.st_rdev = new_encode_dev(fe.attr.st_rdev);
-    fuse_reply_entry(req, &fe);
-  } else {
-    fuse_reply_err(req, -r);
+    r = cfuse->client->ll_getattr(in, &fe.attr, perm);
+    if (r == 0) {
+      fe.ino = cfuse->make_fake_ino(fe.attr.st_ino, fe.attr.st_dev);
+      fe.attr.st_rdev = new_encode_dev(fe.attr.st_rdev);
+      fuse_reply_entry(req, &fe);
+    }
+  }
 
+  if (r != 0) {
     /*
      * Many ll operations in libcephfs return an extra inode reference, but
      * ll_link currently does not. Still, FUSE needs one for the new dentry,
@@ -530,6 +538,7 @@ static void fuse_ll_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t newparent,
      * On error however, we must put that reference.
      */
     cfuse->iput(in);
+    fuse_reply_err(req, -r);
   }
 
   cfuse->iput(nin);
diff --git a/src/include/cephfs/libcephfs.h b/src/include/cephfs/libcephfs.h
index 286973ff661f..39582af51138 100644
--- a/src/include/cephfs/libcephfs.h
+++ b/src/include/cephfs/libcephfs.h
@@ -1490,8 +1490,8 @@ int ceph_ll_mkdir(struct ceph_mount_info *cmount, Inode *parent,
 		  struct ceph_statx *stx, unsigned want,
 		  unsigned flags, const UserPerm *perms);
 int ceph_ll_link(struct ceph_mount_info *cmount, struct Inode *in,
-		 struct Inode *newparrent, const char *name,
-		 struct stat *attr, int uid, int gid);
+		 struct Inode *newparent, const char *name,
+		 const UserPerm *perms);
 int ceph_ll_opendir(struct ceph_mount_info *cmount, struct Inode *in,
 		    struct ceph_dir_result **dirpp, int uid, int gid);
 int ceph_ll_releasedir(struct ceph_mount_info *cmount,
diff --git a/src/libcephfs.cc b/src/libcephfs.cc
index b565065756be..dd2e648afc95 100644
--- a/src/libcephfs.cc
+++ b/src/libcephfs.cc
@@ -1568,11 +1568,9 @@ extern "C" int ceph_ll_mkdir(class ceph_mount_info *cmount, Inode *parent,
 
 extern "C" int ceph_ll_link(class ceph_mount_info *cmount,
 			    Inode *in, Inode *newparent,
-			    const char *name, struct stat *attr, int uid,
-			    int gid)
+			    const char *name, const UserPerm *perms)
 {
-  UserPerm perms(uid, gid);
-  return (cmount->get_client()->ll_link(in, newparent, name, attr, perms));
+  return cmount->get_client()->ll_link(in, newparent, name, *perms);
 }
 
 extern "C" int ceph_ll_opendir(class ceph_mount_info *cmount,
diff --git a/src/test/libcephfs/test.cc b/src/test/libcephfs/test.cc
index 54164e31a634..385ac98d7a12 100644
--- a/src/test/libcephfs/test.cc
+++ b/src/test/libcephfs/test.cc
@@ -1396,7 +1396,6 @@ TEST(LibCephFS, Nlink) {
   sprintf(filename, "nlinkorig%x", getpid());
   sprintf(linkname, "nlinklink%x", getpid());
 
-  struct stat	st;
   struct ceph_statx stx;
   Fh *fh;
   UserPerm *perms = ceph_mount_perms(cmount);
@@ -1406,8 +1405,9 @@ TEST(LibCephFS, Nlink) {
 			   &file, &fh, &stx, CEPH_STATX_NLINK, 0, perms), 0);
   ASSERT_EQ(stx.stx_nlink, (nlink_t)1);
 
-  ASSERT_EQ(ceph_ll_link(cmount, file, dir, linkname, &st, getuid(), getgid()), 0);
-  ASSERT_EQ(st.st_nlink, (nlink_t)2);
+  ASSERT_EQ(ceph_ll_link(cmount, file, dir, linkname, perms), 0);
+  ASSERT_EQ(ceph_ll_getattr(cmount, file, &stx, CEPH_STATX_NLINK, 0, perms), 0);
+  ASSERT_EQ(stx.stx_nlink, (nlink_t)2);
 
   ASSERT_EQ(ceph_ll_unlink(cmount, dir, linkname, getuid(), getgid()), 0);
   ASSERT_EQ(ceph_ll_lookup(cmount, dir, filename, &file, &stx,