libcephfs: introduce libcephfs *at() APIs

[vshankar]

Checklist

• References tracker ticket
• Updates documentation if necessary
• Includes tests for new functionality or reproducer for bug

---

Show available Jenkins commands

• jenkins retest this please
• jenkins test classic perf
• jenkins test crimson perf
• jenkins test signed
• jenkins test make check
• jenkins test make check arm64
• jenkins test submodules
• jenkins test dashboard
• jenkins test api
• jenkins test docs
• jenkins render docs
• jenkins test ceph-volume all
• jenkins test ceph-volume tox

[jtlayton]

Do we really need this? We have the ceph_ll_* apis already, and you can pretty much do the same thing as the *_at apis with it by taking an Inode reference of the parent (either with or without holding open a fd).

[vshankar]

Do we really need this? We have the ceph_ll_* apis already, and you can pretty much do the same thing as the *_at apis with it by taking an Inode reference of the parent (either with or without holding open a fd).

I haven't closely looked at the *_ll APIs, but they provide a low-level interface. I would be a bit weird to use them from an application POV and have to deal with Inode * stuff and the likes. Wouldn't it be beneficial to havelibcephfs users deal with normal file system paths and use *at APIs if needed?

[jtlayton]

It depends on the application. ganesha uses the *_ll APIs just fine. What applications do you expect to use these?

My main worry is that adding so many interfaces to libcephfs will become a maintenance burden. It's also hard to remove interfaces later, once they proliferate out into the field, so we want to make sure that we're only adding things that we think applications need and will use.

[mattbenjamin]

I guess I'm with Jeff. I don't understand the motivation to avoid handles, in general. The "low level" methods were needed to make libcephfs actually useful to protocol translators. It's less clear how important it is for the library to cater to programmers who cannot understand the concept of a handle.

[vshankar]

It depends on the application. ganesha uses the *_ll APIs just fine. What applications do you expect to use these?

cephfs mirror daemon.

[mattbenjamin]

It depends on the application. ganesha uses the *_ll APIs just fine. What applications do you expect to use these?

cephfs mirror daemon.

I would have thought that the cephfs mirror daemon would be able to use ll apis. As Jeff said, the ganesha "fsal" driver has used them since 2011 or something.

Matt

[vshankar]

I would have thought that the cephfs mirror daemon would be able to use ll apis. As Jeff said, the ganesha "fsal" driver has used them since 2011 or something.

It obviously can use *_ll APIs. However, I do not expect other users of this library to switch using low-level interfaces for things like these.

[phlogistonjohn]

In the go-ceph project we have (partial) coverage of the high level apis but none of the low-level apis, so having *at functions could give some pretty good benefits without having to understand all the details that the lowlevel api might bring. The high level apis are also a very close translation of the typical file systems api calls and thus you can often do use the docs for those calls to help understand the libcephfs apis.

I have personally been hesitant to jump into the low level api for go-ceph... as compared to the pretty good doxygen comments for the high level api the low level apis are more sparsely documented (at least I haven't found much on it). While its certainly not insurmountable, there's stuff like ceph_ll_lookup ceph_ll_put ceph_ll_forget ceph_ll_close and ceph_ll_iclose that don't have an immediate analogue and would take more extensive understanding on our part. And even then, even if we wrapped those calls our users would need to understand when and how to use them.

[batrick]

I'm against proliferating the low-level API any further than we already have. It started out as a hack for ceph-fuse without a lot of thought into its appropriateness for other use-cases. I don't want to increase dependencies on it.

The *at syscalls are common and standard. This PR shows it's not a stretch to add it to libcephfs -- the changeset is fairly minimal and mostly an exercise in wiring things up. I like this change overall.

[mattbenjamin]

The "low level" api calls are somewhat similar to VFS in general, and there are extensions for NFS. While the ll calls do look hacky, the motivations for them were valid. Programs like NFS-Ganesha shouldn't be restricted to a POSIX interface when there's no strong motivation for doing that.

[jtlayton]

Fair enough. If you guys feel it's worth it to duplicate this functionality, then so be it. I won't stand in the way of it.

[vshankar]

@batrick https://pulpito.ceph.com/vshankar-2021-04-20_13:13:19-fs-wip-libcephfs-at-calls-20210420-143220-distro-basic-gibba/

[vshankar]

https://pulpito.ceph.com/vshankar-2021-04-23_06:30:56-fs-wip-libcephfs-at-calls-20210423-002524-distro-basic-gibba/

[vshankar]

@batrick fixed and updated

[vshankar]

fixed and updated

[batrick]

jenkins test make check

[vshankar]

jenkins test make check

[tchaikov]

@vshankar @batrick @petrutlucian94 please note this change breaks the windows build.

../src/client/Client.cc: In member function 'int Client::unlinkat(int, const char*, int, const UserPerm&)':
../src/client/Client.cc:7097:20: error: 'AT_REMOVEDIR' was not declared in this scope
 7097 |     return flags & AT_REMOVEDIR ? -CEPHFS_EBUSY : -CEPHFS_EISDIR;
      |                    ^~~~~~~~~~~~

see https://jenkins.ceph.com/job/ceph-dev-new-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=windows,DIST=windows,MACHINE_SIZE=gigantic/54101//consoleFull

[vshankar]

@vshankar @batrick @petrutlucian94 please note this change breaks the windows build.

../src/client/Client.cc: In member function 'int Client::unlinkat(int, const char*, int, const UserPerm&)':
../src/client/Client.cc:7097:20: error: 'AT_REMOVEDIR' was not declared in this scope
 7097 |     return flags & AT_REMOVEDIR ? -CEPHFS_EBUSY : -CEPHFS_EISDIR;
      |                    ^~~~~~~~~~~~

Thanks @tchaikov. We probably need (from include/uapi/linux/fcntl.h):

#define AT_REMOVEDIR            0x200   /* Remove directory instead of
                                           unlinking file.  */

@petrutlucian94 ?

[petrutlucian94]

@vshankar right, I suggest defining it here, next to AT_SYMLINK_NOFOLLOW: https://github.com/ceph/ceph/blob/master/src/include/win32/fs_compat.h#L33

@tchaikov thanks for keeping an eye on the Windows build.

[tchaikov]

@vshankar @petrutlucian94 mind creating a fix?

[vshankar]

@vshankar @petrutlucian94 mind creating a fix?

I'm on it.

[petrutlucian94]

@vshankar @tchaikov I've submitted a PR here: #41064.

Sorry, haven't noticed that this one was already merged.