* [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2
@ 2024-02-01 4:23 Yang Xu
2024-02-04 13:15 ` Zorro Lang
2024-02-05 15:49 ` David Sterba
0 siblings, 2 replies; 9+ messages in thread
From: Yang Xu @ 2024-02-01 4:23 UTC (permalink / raw)
To: fstests, linux-btrfs; +Cc: Yang Xu
On some platform, struct btrfs_ioctl_vol_args_v2 is defined, but the
macros BTRFS_IOC_SNAP_DESTROY_V2 is not defined. This will cause
compile error. Add check for BTRFS_IOC_SNAP_DESTROY_V2 to solve this
problem.
BTRFS_IOC_SNAP_CREATE_V2 and BTRFS_IOC_SUBVOL_CREATE_V2 were
introduced together with struct btrfs_ioctl_vol_args_v2 by the
commit 55e301fd57a6 ("Btrfs: move fs/btrfs/ioctl.h to
include/uapi/linux/btrfs.h"). So there is no need to check them.
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
configure.ac | 1 +
src/t_snapshot_deleted_subvolume.c | 10 +++++-----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index b22fc52b..b14b1ab8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -109,6 +109,7 @@ AC_CHECK_MEMBERS([struct btrfs_ioctl_vol_args_v2.subvolid], [], [], [[
#include <stddef.h>
#include <linux/btrfs.h>
]])
+AC_CHECK_DECLS([BTRFS_IOC_SNAP_DESTROY_V2],,,[#include <linux/btrfs.h>])
AC_CONFIG_HEADERS([include/config.h])
AC_CONFIG_FILES([include/builddefs])
diff --git a/src/t_snapshot_deleted_subvolume.c b/src/t_snapshot_deleted_subvolume.c
index c3adb1c4..402c0515 100644
--- a/src/t_snapshot_deleted_subvolume.c
+++ b/src/t_snapshot_deleted_subvolume.c
@@ -20,11 +20,6 @@
#define BTRFS_IOCTL_MAGIC 0x94
#endif
-#ifndef BTRFS_IOC_SNAP_DESTROY_V2
-#define BTRFS_IOC_SNAP_DESTROY_V2 \
- _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2)
-#endif
-
#ifndef BTRFS_IOC_SNAP_CREATE_V2
#define BTRFS_IOC_SNAP_CREATE_V2 \
_IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2)
@@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 {
};
#endif
+#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2
+#define BTRFS_IOC_SNAP_DESTROY_V2 \
+ _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2)
+#endif
+
int main(int argc, char **argv)
{
if (argc != 2) {
--
2.39.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-01 4:23 [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 Yang Xu @ 2024-02-04 13:15 ` Zorro Lang 2024-02-05 15:49 ` David Sterba 1 sibling, 0 replies; 9+ messages in thread From: Zorro Lang @ 2024-02-04 13:15 UTC (permalink / raw) To: Yang Xu; +Cc: fstests, linux-btrfs On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: > On some platform, struct btrfs_ioctl_vol_args_v2 is defined, but the > macros BTRFS_IOC_SNAP_DESTROY_V2 is not defined. This will cause > compile error. Add check for BTRFS_IOC_SNAP_DESTROY_V2 to solve this > problem. > > BTRFS_IOC_SNAP_CREATE_V2 and BTRFS_IOC_SUBVOL_CREATE_V2 were > introduced together with struct btrfs_ioctl_vol_args_v2 by the > commit 55e301fd57a6 ("Btrfs: move fs/btrfs/ioctl.h to > include/uapi/linux/btrfs.h"). So there is no need to check them. > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- This patch is good to me, and test passed on rhel-8. Reviewed-by: Zorro Lang <zlang@redhat.com> > configure.ac | 1 + > src/t_snapshot_deleted_subvolume.c | 10 +++++----- > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/configure.ac b/configure.ac > index b22fc52b..b14b1ab8 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -109,6 +109,7 @@ AC_CHECK_MEMBERS([struct btrfs_ioctl_vol_args_v2.subvolid], [], [], [[ > #include <stddef.h> > #include <linux/btrfs.h> > ]]) > +AC_CHECK_DECLS([BTRFS_IOC_SNAP_DESTROY_V2],,,[#include <linux/btrfs.h>]) > > AC_CONFIG_HEADERS([include/config.h]) > AC_CONFIG_FILES([include/builddefs]) > diff --git a/src/t_snapshot_deleted_subvolume.c b/src/t_snapshot_deleted_subvolume.c > index c3adb1c4..402c0515 100644 > --- a/src/t_snapshot_deleted_subvolume.c > +++ b/src/t_snapshot_deleted_subvolume.c > @@ -20,11 +20,6 @@ > #define BTRFS_IOCTL_MAGIC 0x94 > #endif > > -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 > -#define BTRFS_IOC_SNAP_DESTROY_V2 \ > - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > -#endif > - > #ifndef BTRFS_IOC_SNAP_CREATE_V2 > #define BTRFS_IOC_SNAP_CREATE_V2 \ > _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) > @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { > }; > #endif > > +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 > +#define BTRFS_IOC_SNAP_DESTROY_V2 \ > + _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > +#endif > + > int main(int argc, char **argv) > { > if (argc != 2) { > -- > 2.39.3 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-01 4:23 [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 Yang Xu 2024-02-04 13:15 ` Zorro Lang @ 2024-02-05 15:49 ` David Sterba 2024-02-06 10:10 ` Zorro Lang 1 sibling, 1 reply; 9+ messages in thread From: David Sterba @ 2024-02-05 15:49 UTC (permalink / raw) To: Yang Xu; +Cc: fstests, linux-btrfs On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: > On some platform, struct btrfs_ioctl_vol_args_v2 is defined, but the > macros BTRFS_IOC_SNAP_DESTROY_V2 is not defined. This will cause > compile error. Add check for BTRFS_IOC_SNAP_DESTROY_V2 to solve this > problem. > > BTRFS_IOC_SNAP_CREATE_V2 and BTRFS_IOC_SUBVOL_CREATE_V2 were > introduced together with struct btrfs_ioctl_vol_args_v2 by the > commit 55e301fd57a6 ("Btrfs: move fs/btrfs/ioctl.h to > include/uapi/linux/btrfs.h"). So there is no need to check them. > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > --- > configure.ac | 1 + > src/t_snapshot_deleted_subvolume.c | 10 +++++----- > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/configure.ac b/configure.ac > index b22fc52b..b14b1ab8 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -109,6 +109,7 @@ AC_CHECK_MEMBERS([struct btrfs_ioctl_vol_args_v2.subvolid], [], [], [[ > #include <stddef.h> > #include <linux/btrfs.h> > ]]) > +AC_CHECK_DECLS([BTRFS_IOC_SNAP_DESTROY_V2],,,[#include <linux/btrfs.h>]) > > AC_CONFIG_HEADERS([include/config.h]) > AC_CONFIG_FILES([include/builddefs]) > diff --git a/src/t_snapshot_deleted_subvolume.c b/src/t_snapshot_deleted_subvolume.c > index c3adb1c4..402c0515 100644 > --- a/src/t_snapshot_deleted_subvolume.c > +++ b/src/t_snapshot_deleted_subvolume.c > @@ -20,11 +20,6 @@ > #define BTRFS_IOCTL_MAGIC 0x94 > #endif > > -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 > -#define BTRFS_IOC_SNAP_DESTROY_V2 \ > - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > -#endif > - > #ifndef BTRFS_IOC_SNAP_CREATE_V2 > #define BTRFS_IOC_SNAP_CREATE_V2 \ > _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) > @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { > }; > #endif > > +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 This is right for AC_CHECK_DECLS. Other macros like AC_CHECK_HEADERS do not define the HAVE_... in case it's not found so the #if !HAVE_... would be wrong. Slightly confusing. > +#define BTRFS_IOC_SNAP_DESTROY_V2 \ > + _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > +#endif > + > int main(int argc, char **argv) > { > if (argc != 2) { > -- > 2.39.3 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-05 15:49 ` David Sterba @ 2024-02-06 10:10 ` Zorro Lang 2024-02-06 12:02 ` David Sterba 0 siblings, 1 reply; 9+ messages in thread From: Zorro Lang @ 2024-02-06 10:10 UTC (permalink / raw) To: David Sterba; +Cc: Yang Xu, fstests, linux-btrfs On Mon, Feb 05, 2024 at 04:49:07PM +0100, David Sterba wrote: > On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: > > On some platform, struct btrfs_ioctl_vol_args_v2 is defined, but the > > macros BTRFS_IOC_SNAP_DESTROY_V2 is not defined. This will cause > > compile error. Add check for BTRFS_IOC_SNAP_DESTROY_V2 to solve this > > problem. > > > > BTRFS_IOC_SNAP_CREATE_V2 and BTRFS_IOC_SUBVOL_CREATE_V2 were > > introduced together with struct btrfs_ioctl_vol_args_v2 by the > > commit 55e301fd57a6 ("Btrfs: move fs/btrfs/ioctl.h to > > include/uapi/linux/btrfs.h"). So there is no need to check them. > > > > Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com> > > --- > > configure.ac | 1 + > > src/t_snapshot_deleted_subvolume.c | 10 +++++----- > > 2 files changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/configure.ac b/configure.ac > > index b22fc52b..b14b1ab8 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -109,6 +109,7 @@ AC_CHECK_MEMBERS([struct btrfs_ioctl_vol_args_v2.subvolid], [], [], [[ > > #include <stddef.h> > > #include <linux/btrfs.h> > > ]]) > > +AC_CHECK_DECLS([BTRFS_IOC_SNAP_DESTROY_V2],,,[#include <linux/btrfs.h>]) > > > > AC_CONFIG_HEADERS([include/config.h]) > > AC_CONFIG_FILES([include/builddefs]) > > diff --git a/src/t_snapshot_deleted_subvolume.c b/src/t_snapshot_deleted_subvolume.c > > index c3adb1c4..402c0515 100644 > > --- a/src/t_snapshot_deleted_subvolume.c > > +++ b/src/t_snapshot_deleted_subvolume.c > > @@ -20,11 +20,6 @@ > > #define BTRFS_IOCTL_MAGIC 0x94 > > #endif > > > > -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 > > -#define BTRFS_IOC_SNAP_DESTROY_V2 \ > > - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > > -#endif > > - > > #ifndef BTRFS_IOC_SNAP_CREATE_V2 > > #define BTRFS_IOC_SNAP_CREATE_V2 \ > > _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) > > @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { > > }; > > #endif > > > > +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 > > This is right for AC_CHECK_DECLS. Other macros like AC_CHECK_HEADERS do > not define the HAVE_... in case it's not found so the #if !HAVE_... > would be wrong. Slightly confusing. Won't AC_CHECK_HEADERS define the HAVE_... ? But how do we get the ... /* Define to 1 if you have the <linux/falloc.h> header file. */ #define HAVE_LINUX_FALLOC_H 1 in include/config.h file? > > > +#define BTRFS_IOC_SNAP_DESTROY_V2 \ > > + _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > > +#endif > > + > > int main(int argc, char **argv) > > { > > if (argc != 2) { > > -- > > 2.39.3 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-06 10:10 ` Zorro Lang @ 2024-02-06 12:02 ` David Sterba 2024-02-06 13:32 ` Zorro Lang 0 siblings, 1 reply; 9+ messages in thread From: David Sterba @ 2024-02-06 12:02 UTC (permalink / raw) To: Zorro Lang; +Cc: David Sterba, Yang Xu, fstests, linux-btrfs On Tue, Feb 06, 2024 at 06:10:05PM +0800, Zorro Lang wrote: > On Mon, Feb 05, 2024 at 04:49:07PM +0100, David Sterba wrote: > > On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: > > > @@ -20,11 +20,6 @@ > > > #define BTRFS_IOCTL_MAGIC 0x94 > > > #endif > > > > > > -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 > > > -#define BTRFS_IOC_SNAP_DESTROY_V2 \ > > > - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > > > -#endif > > > - > > > #ifndef BTRFS_IOC_SNAP_CREATE_V2 > > > #define BTRFS_IOC_SNAP_CREATE_V2 \ > > > _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) > > > @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { > > > }; > > > #endif > > > > > > +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 > > > > This is right for AC_CHECK_DECLS. Other macros like AC_CHECK_HEADERS do > > not define the HAVE_... in case it's not found so the #if !HAVE_... > > would be wrong. Slightly confusing. > > Won't AC_CHECK_HEADERS define the HAVE_... ? But how do we get the ... > > /* Define to 1 if you have the <linux/falloc.h> header file. */ > #define HAVE_LINUX_FALLOC_H 1 > > in include/config.h file? Yes the HAVE_ macros are defined, just that it actually also defines #define HAVE_LINUX_FALLOC_H 0 if not found, unlike other macros result in /* #undef HAVE_SOME_FUNCTION */ What you did will work, the inconsistency is in the autoconf macros. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-06 12:02 ` David Sterba @ 2024-02-06 13:32 ` Zorro Lang 2024-02-06 13:47 ` David Sterba 0 siblings, 1 reply; 9+ messages in thread From: Zorro Lang @ 2024-02-06 13:32 UTC (permalink / raw) To: David Sterba; +Cc: Yang Xu, fstests, linux-btrfs On Tue, Feb 06, 2024 at 01:02:01PM +0100, David Sterba wrote: > On Tue, Feb 06, 2024 at 06:10:05PM +0800, Zorro Lang wrote: > > On Mon, Feb 05, 2024 at 04:49:07PM +0100, David Sterba wrote: > > > On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: > > > > @@ -20,11 +20,6 @@ > > > > #define BTRFS_IOCTL_MAGIC 0x94 > > > > #endif > > > > > > > > -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 > > > > -#define BTRFS_IOC_SNAP_DESTROY_V2 \ > > > > - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > > > > -#endif > > > > - > > > > #ifndef BTRFS_IOC_SNAP_CREATE_V2 > > > > #define BTRFS_IOC_SNAP_CREATE_V2 \ > > > > _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) > > > > @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { > > > > }; > > > > #endif > > > > > > > > +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 > > > > > > This is right for AC_CHECK_DECLS. Other macros like AC_CHECK_HEADERS do > > > not define the HAVE_... in case it's not found so the #if !HAVE_... > > > would be wrong. Slightly confusing. > > > > Won't AC_CHECK_HEADERS define the HAVE_... ? But how do we get the ... > > > > /* Define to 1 if you have the <linux/falloc.h> header file. */ > > #define HAVE_LINUX_FALLOC_H 1 > > > > in include/config.h file? > > Yes the HAVE_ macros are defined, just that it actually also defines > > #define HAVE_LINUX_FALLOC_H 0 Oh I didn't find that in my local fstests code (has been built), I got something likes this in include/config.h (for defined or un-defined): /* Define to 1 if you have the <cifs/ioctl.h> header file. */ /* #undef HAVE_CIFS_IOCTL_H */ /* Define to 1 if you have the declaration of `BTRFS_IOC_SNAP_DESTROY_V2', and to 0 if you don't. */ #define HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 1 > > if not found, unlike other macros result in > > /* #undef HAVE_SOME_FUNCTION */ > > What you did will work, the inconsistency is in the autoconf macros. But I'm not familar with these AC_CHECK things:) Maybe its behavior isn't sure, AC_CHECK_DECLS is sure to define HAVE_.... to 1, AC_CHECK_HEADERS is sure to have a definition but not sure what's defined. Do you mean that? BTW, I think you're not nacking this patch, right? :) Thanks, Zorro > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-06 13:32 ` Zorro Lang @ 2024-02-06 13:47 ` David Sterba 2024-02-06 15:32 ` Zorro Lang 0 siblings, 1 reply; 9+ messages in thread From: David Sterba @ 2024-02-06 13:47 UTC (permalink / raw) To: Zorro Lang; +Cc: David Sterba, Yang Xu, fstests, linux-btrfs On Tue, Feb 06, 2024 at 09:32:35PM +0800, Zorro Lang wrote: > On Tue, Feb 06, 2024 at 01:02:01PM +0100, David Sterba wrote: > > On Tue, Feb 06, 2024 at 06:10:05PM +0800, Zorro Lang wrote: > > > On Mon, Feb 05, 2024 at 04:49:07PM +0100, David Sterba wrote: > > > > On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: > > > > > @@ -20,11 +20,6 @@ > > > > > #define BTRFS_IOCTL_MAGIC 0x94 > > > > > #endif > > > > > > > > > > -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 > > > > > -#define BTRFS_IOC_SNAP_DESTROY_V2 \ > > > > > - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > > > > > -#endif > > > > > - > > > > > #ifndef BTRFS_IOC_SNAP_CREATE_V2 > > > > > #define BTRFS_IOC_SNAP_CREATE_V2 \ > > > > > _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) > > > > > @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { > > > > > }; > > > > > #endif > > > > > > > > > > +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 > > > > > > > > This is right for AC_CHECK_DECLS. Other macros like AC_CHECK_HEADERS do > > > > not define the HAVE_... in case it's not found so the #if !HAVE_... > > > > would be wrong. Slightly confusing. > > > > > > Won't AC_CHECK_HEADERS define the HAVE_... ? But how do we get the ... > > > > > > /* Define to 1 if you have the <linux/falloc.h> header file. */ > > > #define HAVE_LINUX_FALLOC_H 1 > > > > > > in include/config.h file? > > > > Yes the HAVE_ macros are defined, just that it actually also defines > > > > #define HAVE_LINUX_FALLOC_H 0 > > Oh I didn't find that in my local fstests code (has been built), I got > something likes this in include/config.h (for defined or un-defined): > > /* Define to 1 if you have the <cifs/ioctl.h> header file. */ > /* #undef HAVE_CIFS_IOCTL_H */ > > /* Define to 1 if you have the declaration of `BTRFS_IOC_SNAP_DESTROY_V2', and > to 0 if you don't. */ > #define HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 1 > > > > > if not found, unlike other macros result in > > > > /* #undef HAVE_SOME_FUNCTION */ > > > > What you did will work, the inconsistency is in the autoconf macros. > > But I'm not familar with these AC_CHECK things:) Maybe its behavior isn't > sure, AC_CHECK_DECLS is sure to define HAVE_.... to 1, AC_CHECK_HEADERS is > sure to have a definition but not sure what's defined. Do you mean that? > > BTW, I think you're not nacking this patch, right? :) No I'm not, sorry if this was confusing, it was a comment about the autoconf macros and how are the defines supposed to be checked. We once had a bug where #ifdef MACRO vs #if MACRO was not doing the same thing because of the sometimes/always defined semantics. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-06 13:47 ` David Sterba @ 2024-02-06 15:32 ` Zorro Lang 2024-02-07 6:08 ` Yang Xu (Fujitsu) 0 siblings, 1 reply; 9+ messages in thread From: Zorro Lang @ 2024-02-06 15:32 UTC (permalink / raw) To: David Sterba; +Cc: Yang Xu, fstests, linux-btrfs On Tue, Feb 06, 2024 at 02:47:13PM +0100, David Sterba wrote: > On Tue, Feb 06, 2024 at 09:32:35PM +0800, Zorro Lang wrote: > > On Tue, Feb 06, 2024 at 01:02:01PM +0100, David Sterba wrote: > > > On Tue, Feb 06, 2024 at 06:10:05PM +0800, Zorro Lang wrote: > > > > On Mon, Feb 05, 2024 at 04:49:07PM +0100, David Sterba wrote: > > > > > On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: > > > > > > @@ -20,11 +20,6 @@ > > > > > > #define BTRFS_IOCTL_MAGIC 0x94 > > > > > > #endif > > > > > > > > > > > > -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 > > > > > > -#define BTRFS_IOC_SNAP_DESTROY_V2 \ > > > > > > - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) > > > > > > -#endif > > > > > > - > > > > > > #ifndef BTRFS_IOC_SNAP_CREATE_V2 > > > > > > #define BTRFS_IOC_SNAP_CREATE_V2 \ > > > > > > _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) > > > > > > @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { > > > > > > }; > > > > > > #endif > > > > > > > > > > > > +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 > > > > > > > > > > This is right for AC_CHECK_DECLS. Other macros like AC_CHECK_HEADERS do > > > > > not define the HAVE_... in case it's not found so the #if !HAVE_... > > > > > would be wrong. Slightly confusing. > > > > > > > > Won't AC_CHECK_HEADERS define the HAVE_... ? But how do we get the ... > > > > > > > > /* Define to 1 if you have the <linux/falloc.h> header file. */ > > > > #define HAVE_LINUX_FALLOC_H 1 > > > > > > > > in include/config.h file? > > > > > > Yes the HAVE_ macros are defined, just that it actually also defines > > > > > > #define HAVE_LINUX_FALLOC_H 0 > > > > Oh I didn't find that in my local fstests code (has been built), I got > > something likes this in include/config.h (for defined or un-defined): > > > > /* Define to 1 if you have the <cifs/ioctl.h> header file. */ > > /* #undef HAVE_CIFS_IOCTL_H */ > > > > /* Define to 1 if you have the declaration of `BTRFS_IOC_SNAP_DESTROY_V2', and > > to 0 if you don't. */ > > #define HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 1 > > > > > > > > if not found, unlike other macros result in > > > > > > /* #undef HAVE_SOME_FUNCTION */ > > > > > > What you did will work, the inconsistency is in the autoconf macros. > > > > But I'm not familar with these AC_CHECK things:) Maybe its behavior isn't > > sure, AC_CHECK_DECLS is sure to define HAVE_.... to 1, AC_CHECK_HEADERS is > > sure to have a definition but not sure what's defined. Do you mean that? > > > > BTW, I think you're not nacking this patch, right? :) > > No I'm not, sorry if this was confusing, it was a comment about the > autoconf macros and how are the defines supposed to be checked. We once > had a bug where > > #ifdef MACRO > > vs > > #if MACRO > > was not doing the same thing because of the sometimes/always defined > semantics. Sure, thanks for this clarification, and the double checking from you :) > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 2024-02-06 15:32 ` Zorro Lang @ 2024-02-07 6:08 ` Yang Xu (Fujitsu) 0 siblings, 0 replies; 9+ messages in thread From: Yang Xu (Fujitsu) @ 2024-02-07 6:08 UTC (permalink / raw) To: Zorro Lang, David Sterba Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org > On Tue, Feb 06, 2024 at 02:47:13PM +0100, David Sterba wrote: >> On Tue, Feb 06, 2024 at 09:32:35PM +0800, Zorro Lang wrote: >>> On Tue, Feb 06, 2024 at 01:02:01PM +0100, David Sterba wrote: >>>> On Tue, Feb 06, 2024 at 06:10:05PM +0800, Zorro Lang wrote: >>>>> On Mon, Feb 05, 2024 at 04:49:07PM +0100, David Sterba wrote: >>>>>> On Wed, Jan 31, 2024 at 11:23:48PM -0500, Yang Xu wrote: >>>>>>> @@ -20,11 +20,6 @@ >>>>>>> #define BTRFS_IOCTL_MAGIC 0x94 >>>>>>> #endif >>>>>>> >>>>>>> -#ifndef BTRFS_IOC_SNAP_DESTROY_V2 >>>>>>> -#define BTRFS_IOC_SNAP_DESTROY_V2 \ >>>>>>> - _IOW(BTRFS_IOCTL_MAGIC, 63, struct btrfs_ioctl_vol_args_v2) >>>>>>> -#endif >>>>>>> - >>>>>>> #ifndef BTRFS_IOC_SNAP_CREATE_V2 >>>>>>> #define BTRFS_IOC_SNAP_CREATE_V2 \ >>>>>>> _IOW(BTRFS_IOCTL_MAGIC, 23, struct btrfs_ioctl_vol_args_v2) >>>>>>> @@ -58,6 +53,11 @@ struct btrfs_ioctl_vol_args_v2 { >>>>>>> }; >>>>>>> #endif >>>>>>> >>>>>>> +#if !HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 >>>>>> >>>>>> This is right for AC_CHECK_DECLS. Other macros like AC_CHECK_HEADERS do >>>>>> not define the HAVE_... in case it's not found so the #if !HAVE_... >>>>>> would be wrong. Slightly confusing. >>>>> >>>>> Won't AC_CHECK_HEADERS define the HAVE_... ? But how do we get the ... >>>>> >>>>> /* Define to 1 if you have the <linux/falloc.h> header file. */ >>>>> #define HAVE_LINUX_FALLOC_H 1 >>>>> >>>>> in include/config.h file? >>>> >>>> Yes the HAVE_ macros are defined, just that it actually also defines >>>> >>>> #define HAVE_LINUX_FALLOC_H 0 >>> >>> Oh I didn't find that in my local fstests code (has been built), I got >>> something likes this in include/config.h (for defined or un-defined): >>> >>> /* Define to 1 if you have the <cifs/ioctl.h> header file. */ >>> /* #undef HAVE_CIFS_IOCTL_H */ >>> >>> /* Define to 1 if you have the declaration of `BTRFS_IOC_SNAP_DESTROY_V2', and >>> to 0 if you don't. */ >>> #define HAVE_DECL_BTRFS_IOC_SNAP_DESTROY_V2 1 >>> >>>> >>>> if not found, unlike other macros result in >>>> >>>> /* #undef HAVE_SOME_FUNCTION */ >>>> >>>> What you did will work, the inconsistency is in the autoconf macros. >>> >>> But I'm not familar with these AC_CHECK things:) Maybe its behavior isn't >>> sure, AC_CHECK_DECLS is sure to define HAVE_.... to 1, AC_CHECK_HEADERS is >>> sure to have a definition but not sure what's defined. Do you mean that? >>> >>> BTW, I think you're not nacking this patch, right? :) >> >> No I'm not, sorry if this was confusing, it was a comment about the >> autoconf macros and how are the defines supposed to be checked. We once >> had a bug where >> >> #ifdef MACRO >> >> vs >> >> #if MACRO >> >> was not doing the same thing because of the sometimes/always defined >> semantics. > > Sure, thanks for this clarification, and the double checking from you :) > >> > In the document of GNU autoconf. https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Generic-Declarations.html It says the AC_CHECK_DECLS is different from other AC_CHECK_*S. When a symbol is not declared, HAVE_DECL_symbol is defined to ‘0’ instead of leaving HAVE_DECL_symbol undeclared. Its sample is also in the following format: #if !HAVE_DECL_SYMBOL #do something here #endif This difference is really a bit difficult to understand. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-07 6:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-01 4:23 [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2 Yang Xu 2024-02-04 13:15 ` Zorro Lang 2024-02-05 15:49 ` David Sterba 2024-02-06 10:10 ` Zorro Lang 2024-02-06 12:02 ` David Sterba 2024-02-06 13:32 ` Zorro Lang 2024-02-06 13:47 ` David Sterba 2024-02-06 15:32 ` Zorro Lang 2024-02-07 6:08 ` Yang Xu (Fujitsu)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox