Skip to content

Fix MS_LAZYTIME not defined on uclibc#753

Merged
Nikratio merged 1 commit intolibfuse:masterfrom
Benetti-Engineering:fix/uclibc-MS_LAZYTIME
Apr 6, 2023
Merged

Fix MS_LAZYTIME not defined on uclibc#753
Nikratio merged 1 commit intolibfuse:masterfrom
Benetti-Engineering:fix/uclibc-MS_LAZYTIME

Conversation

@giuliobenetti
Copy link
Copy Markdown
Contributor

No description provided.

giuliobenetti added a commit to Benetti-Engineering/buildroot that referenced this pull request Mar 27, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

/* uclibc doesn't define MS_LAZYTIME */
#ifndef MS_LAZYTIME
#define MS_LAZYTIME (1<<25)
Copy link
Copy Markdown
Contributor

@Nikratio Nikratio Mar 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this value come from? (please document in the comment)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Nikratio,

it comes from Linux header mount.h:
https://elixir.bootlin.com/linux/v6.2.8/source/include/uapi/linux/mount.h#L39
and it never changed value throug various versions since it's been added with Linux 4.0:
https://elixir.bootlin.com/linux/v4.0.9/source/include/uapi/linux/fs.h#L93

Unforunately uclibc doesn't add it to <sys/mount.h>. The other chance to fix the bug is to include Linux's:
include/uapi/linux/mount.h
but before Linux 5.0 that define is contained in:
https://elixir.bootlin.com/linux/v4.20.17/source/include/uapi/linux/fs.h#L133
So I don't know if that's a good idea. Or if we do that I can check against Linux version and include the 2 different headers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing wrong with hardcoding the value, I just want you to add the source to the (C) comment in the file :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I don't know why I've left a commit so bad written :-/

@bsbernd
Copy link
Copy Markdown
Collaborator

bsbernd commented Mar 27, 2023

This patch would miss util/fusermount.c.

Can you try out this?

diff --git a/lib/mount.c b/lib/mount.c
index 1f1ee860..b7965d6c 100644
--- a/lib/mount.c
+++ b/lib/mount.c
@@ -27,6 +27,9 @@
 #include <sys/wait.h>
 #include <sys/mount.h>
 
+#include <linux/fs.h>
+#include <linux/mount.h>
+
 #ifdef __NetBSD__
 #include <perfuse.h>
 
@@ -47,9 +50,11 @@
 #define fork() vfork()
 #endif
 
+#if 0
 #ifndef MS_DIRSYNC
 #define MS_DIRSYNC 128
 #endif
