| Message ID | 1682400834-16985-1-git-send-email-xuyang2018.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | src/detached_mounts_propagation: Fix compile error | expand |
On Tue, Apr 25, 2023 at 01:33:54PM +0800, Yang Xu wrote: > Newer glibc such as glibc 2.36 also defines 'struct mount_attr' > in addition to <linux/mount.h>(we also include this kernel header when using > global.h). > > Usually we should use glibc header instead of kernel header. > But now mount.h is a special case because both new glibc header and > kernel header all define "struct mount_attr'. They also define MS* > macro. > > Since we have some syscall wrapper in vfs/missing.h, we can use > <linux.mount.h> directly instead of <sys/mount.h>. > > In fact, newer glibc(2.37-1)[1] has sloved conflict problem between > <sys/mount.h> and <linux/mount.h>. In the future(maybe ten years), > we can remove this kernel header and use glibc header. > > [1]https://sourceware.org/git/?p=glibc.git;a=commit;h=774058d72942249f71d74e7f2b639f77184160a6 > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- There are neither enough drugs nor curses to express how enraging this header mess is. I've given up trying to understand what the "correct" solution is. So if this thing below works, Acked-by: Christian Brauner <brauner@kernel.org> > src/detached_mounts_propagation.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/src/detached_mounts_propagation.c b/src/detached_mounts_propagation.c > index 17db2c02..dd11f7be 100644 > --- a/src/detached_mounts_propagation.c > +++ b/src/detached_mounts_propagation.c > @@ -20,7 +20,6 @@ > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > -#include <sys/mount.h> > #include <sys/stat.h> > #include <sys/syscall.h> > #include <sys/types.h> > @@ -127,7 +126,7 @@ int main(int argc, char *argv[]) > if (ret < 0) > exit_log("%m - Failed to create new mount namespace"); > > - ret = mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); > + ret = sys_mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); > if (ret < 0) > exit_log("%m - Failed to make base_dir shared mountpoint"); > > @@ -174,7 +173,7 @@ int main(int argc, char *argv[]) > } > close(fd_tree); > > - ret = umount2(target, MNT_DETACH); > + ret = sys_umount2(target, MNT_DETACH); > if (ret < 0) { > fprintf(stderr, "%m - Failed to unmount %s", target); > exit_code = EXIT_FAILURE; > -- > 2.39.1 >
On 2023/4/25 13:33, Yang Xu wrote: > Newer glibc such as glibc 2.36 also defines 'struct mount_attr' > in addition to <linux/mount.h>(we also include this kernel header when using > global.h). > > Usually we should use glibc header instead of kernel header. > But now mount.h is a special case because both new glibc header and > kernel header all define "struct mount_attr'. They also define MS* > macro. > > Since we have some syscall wrapper in vfs/missing.h, we can use > <linux.mount.h> directly instead of <sys/mount.h>. > > In fact, newer glibc(2.37-1)[1] has sloved conflict problem between > <sys/mount.h> and <linux/mount.h>. In the future(maybe ten years), > we can remove this kernel header and use glibc header. > > [1]https://sourceware.org/git/?p=glibc.git;a=commit;h=774058d72942249f71d74e7f2b639f77184160a6 > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > src/detached_mounts_propagation.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/src/detached_mounts_propagation.c b/src/detached_mounts_propagation.c > index 17db2c02..dd11f7be 100644 > --- a/src/detached_mounts_propagation.c > +++ b/src/detached_mounts_propagation.c > @@ -20,7 +20,6 @@ > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > -#include <sys/mount.h> > #include <sys/stat.h> > #include <sys/syscall.h> > #include <sys/types.h> > @@ -127,7 +126,7 @@ int main(int argc, char *argv[]) > if (ret < 0) > exit_log("%m - Failed to create new mount namespace"); > > - ret = mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); > + ret = sys_mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); > if (ret < 0) > exit_log("%m - Failed to make base_dir shared mountpoint"); > > @@ -174,7 +173,7 @@ int main(int argc, char *argv[]) > } > close(fd_tree); > > - ret = umount2(target, MNT_DETACH); > + ret = sys_umount2(target, MNT_DETACH); > if (ret < 0) { > fprintf(stderr, "%m - Failed to unmount %s", target); > exit_code = EXIT_FAILURE; Hi Yang, Unfortunately, we find another error after applying this patch: Building vfs [CC] vfstest In file included from utils.h:32, from utils.c:21: missing.h:115:8: error: redefinition of 'struct mount_attr' 115 | struct mount_attr { | ^~~~~~~~~~ In file included from utils.c:13: /usr/include/sys/mount.h:210:8: note: originally defined here 210 | struct mount_attr | ^~~~~~~~~~ gmake[4]: *** [Makefile:30: vfstest] Error 1 gmake[3]: *** [../include/buildrules:31: vfs] Error 2 gmake[2]: *** [include/buildrules:31: src] Error 2 make[1]: *** [Makefile:51: default] Error 2 make: *** [Makefile:49: default] Error 2 Looks like we have to fix src/vfs/utils.c too because it includes both <sys/mount.h> and "utils.h" and "utils.h" includes "missing.h". I simply remove #include <sys/mount.h> in src/vfs/utils.c and it works: diff --git a/src/vfs/utils.c b/src/vfs/utils.c index 6db7a11d..f30e3bd9 100644 --- a/src/vfs/utils.c +++ b/src/vfs/utils.c @@ -10,7 +10,6 @@ #include <stdlib.h> #include <sys/eventfd.h> #include <sys/fsuid.h> -#include <sys/mount.h> #include <sys/prctl.h> #include <sys/socket.h> #include <sys/stat.h>
on 2023/04/25 15:47, Ziyang Zhang wrote: > On 2023/4/25 13:33, Yang Xu wrote: >> Newer glibc such as glibc 2.36 also defines 'struct mount_attr' >> in addition to <linux/mount.h>(we also include this kernel header when using >> global.h). >> >> Usually we should use glibc header instead of kernel header. >> But now mount.h is a special case because both new glibc header and >> kernel header all define "struct mount_attr'. They also define MS* >> macro. >> >> Since we have some syscall wrapper in vfs/missing.h, we can use >> <linux.mount.h> directly instead of <sys/mount.h>. >> >> In fact, newer glibc(2.37-1)[1] has sloved conflict problem between >> <sys/mount.h> and <linux/mount.h>. In the future(maybe ten years), >> we can remove this kernel header and use glibc header. >> >> [1]https://sourceware.org/git/?p=glibc.git;a=commit;h=774058d72942249f71d74e7f2b639f77184160a6 >> >> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> >> --- >> src/detached_mounts_propagation.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/src/detached_mounts_propagation.c b/src/detached_mounts_propagation.c >> index 17db2c02..dd11f7be 100644 >> --- a/src/detached_mounts_propagation.c >> +++ b/src/detached_mounts_propagation.c >> @@ -20,7 +20,6 @@ >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> -#include <sys/mount.h> >> #include <sys/stat.h> >> #include <sys/syscall.h> >> #include <sys/types.h> >> @@ -127,7 +126,7 @@ int main(int argc, char *argv[]) >> if (ret < 0) >> exit_log("%m - Failed to create new mount namespace"); >> >> - ret = mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); >> + ret = sys_mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); >> if (ret < 0) >> exit_log("%m - Failed to make base_dir shared mountpoint"); >> >> @@ -174,7 +173,7 @@ int main(int argc, char *argv[]) >> } >> close(fd_tree); >> >> - ret = umount2(target, MNT_DETACH); >> + ret = sys_umount2(target, MNT_DETACH); >> if (ret < 0) { >> fprintf(stderr, "%m - Failed to unmount %s", target); >> exit_code = EXIT_FAILURE; > > Hi Yang, > > Unfortunately, we find another error after applying this patch: > > Building vfs > [CC] vfstest > In file included from utils.h:32, > from utils.c:21: > missing.h:115:8: error: redefinition of 'struct mount_attr' > 115 | struct mount_attr { > | ^~~~~~~~~~ > In file included from utils.c:13: > /usr/include/sys/mount.h:210:8: note: originally defined here > 210 | struct mount_attr > | ^~~~~~~~~~ > gmake[4]: *** [Makefile:30: vfstest] Error 1 > gmake[3]: *** [../include/buildrules:31: vfs] Error 2 > gmake[2]: *** [include/buildrules:31: src] Error 2 > make[1]: *** [Makefile:51: default] Error 2 > make: *** [Makefile:49: default] Error 2 > > I have searched the placse of include <sys/mount.h> as below: m4/package_libcdev.m4:88:#include <sys/mount.h> src/aio-dio-regress/aiodio_sparse2.c:23:#include <sys/mount.h> src/ext4_resize.c:15:#include <sys/mount.h> src/getdevicesize.c:17:#include <sys/mount.h> src/vfs/utils.c:13:#include <sys/mount.h> autom4te.cache/traces.0:1929:#include <sys/mount.h> autom4te.cache/output.0:13858:# include <sys/mount.h> autom4te.cache/output.1:13858:# include <sys/mount.h only utils.c include xfstests owner header, other c source files should not be affected. I will remove <sys/mount.h> in utils.c and then send a v2. Best Regards Yang Xu > Looks like we have to fix src/vfs/utils.c too because it includes both <sys/mount.h> > and "utils.h" and "utils.h" includes "missing.h". > > I simply remove #include <sys/mount.h> in src/vfs/utils.c and it works: > > diff --git a/src/vfs/utils.c b/src/vfs/utils.c > index 6db7a11d..f30e3bd9 100644 > --- a/src/vfs/utils.c > +++ b/src/vfs/utils.c > @@ -10,7 +10,6 @@ > #include <stdlib.h> > #include <sys/eventfd.h> > #include <sys/fsuid.h> > -#include <sys/mount.h> > #include <sys/prctl.h> > #include <sys/socket.h> > #include <sys/stat.h>
diff --git a/src/detached_mounts_propagation.c b/src/detached_mounts_propagation.c index 17db2c02..dd11f7be 100644 --- a/src/detached_mounts_propagation.c +++ b/src/detached_mounts_propagation.c @@ -20,7 +20,6 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sys/mount.h> #include <sys/stat.h> #include <sys/syscall.h> #include <sys/types.h> @@ -127,7 +126,7 @@ int main(int argc, char *argv[]) if (ret < 0) exit_log("%m - Failed to create new mount namespace"); - ret = mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); + ret = sys_mount(NULL, base_dir, NULL, MS_REC | MS_SHARED, NULL); if (ret < 0) exit_log("%m - Failed to make base_dir shared mountpoint"); @@ -174,7 +173,7 @@ int main(int argc, char *argv[]) } close(fd_tree); - ret = umount2(target, MNT_DETACH); + ret = sys_umount2(target, MNT_DETACH); if (ret < 0) { fprintf(stderr, "%m - Failed to unmount %s", target); exit_code = EXIT_FAILURE;
Newer glibc such as glibc 2.36 also defines 'struct mount_attr' in addition to <linux/mount.h>(we also include this kernel header when using global.h). Usually we should use glibc header instead of kernel header. But now mount.h is a special case because both new glibc header and kernel header all define "struct mount_attr'. They also define MS* macro. Since we have some syscall wrapper in vfs/missing.h, we can use <linux.mount.h> directly instead of <sys/mount.h>. In fact, newer glibc(2.37-1)[1] has sloved conflict problem between <sys/mount.h> and <linux/mount.h>. In the future(maybe ten years), we can remove this kernel header and use glibc header. [1]https://sourceware.org/git/?p=glibc.git;a=commit;h=774058d72942249f71d74e7f2b639f77184160a6 Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> --- src/detached_mounts_propagation.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)