ibm-s390-linux / s390-tools

Tools for use with the s390 Linux kernel and device drivers
MIT License
63 stars 59 forks source link

Switch to fuse3 #117

Closed ginggs closed 2 years ago

ginggs commented 3 years ago

Fuse 3.0.0 was released in December 2016. The last maintenance release from the 2.9 branch was in January 2019, and users are encourage to transition to the actively developed 3.x branch. https://github.com/libfuse/libfuse/releases/tag/fuse-2.9.9 Closes #116

ginggs commented 3 years ago

I prepared this based on the fuse 3.0 release notes and upstream examples.

Please note, although I've confirmed that s390-tools compiles with these changes, I have no idea how to test the zgetdump, cmsfs-fuse, etc. tools, so this is completely untested.

hoeppnerj commented 3 years ago

Patch looks good from my side. @stefan-haberland @eaibmz can you have a look at zdsfs, zdump, and hsavmcore respectively to ensure the switch to fuse3 doesn't break these tools? I'll try to have a look at the other components. I'll notify Sven about the other components.

eaibmz commented 3 years ago

I'm seeing hsavmcore warnings on fedora 34:

# make -C hsavmcore/
make: Entering directory '/root/s390-tools/hsavmcore'
Makefile:47: "systemd support disabled"
  CC      hsavmcore/overlay.o
overlay.c:168:20: warning: initialization of ‘int (*)(const char *, struct stat *, struct fuse_file_info *)’ from incompatible pointer type ‘int (*)(const char *, struct stat *)’ [-Wincompatible-pointer-types]
  168 |         .getattr = vmcore_fuse_getattr,
      |                    ^~~~~~~~~~~~~~~~~~~
overlay.c:168:20: note: (near initialization for ‘vmcore_fuse_ops.getattr’)
overlay.c:169:20: warning: initialization of ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, off_t,  enum fuse_fill_dir_flags), off_t,  struct fuse_file_info *, enum fuse_readdir_flags)’ {aka ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, long int,  enum fuse_fill_dir_flags), long int,  struct fuse_file_info *, enum fuse_readdir_flags)’} from incompatible pointer type ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, off_t,  enum fuse_fill_dir_flags), off_t,  struct fuse_file_info *)’ {aka ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, long int,  enum fuse_fill_dir_flags), long int,  struct fuse_file_info *)’} [-Wincompatible-pointer-types]
  169 |         .readdir = vmcore_fuse_readdir,
      |                    ^~~~~~~~~~~~~~~~~~~
overlay.c:169:20: note: (near initialization for ‘vmcore_fuse_ops.readdir’)
  CC      hsavmcore/proxy.o
  CC      hsavmcore/swap.o
make[1]: Entering directory '/root/s390-tools/libutil'
  CC      libutil/util_arch.o
  CC      libutil/util_log.o
  CC      libutil/util_opt.o
  CC      libutil/util_path.o
  CC      libutil/util_prg.o
  CC      libutil/util_proc.o
  CC      libutil/util_rec.o
  AR      libutil/libutil.a
make[1]: Leaving directory '/root/s390-tools/libutil'
  LINK    hsavmcore/hsavmcore
make: Leaving directory '/root/s390-tools/hsavmcore'
[root@t83lp49 s390-tools]# make -C hsavmcore/
make: Entering directory '/root/s390-tools/hsavmcore'
Makefile:47: "systemd support disabled"
Makefile:47: "systemd support disabled"
  CC      hsavmcore/cmdline_options.o
  CC      hsavmcore/config.o
  CC      hsavmcore/hsa.o
  CC      hsavmcore/hsa_file.o
  CC      hsavmcore/hsa_mem.o
  CC      hsavmcore/main.o
  CC      hsavmcore/mount.o
  CC      hsavmcore/overlay.o
overlay.c:170:20: warning: initialization of ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, off_t,  enum fuse_fill_dir_flags), off_t,  struct fuse_file_info *, enum fuse_readdir_flags)’ {aka ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, long int,  enum fuse_fill_dir_flags), long int,  struct fuse_file_info *, enum fuse_readdir_flags)’} from incompatible pointer type ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, off_t,  enum fuse_fill_dir_flags), off_t)’ {aka ‘int (*)(const char *, void *, int (*)(void *, const char *, const struct stat *, long int,  enum fuse_fill_dir_flags), long int)’} [-Wincompatible-pointer-types]
  170 |         .readdir = vmcore_fuse_readdir,
      |                    ^~~~~~~~~~~~~~~~~~~
overlay.c:170:20: note: (near initialization for ‘vmcore_fuse_ops.readdir’)
  CC      hsavmcore/proxy.o
  CC      hsavmcore/swap.o
make[1]: Entering directory '/root/s390-tools/libutil'
  CC      libutil/util_arch.o
  CC      libutil/util_log.o
  CC      libutil/util_opt.o
  CC      libutil/util_path.o
  CC      libutil/util_prg.o
  CC      libutil/util_proc.o
  CC      libutil/util_rec.o
  AR      libutil/libutil.a
make[1]: Leaving directory '/root/s390-tools/libutil'
  LINK    hsavmcore/hsavmcore
make: Leaving directory '/root/s390-tools/hsavmcore'

My fix for these warnings:

