From: "Yang Xu (Fujitsu)" <xuyang2018.jy@fujitsu.com>
To: Zorro Lang <zlang@redhat.com>, David Sterba <dsterba@suse.cz>
Cc: "fstests@vger.kernel.org" <fstests@vger.kernel.org>,
"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH v2] t_snapshot_deleted_subvolume: add check for BTRFS_IOC_SNAP_DESTROY_V2
Date: Wed, 7 Feb 2024 06:08:44 +0000 [thread overview]
Message-ID: <3b2bd3e2-be39-43f7-b541-5d70a06b0db7@fujitsu.com> (raw)
In-Reply-To: <20240206153214.kskmv2ug7b7rmtdq@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com>
> 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.
prev parent reply other threads:[~2024-02-07 6:09 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3b2bd3e2-be39-43f7-b541-5d70a06b0db7@fujitsu.com \
--to=xuyang2018.jy@fujitsu.com \
--cc=dsterba@suse.cz \
--cc=fstests@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=zlang@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).