On Tue, 2016-09-27 at 15:13 +0000, Sage Weil wrote: > On Tue, 27 Sep 2016, Jeff Layton wrote: > > (tl;dr: we need to make some changes to the libcephfs API to allow > > proper credential handling. What's the right approach to take?) > > > > Greg Farnum has posted a pull request to rework how credentials are > > handled in cephfs: > > > > https://github.com/ceph/ceph/pull/11218 > > > > ...the patch pile is rather large, but the upshot is basically to > > change the code not to carve out '-1' in the uid and gid ranges, and to > > allow callers to pass down lists of supplementary groups. This is > > mainly needed for nfs-ganesha, but it also allows ceph-fuse to delegate > > permissions checks to the MDS. > > > > That PR adds most of the low-level plumbing that's needed, but we still > > need to expose this capability to applications via libcephfs. Many > > libcephfs calls either take "int uid, int gid" or don't allow the > > caller to send down creds at all. > > > > I think what we'll want to do is allow applications to create a new > > credential object and pass that down to the library in some fashion. > > The question is: what's the best way to handle those API changes? > > > > We have several options here: > > > > 1) make a clean break with the old API: just change the arguments to > > the existing functions, bump the library's major version and set the > > "age" field in it to 0. If you built against old headers, it'll fail to > > load the library at runtime. This is clearly the simplest option, but > > requires everyone to rebuild applications that were built against the > > old headers. > > I think this is the way to go. We'd bump the SONAME (libcephfs2!). The > packaging infrastructure is such that you can have the old version of the > package (libcephfs1) installed at the same time as libcephfs2 (and > -devel), so stuff that's linked to the old API can still work and > coexist with stuff linked to the new one. The only restriction (on debian > at least; I assume in rpm-land it is the same) is that you can only have > one version of the -dev package (headers) installed at a time, so you can > only compile new code against one version at a time. > Ok, so basically we just rely on the linker picking the right lib at build time. That would work too. That said, the RPMs are currently named libcephfs1 and libcephfs1-devel. I guess then we'd end up with a libcephfs2-devel. I think we could add a "conflicts" directive in the specfile for it to make sure we have a conflict with libcephfs1-devel. I can live with that approach if that's the consensus. It sucks for anyone who happens to have apps built against the old version, but that's life... > IIRC with the SONAME stuff you can specify the oldest compatible > version. We can take the lazy route and skip #2 below and make that 2 as > well, which would mean installing libcephfs1 and libcephfs2 concurrently > for compatibility. > You can do this. In libtool versioning parlance what you do is bump "current" and "age", and set revision to 0: https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html That was what I was figuring we'd do if we took option #2 below. If we go with option #1 though, then we just bump "current" and set the other two fields to 0. > It seems like the key downside here is that any project building against > libcephfs that isn't updated will not be able to use newer versions of the > client until they move to the new API... > Exactly. AIUI though, there aren't really many apps in the field that use libcephfs. The only two major ones I know of that aren't part of ceph itself are nfs-ganesha and samba. I intend to fix both of them to use the new API when it's available. Are there other consumers of this library that I should be aware of? > > > > > > > 2) create a "parallel" API that takes pointers to UserPerm objects, and > > add new new calls for them. The old API then becomes a wrapper around > > the new code. This would work, but it's more work -- we'd need to add a > > whole swath of new calls that take this pointer. This has the advantage > > of allowing existing programs to continue working through a lib > > upgrade. For this, we'd bump the major lib version, and "age" field to > > indicate that the library has a new API, but is backward compatible > > with the old one. If we want to go this route, how should the new > > functions be named? > > > > 3) get tricky with global and thread local variables. Add calls to > > allow users to set libcephfs creds at the process and thread level. > > When the API is called, we'll check to see if creds have been installed > > in either spot and use those if they're present. If they're not, then > > we'll grab them from the current task (much like we do today). This is > > somewhat analogous to using setuid/setgid/setgroups before making a > > syscall. It's less intuitive than option #2 and requires callers to > > manage their creds carefully, but means that we don't need to explode > > out the API _and_ preserves the ability to run programs built against > > the old headers. > > > > It's a fair bit of work any way we approach it, so I want to be clear > > on a direction before I dive in too deeply. > > > > Thoughts? > > -- > > Jeff Layton <jlayton@xxxxxxxxxx> > > -- > > 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 > > > > -- Jeff Layton <jlayton@xxxxxxxxxx> -- 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