diff --git a/hsavmcore/overlay.c b/hsavmcore/overlay.c
index 1d9639920c22..a4d59020747e 100644
--- a/hsavmcore/overlay.c
+++ b/hsavmcore/overlay.c
@@ -31,8 +31,11 @@ struct vmcore_overlay {
        bool fuse_debug;
 };

-static int vmcore_fuse_getattr(const char *path, struct stat *stbuf)
+static int vmcore_fuse_getattr(const char *path, struct stat *stbuf,
+                              struct fuse_file_info *fi)
 {
+       (void)fi;
+
        struct vmcore_overlay *overlay = fuse_get_context()->private_data;
        int ret = 0;

@@ -54,10 +57,12 @@ static int vmcore_fuse_getattr(const char *path, struct stat *stbuf)

 static int vmcore_fuse_readdir(const char *path, void *buf,
                               fuse_fill_dir_t filler, off_t offset,
-                              struct fuse_file_info *fi)
+                              struct fuse_file_info *fi,
+                              enum fuse_readdir_flags flags)
 {
        (void)offset;
        (void)fi;
+       (void)flags;

        if (strcmp(path, ROOT_DIR) != 0)
                return -ENOENT;
eaibmz commented 3 years ago

The same warnings are thrown for zdump. My fix:

diff --git a/zdump/zfuse.c b/zdump/zfuse.c
index effcaed3c8b8..dc85870a2d91 100644
--- a/zdump/zfuse.c
+++ b/zdump/zfuse.c
@@ -79,8 +79,11 @@ static void stat_dump_init(void)
 /*
  * FUSE callback: Getattr
  */
-static int zfuse_getattr(const char *path, struct stat *stat)
+static int zfuse_getattr(const char *path, struct stat *stat,
+                        struct fuse_file_info *fi)
 {
+       (void) fi;
+
        if (strcmp(path, "/") == 0) {
                *stat = l.stat_root;
                return 0;
@@ -96,10 +99,12 @@ static int zfuse_getattr(const char *path, struct stat *stat)
  * FUSE callback: Readdir - Return ".", ".." and dump file
  */
 static int zfuse_readdir(const char *path, void *buf, fuse_fill_dir_t filler,
-                        off_t offset, struct fuse_file_info *fi)
+                        off_t offset, struct fuse_file_info *fi,
+                        enum fuse_readdir_flags flags)
 {
        (void) offset;
        (void) fi;
+       (void) flags;

        if (strcmp(path, "/") != 0)
                return -ENOENT;
eaibmz commented 3 years ago

hsavmcore and zgetdump work with FUSE3, verified by me. Apart from warnings, a bunch of READMEs must be fixed within hsavmcore/initramfs because they assume that FUSE2 is being used.

eaibmz commented 3 years ago

Forgot to mention this: In FUSE 3, nonempty is always true, and has been removed. By default it is false on FUSE 2. Therefore, please remove nonempty option when mounting a file system in zfuse_mount_dump().

Otherwise i get the following error:

# ./s390-tools/zdump/zgetdump -m /dev/mapper/mpathd1 /mnt
fuse: unknown option(s): `-o nonempty'
hoeppnerj commented 2 years ago

@ginggs Do you plan to incorporate the changes suggested by @eaibmz and push a new version?

ginggs commented 2 years ago

@ginggs Do you plan to incorporate the changes suggested by @eaibmz and push a new version?

Sure, I will do that.

ginggs commented 2 years ago

I've included @eaibmz 's changes to fix the warnings in hsavmcore and zdump, and then found similar warnings in cmsfs-fuse, hmcdrvfs and zdsfs. I'll look at nonempty soon.

eaibmz commented 2 years ago

Please also fix fuse-devel mentions in README files:

git grep fuse-devel
README.md:  the fuse-devel package is required.
hsavmcore/initramfs/fedora-rhel/README.md:sudo dnf install -y fuse fuse-devel systemd-devel
hsavmcore/initramfs/sles/README.md:sudo zypper install -y fuse fuse-devel systemd-devel

Otherwise, looks OK to me with regard to zdump and hsavmcore.

And please clean up your pull request by rebasing it onto the current master and getting rid of merge commits. It is very hard to review otherwise when commits are merged back and forth.

And i have a general question to s390-tools maintainers. Do we want to drop FUSE2 support completely or support both versions, e.g. switching between different version via a preprocessor define ?

Regards Alex

hoeppnerj commented 2 years ago

And i have a general question to s390-tools maintainers. Do we want to drop FUSE2 support completely or support both versions, e.g. switching between different version via a preprocessor define ?

I don't see any benefits in supporting the older version. Switching upstream (and newer distros) to FUSE3 is completely fine. Not worth the extra effort.

ginggs commented 2 years ago

Thank you for your patience!

To be clear, I should rebase on master, flesh out the commit messages and then force-push on to this branch?

hoeppnerj commented 2 years ago

Thank you for your patience!

To be clear, I should rebase on master, flesh out the commit messages and then force-push on to this branch?

Yes, please :)

ginggs commented 2 years ago

Will do, thanks.

hoeppnerj commented 2 years ago

@ginggs As I'm about to prepare another release, any chance for an update here within the next couple of days?

ginggs commented 2 years ago

@hoeppnerj thanks for the ping. I'll make some time to do it.

ginggs commented 2 years ago

@hoeppnerj I'm getting to this now.

hoeppnerj commented 2 years ago

PR pulled and will be pushed upstream later this day. Thanks for your contribution!