[PATCH 2/2] virfile: Fix build with glibc 2.36

Cole Robinson posted 2 patches 3 years, 6 months ago
[PATCH 2/2] virfile: Fix build with glibc 2.36
Posted by Cole Robinson 3 years, 6 months ago
With glibc 2.36, sys/mount.h and linux/mount.h conflict:
https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E

virfile.c imports sys/mount.h and linux/fs.h, which pulls in
linux/mount.h.

Manually define the constants we need from linux/fs.h, like was
done in llvm:

https://reviews.llvm.org/rGb379129c4beb3f26223288627a1291739f33af02

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 src/util/virfile.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 99da058db3..65d6d2a701 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -71,7 +71,9 @@
 # endif
 # include <sys/ioctl.h>
 # include <linux/cdrom.h>
-# include <linux/fs.h>
+# define FS_IOC_GETFLAGS _IOR('f', 1, long)
+# define FS_IOC_SETFLAGS _IOR('f', 2, long)
+# define FS_NOCOW_FL 0x00800000
 #endif
 
 #if WITH_LIBATTR
-- 
2.36.1
Re: [PATCH 2/2] virfile: Fix build with glibc 2.36
Posted by Erik Skultety 3 years, 6 months ago
On Mon, Aug 01, 2022 at 03:59:15PM -0400, Cole Robinson wrote:
> With glibc 2.36, sys/mount.h and linux/mount.h conflict:
> https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
> 
> virfile.c imports sys/mount.h and linux/fs.h, which pulls in
> linux/mount.h.
> 
> Manually define the constants we need from linux/fs.h, like was
> done in llvm:
> 
> https://reviews.llvm.org/rGb379129c4beb3f26223288627a1291739f33af02
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
>  src/util/virfile.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 99da058db3..65d6d2a701 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -71,7 +71,9 @@
>  # endif
>  # include <sys/ioctl.h>
>  # include <linux/cdrom.h>
> -# include <linux/fs.h>

The commit message does explain the issue, but I'd still appreciate if there
was a short commentary explaining this explicit constant definition here as
well.

> +# define FS_IOC_GETFLAGS _IOR('f', 1, long)
> +# define FS_IOC_SETFLAGS _IOR('f', 2, long)

^this one has to be defined as FS_IOC_SETFLAGS _IOW('f', 2, long)

> +# define FS_NOCOW_FL 0x00800000
>  #endif
>  
>  #if WITH_LIBATTR
> -- 
> 2.36.1
> 

With the fixes:
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Re: [PATCH 2/2] virfile: Fix build with glibc 2.36
Posted by Cole Robinson 3 years, 6 months ago
On 8/2/22 3:12 AM, Erik Skultety wrote:
> On Mon, Aug 01, 2022 at 03:59:15PM -0400, Cole Robinson wrote:
>> With glibc 2.36, sys/mount.h and linux/mount.h conflict:
>> https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
>>
>> virfile.c imports sys/mount.h and linux/fs.h, which pulls in
>> linux/mount.h.
>>
>> Manually define the constants we need from linux/fs.h, like was
>> done in llvm:
>>
>> https://reviews.llvm.org/rGb379129c4beb3f26223288627a1291739f33af02
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>> ---
>>  src/util/virfile.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/util/virfile.c b/src/util/virfile.c
>> index 99da058db3..65d6d2a701 100644
>> --- a/src/util/virfile.c
>> +++ b/src/util/virfile.c
>> @@ -71,7 +71,9 @@
>>  # endif
>>  # include <sys/ioctl.h>
>>  # include <linux/cdrom.h>
>> -# include <linux/fs.h>
> 
> The commit message does explain the issue, but I'd still appreciate if there
> was a short commentary explaining this explicit constant definition here as
> well.
> 

Done.

>> +# define FS_IOC_GETFLAGS _IOR('f', 1, long)
>> +# define FS_IOC_SETFLAGS _IOR('f', 2, long)
> 
> ^this one has to be defined as FS_IOC_SETFLAGS _IOW('f', 2, long)
>

Darn, nice catch!

Here's the CI pipeline:
https://gitlab.com/crobinso/libvirt/-/pipelines/602982400

It passed, so I pushed with your suggested changes.

Thanks,
Cole