libcephfs: add ceph_fsetattr&&ceph_lchmod&&ceph_lutime #11191
Conversation
renhwztetecs
force-pushed
the
renhw-wip-add-libcephfs
branch
from
98a9f91
to
73d0b7f
Compare
|
Can you comment on why lchmod is needed? It doesn't seem to part of e.g. Linux's file I/O implementation. |
|
...and same question about lutime. |
|
@jcsp |
|
Right, but ordinary applications presumably don't use calls like this, since they're not part of the usual Linux stdlib interface. So why does libcephfs need them? |
|
forgive my ignorant and ill-informed, and I get it I dropped ceph_lchmod() && ceph_lutime() |
renhwztetecs
force-pushed
the
renhw-wip-add-libcephfs
branch
from
73d0b7f
to
2ad8445
Compare
|
Hmm, would lchmod have allowed setting the mode of a symlink object rather than it's target? nfs-ganesha could actually use such a function, the NFS protocol allows setting attributes of symlink objects and freebsd at least supports lchmod so we could have clients trying it. |
|
Sorry for the delay. I'm okay with this now as the fsetattr is at least a normal function on some UNIX variants. @renhwztetecs please could you add a unit test for this in test/libcephfs/test.cc. |
|
This currently does not compile when merged to master, needs a rebase (change in definition of Client::fsetattr) |
renhwztetecs
force-pushed
the
renhw-wip-add-libcephfs
branch
from
2ad8445
to
f5fb6ae
Compare
|
@jcsp I will add unit test for fsetattr |
renhwztetecs
force-pushed
the
renhw-wip-add-libcephfs
branch
from
f5fb6ae
to
b399d65
Compare
Signed-off-by: huanwen ren <ren.huanwen@zte.com.cn>
renhwztetecs
force-pushed
the
renhw-wip-add-libcephfs
branch
from
eca0620
to
6ea7cab
Compare
|
@jcsp |
| uint64_t size = 8388608; | ||
| stx.stx_size = size; | ||
|
|
||
| ASSERT_EQ(ceph_fsetattr(cmount, fd, &stx, CEPH_SETATTR_SIZE), 0); |
There was a problem hiding this comment.
You see to be passing a ceph_statx to a function that expects a struct stat?
There was a problem hiding this comment.
thanks for your reviews
I changed it
renhwztetecs
force-pushed
the
renhw-wip-add-libcephfs
branch
3 times, most recently
from
36c98d6
to
edcf29f
Compare
|
@jcsp |
| @@ -1158,6 +1158,7 @@ TEST(LibCephFS, UseUnmounted) { | |||
| EXPECT_EQ(-ENOTCONN, ceph_lremovexattr(cmount, "/path", "name")); | |||
| EXPECT_EQ(-ENOTCONN, ceph_setxattr(cmount, "/path", "name", NULL, 0, 0)); | |||
| EXPECT_EQ(-ENOTCONN, ceph_lsetxattr(cmount, "/path", "name", NULL, 0, 0)); | |||
| EXPECT_EQ(-ENOTCONN, ceph_fsetattr(cmount, 0, &st, 0); | |||
There was a problem hiding this comment.
Missing closing ) here. Please do compile this test and check that it works before updating the PR.
There was a problem hiding this comment.
I omitted this step, the follow-up I will pay attention to this problem,
Thank you for reminding.
Signed-off-by: huanwen ren <ren.huanwen@zte.com.cn>
renhwztetecs
force-pushed
the
renhw-wip-add-libcephfs
branch
from
edcf29f
to
1d6a3e4
Compare
|
Ahh, just came into this late. Yeah, I agree that lchmod/lutimes isn't really needed. We have a ceph_setattrx now, which can take an AT_SYMLINK_NOFOLLOW flag. That should allow setting any of those fields on the symlink itself. |
|
@jtlayton thanks |
export ceph_fsetattr() && ceph_lchmod() && ceph_lutime()
Signed-off-by: huanwen ren ren.huanwen@zte.com.cn