[PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[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

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[Zorro Lang]

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
> 
> 

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[David Sterba]

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
> 

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[Zorro Lang]

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
> > 
> 

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[David Sterba]

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.

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[Zorro Lang]

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

> 

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[David Sterba]

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.

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[Zorro Lang]

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 :)

> 

Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2

[Yang Xu (Fujitsu)]

> 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.