1. Patchew
  2. QEMU
  3. [RFC 0/3] block/file-posix: Work around XFS bug
  4. View patch

[RFC 2/3] block/file-posix: Detect XFS with CONFIG_FALLOCATE

Max Reitz posted 3 patches 6 years, 3 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Max Reitz <mreitz@redhat.com>, Fam Zheng <fam@euphon.net>
[RFC 2/3] block/file-posix: Detect XFS with CONFIG_FALLOCATE
Posted by Max Reitz 6 years, 3 months ago 
We will need this for the next patch.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/file-posix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 695fcf740d..5cd54c8bff 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -149,7 +149,7 @@ typedef struct BDRVRawState {
     int perm_change_flags;
     BDRVReopenState *reopen_state;
 
-#ifdef CONFIG_XFS
+#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
     bool is_xfs:1;
 #endif
     bool has_discard:1;
@@ -667,7 +667,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     }
 #endif
 
-#ifdef CONFIG_XFS
+#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
     if (platform_test_xfs_fd(s->fd)) {
         s->is_xfs = true;
     }
-- 
2.21.0
Re: [RFC 2/3] block/file-posix: Detect XFS with CONFIG_FALLOCATE
Posted by Kevin Wolf 6 years, 3 months ago 
Am 25.10.2019 um 11:58 hat Max Reitz geschrieben:
> We will need this for the next patch.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/file-posix.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 695fcf740d..5cd54c8bff 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -149,7 +149,7 @@ typedef struct BDRVRawState {
>      int perm_change_flags;
>      BDRVReopenState *reopen_state;
>  
> -#ifdef CONFIG_XFS
> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>      bool is_xfs:1;
>  #endif
>      bool has_discard:1;
> @@ -667,7 +667,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>      }
>  #endif
>  
> -#ifdef CONFIG_XFS
> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>      if (platform_test_xfs_fd(s->fd)) {

platform_test_xfs_fd() is defined in a header file from xfsprogs. I
don't think you can call that without CONFIG_XFS, it would break the
build.

>          s->is_xfs = true;
>      }

Kevin
Re: [RFC 2/3] block/file-posix: Detect XFS with CONFIG_FALLOCATE
Posted by Max Reitz 6 years, 3 months ago 
On 25.10.19 12:19, Kevin Wolf wrote:
> Am 25.10.2019 um 11:58 hat Max Reitz geschrieben:
>> We will need this for the next patch.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/file-posix.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index 695fcf740d..5cd54c8bff 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -149,7 +149,7 @@ typedef struct BDRVRawState {
>>      int perm_change_flags;
>>      BDRVReopenState *reopen_state;
>>  
>> -#ifdef CONFIG_XFS
>> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>>      bool is_xfs:1;
>>  #endif
>>      bool has_discard:1;
>> @@ -667,7 +667,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>>      }
>>  #endif
>>  
>> -#ifdef CONFIG_XFS
>> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>>      if (platform_test_xfs_fd(s->fd)) {
> 
> platform_test_xfs_fd() is defined in a header file from xfsprogs. I
> don't think you can call that without CONFIG_XFS, it would break the
> build.

Aw.

OK.  Should we then just assume is_xfs to be true (for the next patch)
with !defined(CONFIG_XFS) && defined(CONFIG_FALLOCATE)?

Max
Re: [RFC 2/3] block/file-posix: Detect XFS with CONFIG_FALLOCATE
Posted by Kevin Wolf 6 years, 3 months ago 
Am 25.10.2019 um 12:22 hat Max Reitz geschrieben:
> On 25.10.19 12:19, Kevin Wolf wrote:
> > Am 25.10.2019 um 11:58 hat Max Reitz geschrieben:
> >> We will need this for the next patch.
> >>
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> ---
> >>  block/file-posix.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/block/file-posix.c b/block/file-posix.c
> >> index 695fcf740d..5cd54c8bff 100644
> >> --- a/block/file-posix.c
> >> +++ b/block/file-posix.c
> >> @@ -149,7 +149,7 @@ typedef struct BDRVRawState {
> >>      int perm_change_flags;
> >>      BDRVReopenState *reopen_state;
> >>  
> >> -#ifdef CONFIG_XFS
> >> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
> >>      bool is_xfs:1;
> >>  #endif
> >>      bool has_discard:1;
> >> @@ -667,7 +667,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
> >>      }
> >>  #endif
> >>  
> >> -#ifdef CONFIG_XFS
> >> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
> >>      if (platform_test_xfs_fd(s->fd)) {
> > 
> > platform_test_xfs_fd() is defined in a header file from xfsprogs. I
> > don't think you can call that without CONFIG_XFS, it would break the
> > build.
> 
> Aw.
> 
> OK.  Should we then just assume is_xfs to be true (for the next patch)
> with !defined(CONFIG_XFS) && defined(CONFIG_FALLOCATE)?

It's a small inline function that basically just calls statfs() and then
checks f_type.

I think we can have a small function to implement this in the file-posix
code. Don't copy it from the header because of licensing (LGPL, while
file-posix is MIT); refer to the statfs() manpage instead and write it
yourself.

Kevin
Re: [RFC 2/3] block/file-posix: Detect XFS with CONFIG_FALLOCATE
Posted by Max Reitz 6 years, 3 months ago 
On 25.10.19 12:35, Kevin Wolf wrote:
> Am 25.10.2019 um 12:22 hat Max Reitz geschrieben:
>> On 25.10.19 12:19, Kevin Wolf wrote:
>>> Am 25.10.2019 um 11:58 hat Max Reitz geschrieben:
>>>> We will need this for the next patch.
>>>>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>> ---
>>>>  block/file-posix.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/block/file-posix.c b/block/file-posix.c
>>>> index 695fcf740d..5cd54c8bff 100644
>>>> --- a/block/file-posix.c
>>>> +++ b/block/file-posix.c
>>>> @@ -149,7 +149,7 @@ typedef struct BDRVRawState {
>>>>      int perm_change_flags;
>>>>      BDRVReopenState *reopen_state;
>>>>  
>>>> -#ifdef CONFIG_XFS
>>>> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>>>>      bool is_xfs:1;
>>>>  #endif
>>>>      bool has_discard:1;
>>>> @@ -667,7 +667,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>>>>      }
>>>>  #endif
>>>>  
>>>> -#ifdef CONFIG_XFS
>>>> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>>>>      if (platform_test_xfs_fd(s->fd)) {
>>>
>>> platform_test_xfs_fd() is defined in a header file from xfsprogs. I
>>> don't think you can call that without CONFIG_XFS, it would break the
>>> build.
>>
>> Aw.
>>
>> OK.  Should we then just assume is_xfs to be true (for the next patch)
>> with !defined(CONFIG_XFS) && defined(CONFIG_FALLOCATE)?
> 
> It's a small inline function that basically just calls statfs() and then
> checks f_type.

Yes, this is where my error came from.  I asked cscope, which happily
referred me to the inline implementation and so I didn’t bother looking
at the filename and just assumed it’d be some code that belongs to qemu.

> I think we can have a small function to implement this in the file-posix
> code. Don't copy it from the header because of licensing (LGPL, while
> file-posix is MIT); refer to the statfs() manpage instead and write it
> yourself.

OK.

Max
Re: [RFC 2/3] block/file-posix: Detect XFS with CONFIG_FALLOCATE
Posted by Nir Soffer 6 years, 3 months ago 
On Fri, Oct 25, 2019 at 1:22 PM Max Reitz <mreitz@redhat.com> wrote:
>
> We will need this for the next patch.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/file-posix.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 695fcf740d..5cd54c8bff 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -149,7 +149,7 @@ typedef struct BDRVRawState {
>      int perm_change_flags;
>      BDRVReopenState *reopen_state;
>
> -#ifdef CONFIG_XFS
> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>      bool is_xfs:1;
>  #endif
>      bool has_discard:1;
> @@ -667,7 +667,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
>      }
>  #endif
>
> -#ifdef CONFIG_XFS
> +#if defined(CONFIG_XFS) || defined(CONFIG_FALLOCATE)
>      if (platform_test_xfs_fd(s->fd)) {
>          s->is_xfs = true;

What about remote xfs filesystem, e.g. glusterfs over xfs mounted using fuse?
(how oVirt uses glusterfs)

The buggy behavior with concurrent fallocate/pwrite can affect this, and
platform_test_xfs_fd() will probably fail to detect xfs.

Nir

>      }
> --
> 2.21.0
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.