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

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



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

> 





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux