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

Kris Van Hees kris.van.hees at oracle.com
Tue Nov 8 05:41:55 UTC 2022


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.

>    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
we can elect to have DTrace built using libfuse 2 even if libfuse 3 is
available. 

> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>  Makeconfig          |  2 ++
>  dtprobed/Build      | 11 +++++--
>  dtprobed/dtprobed.c | 78 ++++++++++++++++++++++++++++++++++++++++++++-
>  dtrace.spec         | 11 +++++--
>  4 files changed, 97 insertions(+), 5 deletions(-)
> 
> diff --git a/Makeconfig b/Makeconfig
> index 15614384fd94..52071e29935b 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -97,4 +97,6 @@ $(eval $(call check-symbol-rule,STRRSTR,strrstr,c))
>  $(eval $(call check-symbol-rule,WAITFD,waitfd,c))
>  $(eval $(call check-symbol-rule,LIBSYSTEMD,sd_notify,systemd))
>  $(eval $(call check-symbol-rule,FUSE_LOG,fuse_set_log_func,fuse3))
> +$(eval $(call check-symbol-rule,FUSE3,fuse_session_receive_buf,fuse3))
> +$(eval $(call check-header-rule,FUSE_NUMA,fuse_set_numa,fuse/fuse_lowlevel))
>  $(eval $(call check-header-symbol-rule,CLOSE_RANGE,close_range(3,~0U,0),c,unistd))
> diff --git a/dtprobed/Build b/dtprobed/Build
> index 6775267ec77f..6041cd9fc0f3 100644
> --- a/dtprobed/Build
> +++ b/dtprobed/Build
> @@ -5,11 +5,18 @@
>  
>  CMDS += dtprobed
>  
> +# Support FUSE 2 as well as FUSE 3.
> +ifdef HAVE_FUSE3
> +LIBFUSE := fuse3
> +else
> +LIBFUSE := fuse
> +endif
> +
>  dtprobed_DIR := $(current-dir)
>  dtprobed_TARGET = dtprobed
>  dtprobed_CPPFLAGS := -I. -Idtprobed -Ilibproc -Ilibcommon -Ilibport
> -dtprobed_CFLAGS := $(shell pkg-config --cflags fuse3)
> -dtprobed_LIBS := -lcommon -lproc -lcommon -lport -lelf $(shell pkg-config --libs fuse3)
> +dtprobed_CFLAGS := $(shell pkg-config --cflags $(LIBFUSE))
> +dtprobed_LIBS := -lcommon -lproc -lcommon -lport -lelf $(shell pkg-config --libs $(LIBFUSE))
>  dtprobed_DEPS := libproc.a libcommon.a libport.a
>  dtprobed_SOURCES := dtprobed.c
>  dtprobed_LIBSOURCES := libproc libcommon
> diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
> index b76bc213e650..77d66772209c 100644
> --- a/dtprobed/dtprobed.c
> +++ b/dtprobed/dtprobed.c
> @@ -93,6 +93,31 @@ dt_debug_printf(const char *subsys, const char *fmt, va_list ap)
>  	}
>  }
>  
> +#if HAVE_FUSE3
> +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)...

> +
> +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.

> +{
> +	cuse_chan = fuse_session_next_chan(cuse_session, NULL);
> +}
> +
> +static int
> +session_fd(struct fuse_session *cuse_session)
> +{
> +	return fuse_chan_fd(cuse_chan);
> +}
> +#endif
> +
>  /*
>   * States for the ioctl processing loop, which gets repeatedly called due to the
>   * request/reply nature of unrestricted FUSE ioctls.
> @@ -141,6 +166,8 @@ setup_helper_device(int argc, char **argv, char *devname, dtprobed_userdata_t *u
>  		return NULL;
>  	}
>  
> +	old_fuse_init(cs);
> +

#ifndef HAVE_FUSE3
	init_cuse_chan();
#endif

>  	if (multithreaded) {
>  		fprintf(stderr, "CUSE thinks dtprobed is multithreaded!\n");
>  		fprintf(stderr, "This should never happen.\n");
> @@ -202,7 +229,7 @@ dof_parser_start(int sync_fd)
>  		 * Sandboxed parser child.  Close unwanted fds and nail into
>  		 * seccomp jail.
>  		 */
> -		close(fuse_session_fd(cuse_session));
> +		close(session_fd(cuse_session));
>  		close(parser_in_pipe[1]);
>  		close(parser_out_pipe[0]);
>  		if (!foreground)
> @@ -507,6 +534,7 @@ err:
>  	return -1;
>  }	
>  
> +#if HAVE_FUSE3
>  static int
>  loop(void)
>  {
> @@ -538,6 +566,54 @@ loop(void)
>  	fuse_session_reset(cuse_session);
>  	return ret < 0 ? -1 : 0;
>  }
> +#else /* FUSE 2 */
> +static int
> +loop(void)
> +{
> +	struct pollfd fds[1];
> +	void *buf;
> +	size_t bufsize;
> +	int ret = 0;
> +
> +	bufsize = fuse_chan_bufsize(cuse_chan);
> +	buf = malloc(bufsize);
> +	if (!buf) {
> +		fprintf(stderr, "Cannot allocate memory for FUSE buffer\n");
> +		return -1;
> +	}
> +
> +	fds[0].fd = fuse_chan_fd(cuse_chan);
> +	fds[0].events = POLLIN;
> +
> +	while (!fuse_session_exited(cuse_session)) {
> +		struct fuse_buf fbuf = { .mem = buf, .size = bufsize };
> +		struct fuse_chan *tmpch = cuse_chan;
> +
> +		if ((ret = poll(fds, 1, -1)) < 0)
> +			break;
> +
> +		if (fds[0].revents != 0) {
> +			if ((ret = fuse_session_receive_buf(cuse_session,
> +							    &fbuf, &tmpch)) <= 0) {
> +				if (ret == -EINTR)
> +					continue;
> +
> +				break;
> +			}
> +
> +#ifdef HAVE_FUSE_NUMA
> +			fuse_session_process_buf(cuse_session, &fbuf, tmpch, 0);
> +#else
> +			fuse_session_process_buf(cuse_session, &fbuf, tmpch);
> +#endif
> +		}
> +	}
> +
> +	free(buf);
> +	fuse_session_reset(cuse_session);
> +	return ret < 0 ? -1 : 0;
> +}
> +#endif
>  
>  static void
>  usage(void)
> diff --git a/dtrace.spec b/dtrace.spec
> index 0d87591dddc6..62515acd2700 100644
> --- a/dtrace.spec
> +++ b/dtrace.spec
> @@ -55,9 +55,16 @@ BuildRequires: rpm
>  Name:         dtrace
>  License:      Universal Permissive License (UPL), Version 1.0
>  Group:        Development/Tools
> -Requires:     cpp elfutils-libelf zlib libpcap fuse3 >= 3.2.0
> -BuildRequires: glibc-headers bison flex zlib-devel elfutils-libelf-devel fuse3-devel >= 3.2.0 systemd systemd-devel
> +Requires:     cpp elfutils-libelf zlib libpcap
> +BuildRequires: glibc-headers bison flex zlib-devel elfutils-libelf-devel systemd systemd-devel
>  BuildRequires: glibc-static %{glibc32} wireshark libpcap-devel valgrind-devel
> +%if "%{?dist}" == ".el7"
> +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.

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.

>  %{?systemd_requires}
>  BuildRequires: kernel%{variant}-devel = %{build_kernel}
>  %if "%{?dist}" == ".el8"
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel



More information about the DTrace-devel mailing list