[DTrace-devel] [PATCH 3/3] daemon: support libfuse 2

[Nick Alcock]

On 8 Nov 2022, Kris Van Hees said:

> On Mon, Nov 07, 2022 at 10:19:30PM +0000, Nick Alcock via DTrace-devel wrote:
>> So it turns out that some people want to use libfuse 2 still, even
>> though it's been obsoleted by libfuse 3 for nearly a decade.  This turns
>> out to be not terribly difficult!
>> 
>> There are two major changess that impact us (ignoring the options-
>> parsing thing fixed in the last commit):
>> 
>>  - libfuse 2 has a 'channel' abstraction in which the fd to the kernel
>>    is nested.  libfuse 3 dropped it because it turned out there was only
>>    ever one of them so the abstraction was redundant, but that makes it
>>    easy for us to pick the fd out: we can grab the fuse_session_next_chan()
>>    without ever worrying that we might have to call that function more
>>    than once or deal with more than one channel.
>> 
>>    This changes the main read loop() enough that we can just implement
>>    it twice.
>> 
>>  - that's good because of another annoying difference: OL carries a
>>    NUMA-aware API which actually changes the public API of libfuse's
>>    fuse_session_receive_buf function (I guess the assumption was that
>>    nobody else would implement their own event loop, which is... a
>>    heroic assumption given that a quick search shows a bunch of users
>>    including minor ones like QEMU).  This is hard to check for because
>
> I would leave out the (...) portion because it is merely commentary (and
> conjecture) that does not add anything that is relevant to this patch.

Yeah. It was mostly there to make sure you were reading the patch :P

>>    the extra functions you need to call to actually turn this behaviour
>>    on were never added to the symbol version file, so you can't link
>>    with them.  So we detect these functions using the macros just added
>>    to do straight compilation checking, with no linking, and use their
>>    presence as a clue that fuse_session_receive_buf has the NUMA-aware
>>    API change.  (We don't use the NUMA stuff: it's only useful if you're
>>    using a multithreaded event loop, which we're not.)
>> 
>> We currently handle systems with both libfuse 2 and 3 by checking that
>> libfuse 3 is available, and if not, falling back to libfuse 2 (and if
>> that isn't present either you get a link failure).  Thus libfuse 3 is
>> preferred if both are present.
>
> That is an issue because fuse3 being present at compile time (especially when
> building packages) does not mean that it will be available at runtime.  See
> more on that below...
> 
> What we should do here is making it possible to build using libfuse 2 by
> passing something to the make command (e.g. -DUSE_FUSE2 or whatever) so that

make fuse2=yes would seem like a more DTrace-build-systemy way of saying
it :)

> we can elect to have DTrace built using libfuse 2 even if libfuse 3 is
> available. 

Really? Jenkins installs into its neat containers not only random
packages but random *devel* packages? The only way I could see that
happening is if some other devel package we use pulls it in, and in that
case dtrace will probably end up depending on *both* fuses (one
indirectly), which woudl be disastrous.

Worse this theoretically applies to *every* package we depend on, many
of which have been through multiple ABI transitions. Maybe we only care
about this one because it's already bitten us?

>> +#if HAVE_FUSE3

(I'm renaming this, and associated stuff, to HAVE_LIBFUSE3. FUSE is a
kernel component and has not bumped any sort of version at all. Sorry, a
late decision I failed to propagate through the codebase.)

>> +static void
>> +old_fuse_init(struct fuse_session *cuse_session) {}
>
> Since you use conditionals based on HAVE_FUSE2 below already anyway, why not
> just drop this and in setup_helper_device() place the call to old_fuse_init()
> under #ifndef HAVE_FUSE3?  And rename that function (see below)...

Just something I learned from GNU: whenever possible avoid doing that
sort of thing, it's *ugly*. (The only reason I did it with
HAVE_FUSE_NUMA inside libfuse 2's loop() is that I consider all of that
obsolete code even though I only just wrote it, and eventually it will
go away and take its ugliness with it.)

>> +
>> +static int
>> +session_fd(struct fuse_session *cuse_session)
>> +{
>> +	return fuse_session_fd(cuse_session);
>> +}
>> +#else /* FUSE 2 */
>> +static struct fuse_chan *cuse_chan;
>> +
>> +static void
>> +old_fuse_init(struct fuse_session *cuse_session)
>
> init_cuse_chan() might be a better name.

I started out with that, but honestly if we end up needing more horrible
initialization for old FUSE it's going in here too and naming it this
makes it more obvious that it's going away :) but I'm on the fence and
if you prefer that name, I'll flip it.

>> +	old_fuse_init(cs);
>> +
>
> #ifndef HAVE_FUSE3
> 	init_cuse_chan();
> #endif

Ack to the middle, trying to keep #ifdefs out of non-compatibility
functions if possible for the surrounding ifdefs.

>> +Requires:     fuse
>> +BuildRequires: fuse-devel
>> +%else
>> +Requires:     fuse3 >= 3.2.0
>> +BuildRequires: fuse3-devel >= 3.2.0
>> +%endif
>
> This will pose a problem if fuse3-devel is installed on the build system,
> because then DTrace will be built against it, even if the dist is .el7, and
> that will be an issue at runtime.

How could that ever happen?

> What we really need is a mechanism to elect building using libfuse2 regardless
> of whether libfuse 3 is installed or not on the build system.  Because the real
> issue is at runtime, so we need to be able to build a dtrace dtprobed most
> specifically) that is guaranteed to build using libfuse 2.

There is no issue at runtime, It runs fine. The issue is at *install*
time, and surely if people are building packages that depend on
disabled-by-default channels *and* then shipping them, they would expect
those channels to be enabled by the people they ship them to?

I mean yes I'm going to add it anyway because we have more than enough
experience of this sort of thing getting messed up, but dammit (yet
again) we shouldn't have to!

libfuse 3 remains the default though :)

-- 
NULL && (void)