Re: [PATCH] grub-mount: Support libfuse 3

[Daniel Kiper]

On Wed, Jan 12, 2022 at 03:09:07PM +0100, Fabian Vogt wrote:
> libfuse 3.0.0 got released in 2016, with some API changes compared to 2.x.
> This commit introduces support for 3.x while keeping it compatible with 2.6
> as a fallback still.
>
> To detect fuse3, switch configure over to use pkg-config, which is simpler yet
> more reliable than looking for library and header manually. Also set
> FUSE_USE_VERSION that way, as it depends on the used libfuse version.
>
> Now that the CFLAGS are read from pkg-config, use just <fuse.h>, which works
> with 2.x as well as 3.x and is recommended by libfuse upstream.
>
> One behaviour change of libfuse3 is that FUSE_ATOMIC_O_TRUNC is set by 
> default,
> which means that open with O_TRUNC is passed as-is instead of calling the
> truncate operation. With libfuse2, truncate failed with -ENOSYS and that was
> returned to the application. To make O_TRUNC fail with libfuse3, return -EROFS
> explicitly if writing was requested.
>
> Signed-off-by: Fabian Vogt <fvogt@suse.de>
> ---
> I tested building with libfuse 2.9.9 as well as 3.10.5, both successful.
> It appears to work properly when mounting a btrfs partition or ext4 image in
> some quick manual testing.
>
>  Makefile.util.def |  4 +++-
>  configure.ac      | 16 +++++-----------
>  util/grub-mount.c | 24 +++++++++++++++++++++---
>  3 files changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/Makefile.util.def b/Makefile.util.def
> index f8b356cc1..e92c1f346 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -309,11 +309,13 @@ program = {
>    common = grub-core/disk/host.c;
>    common = grub-core/osdep/init.c;
>
> +  cflags = '$(FUSE_CFLAGS)';
> +
>    ldadd = libgrubmods.a;
>    ldadd = libgrubgcry.a;
>    ldadd = libgrubkern.a;
>    ldadd = grub-core/lib/gnulib/libgnu.a;
> -  ldadd = '$(LIBINTL) $(LIBDEVMAPPER) $(LIBZFS) $(LIBNVPAIR) $(LIBGEOM) 
> -lfuse';
> +  ldadd = '$(LIBINTL) $(LIBDEVMAPPER) $(LIBZFS) $(LIBNVPAIR) $(LIBGEOM) 
> $(FUSE_LIBS)';
>    condition = COND_GRUB_MOUNT;
>  };
>
> diff --git a/configure.ac b/configure.ac
> index 4f649edaf..1d40f9560 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1787,17 +1787,11 @@ if test x"$enable_grub_mount" = xno ; then
>  fi
>
>  if test x"$grub_mount_excuse" = x ; then
> -  AC_CHECK_LIB([fuse], [fuse_main_real], [],
> -               [grub_mount_excuse="need FUSE library"])
> -fi
> -
> -if test x"$grub_mount_excuse" = x ; then
> -  # Check for fuse headers.
> -  SAVED_CPPFLAGS="$CPPFLAGS"
> -  CPPFLAGS="$CPPFLAGS -DFUSE_USE_VERSION=26"
> -  AC_CHECK_HEADERS([fuse/fuse.h], [],
> -     [grub_mount_excuse=["need FUSE headers"]])
> -  CPPFLAGS="$SAVED_CPPFLAGS"
> +  PKG_CHECK_MODULES([FUSE], [fuse3], [FUSE_CFLAGS="$FUSE_CFLAGS 
> -DFUSE_USE_VERSION=32"], [
> +    PKG_CHECK_MODULES([FUSE], [fuse], [FUSE_CFLAGS="$FUSE_CFLAGS 
> -DFUSE_USE_VERSION=26"], [
> +      grub_mount_excuse="need fuse or fuse3 libraries"
> +    ])
> +  ])
>  fi
>
>  if test x"$enable_grub_mount" = xyes && test x"$grub_mount_excuse" != x ; 
> then
> diff --git a/util/grub-mount.c b/util/grub-mount.c
> index d7be2a427..f983dd765 100644
> --- a/util/grub-mount.c
> +++ b/util/grub-mount.c
> @@ -16,7 +16,6 @@
>   *  You should have received a copy of the GNU General Public License
>   *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>   */
> -#define FUSE_USE_VERSION 26
>  #include <config.h>
>  #include <grub/types.h>
>  #include <grub/emu/misc.h>
> @@ -34,7 +33,7 @@
>  #include <grub/command.h>
>  #include <grub/zfs/zfs.h>
>  #include <grub/i18n.h>
> -#include <fuse/fuse.h>
> +#include <fuse.h>
>
>  #include <stdio.h>
>  #include <unistd.h>
> @@ -146,8 +145,13 @@ fuse_getattr_find_file (const char *cur_filename,
>    return 0;
>  }
>
> +#if FUSE_USE_VERSION < 30
>  static int
>  fuse_getattr (const char *path, struct stat *st)
> +#else
> +static int
> +fuse_getattr (const char *path, struct stat *st, struct fuse_file_info *fi)

struct fuse_file_info *fi __attribute__ ((unused))) ?

> +#endif
>  {
>    struct fuse_getattr_ctx ctx;
>    char *pathname, *path2;
> @@ -241,8 +245,11 @@ static grub_file_t files[65536];
>  static int first_fd = 1;
>
>  static int
> -fuse_open (const char *path, struct fuse_file_info *fi __attribute__ 
> ((unused)))
> +fuse_open (const char *path, struct fuse_file_info *fi)
>  {
> +  if ((fi->flags & O_ACCMODE) != O_RDONLY)
> +    return -EROFS;
> +
>    grub_file_t file;
>    file = grub_file_open (path, GRUB_FILE_TYPE_MOUNT);
>    if (! file)
> @@ -330,13 +337,24 @@ fuse_readdir_call_fill (const char *filename,
>    st.st_blocks = (st.st_size + 511) >> 9;
>    st.st_atime = st.st_mtime = st.st_ctime
>      = info->mtimeset ? info->mtime : 0;
> +#if FUSE_USE_VERSION < 30
>    ctx->fill (ctx->buf, filename, &st, 0);
> +#else
> +  ctx->fill (ctx->buf, filename, &st, 0, 0);
> +#endif
>    return 0;
>  }
>
> +#if FUSE_USE_VERSION < 30
>  static int
>  fuse_readdir (const char *path, void *buf,
>             fuse_fill_dir_t fill, off_t off, struct fuse_file_info *fi)
> +#else
> +static int
> +fuse_readdir (const char *path, void *buf,
> +           fuse_fill_dir_t fill, off_t off, struct fuse_file_info *fi,
> +           enum fuse_readdir_flags flags)

enum fuse_readdir_flags flags __attribute__ ((unused))) ?

Otherwise LGTM...

Daniel