+#endif
 
 enum {
        KEY_KERN_FLAG,
diff --git a/util/fusermount.c b/util/fusermount.c
index 6e72f0d3..41327e58 100644
--- a/util/fusermount.c
+++ b/util/fusermount.c
@@ -32,13 +32,18 @@
 #include <stdbool.h>
 #include <sys/vfs.h>
 
+#include <linux/fs.h>
+#include <linux/mount.h>
+
 #define FUSE_COMMFD_ENV                "_FUSE_COMMFD"
 
 #define FUSE_DEV "/dev/fuse"
 
+#if 0
 #ifndef MS_DIRSYNC
 #define MS_DIRSYNC 128
 #endif
+
 #ifndef MS_REC
 #define MS_REC 16384
 #endif
@@ -46,6 +51,8 @@
 #define MS_PRIVATE (1<<18)
 #endif
 
+#endif
+
 #ifndef UMOUNT_DETACH
 #define UMOUNT_DETACH  0x00000002      /* Just detach from the tree */
 #endif

I don't see a problem to include multiple header files - nothing bad should happen, except there would be a bug in the headers.

And I have the feeling we need a test for ulibc compilation.

@giuliobenetti
Copy link
Copy Markdown
Contributor Author

giuliobenetti commented Mar 27, 2023

This patch would miss util/fusermount.c.

Can you try out this?

diff --git a/lib/mount.c b/lib/mount.c
index 1f1ee860..b7965d6c 100644
--- a/lib/mount.c
+++ b/lib/mount.c
@@ -27,6 +27,9 @@
 #include <sys/wait.h>
 #include <sys/mount.h>
 
+#include <linux/fs.h>
+#include <linux/mount.h>
+
 #ifdef __NetBSD__
 #include <perfuse.h>
 
@@ -47,9 +50,11 @@
 #define fork() vfork()
 #endif
 
+#if 0
 #ifndef MS_DIRSYNC
 #define MS_DIRSYNC 128
 #endif
+#endif
 
 enum {
        KEY_KERN_FLAG,
diff --git a/util/fusermount.c b/util/fusermount.c
index 6e72f0d3..41327e58 100644
--- a/util/fusermount.c
+++ b/util/fusermount.c
@@ -32,13 +32,18 @@
 #include <stdbool.h>
 #include <sys/vfs.h>
 
+#include <linux/fs.h>
+#include <linux/mount.h>
+
 #define FUSE_COMMFD_ENV                "_FUSE_COMMFD"
 
 #define FUSE_DEV "/dev/fuse"
 
+#if 0
 #ifndef MS_DIRSYNC
 #define MS_DIRSYNC 128
 #endif
+
 #ifndef MS_REC
 #define MS_REC 16384
 #endif
@@ -46,6 +51,8 @@
 #define MS_PRIVATE (1<<18)
 #endif
 
+#endif
+
 #ifndef UMOUNT_DETACH
 #define UMOUNT_DETACH  0x00000002      /* Just detach from the tree */
 #endif

I don't see a problem to include multiple header files - nothing bad should happen, except there would be a bug in the headers.

Unfortunately <linux/mount.h> is not always present because we don't always have Linux Headers installed:

/home/giuliobenetti/git/upstream/test-libfuse3/bootlin-armv5-uclibc/host/bin/arm-linux-gcc -Iutil/fusermount3.p -Iutil -I../util -Iinclude -I../include -Ilib -I../lib -I. -I.. -fdiagnostics-color=always -Wall -Winvalid-pch -Wextra -O3 -D_REENTRANT -DHAVE_LIBFUSE_PRIVATE_CONFIG_H -Wno-sign-compare -Wstrict-prototypes -Wmissing-declarations -Wwrite-strings -fno-strict-aliasing -Wno-unused-result -DHAVE_SYMVER_ATTRIBUTE -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g0 '-DFUSE_CONF="/etc/fuse.conf"' -MD -MQ util/fusermount3.p/fusermount.c.o -MF util/fusermount3.p/fusermount.c.o.d -o util/fusermount3.p/fusermount.c.o -c ../util/fusermount.c
../util/fusermount.c:36:10: fatal error: linux/mount.h: No such file or directory
   36 | #include <linux/mount.h>
      |          ^~~~~~~~~~~~~~~
compilation terminated.

I understand your goal to use includes instead of local redefinition of macros, but while dealing with kernel includes
can change between versions the same way as defines, so I don't what's better :-/
IMHO I'd leave it as it is.
Patch(without the comment) for libfuse3.14.1 is already pending on Buildroot's patchwork:
https://patchwork.ozlabs.org/project/buildroot/patch/20230327192628.566995-1-giulio.benetti@benettiengineering.com/

And I have the feeling we need a test for ulibc compilation.

It would be great!

@bsbernd
Copy link
Copy Markdown
Collaborator

bsbernd commented Mar 27, 2023

Ok, yeah, if linux/mount.h is not always available it makes it complicated.

Copy link
Copy Markdown
Collaborator

@bsbernd bsbernd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also needs to be defined in util/fusermount.c. Maybe we should add a mount_compat.h?

@giuliobenetti
Copy link
Copy Markdown
Contributor Author

Also needs to be defined in util/fusermount.c. Maybe we should add a mount_compat.h?

Good idea, indeed I was looking exactly for something like that to touch only one file instead of 2.
I didn't know how to call that file and yes, mount_compat.h is the name :-)

giuliobenetti added a commit to Benetti-Engineering/buildroot that referenced this pull request Mar 28, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
giuliobenetti added a commit to Benetti-Engineering/buildroot that referenced this pull request Mar 29, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
giuliobenetti added a commit to Benetti-Engineering/buildroot that referenced this pull request Apr 3, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
giuliobenetti added a commit to Benetti-Engineering/buildroot that referenced this pull request Apr 3, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
@Nikratio Nikratio marked this pull request as draft April 4, 2023 15:02
@Nikratio
Copy link
Copy Markdown
Contributor

Nikratio commented Apr 4, 2023

Tagging this as "draft" per the comments above. Please let me know when this is ready for review.

as well as <sys/mount.h> inclusion to new fuse_mount_compat.h file.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
@giuliobenetti giuliobenetti force-pushed the fix/uclibc-MS_LAZYTIME branch from 6ead4f3 to d259b26 Compare April 5, 2023 11:26
@giuliobenetti
Copy link
Copy Markdown
Contributor Author

Tagging this as "draft" per the comments above. Please let me know when this is ready for review.

I've updated the patch. Hope it looks good to you. I start the building with Buildroot, so please wait because I still have to try to build!

@giuliobenetti
Copy link
Copy Markdown
Contributor Author

@Nikratio it built successfully all 44 combinations in Buildroot.
Best regards

giuliobenetti added a commit to Benetti-Engineering/buildroot that referenced this pull request Apr 5, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
arnout pushed a commit to buildroot/buildroot that referenced this pull request Apr 5, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
@Nikratio Nikratio marked this pull request as ready for review April 6, 2023 11:37
@Nikratio Nikratio merged commit c60a90b into libfuse:master Apr 6, 2023
citral23 pushed a commit to citral23/buildroot that referenced this pull request Sep 18, 2023
Add local patch pending upstream[0] to fix build failure with uclibc.

[0]: libfuse/libfuse#753

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants