[PATCH v2] file: Add missing include for FALLOC_FL_*
On RHEL 7.5 we need to include <linux/falloc.h> for FALLOC_FL_* macros. Without the macros, fallocate is never used and we fall back to manual zeroing. Here are examples runs with this change with a local file on ext4: $ export SOCK=/tmp/nbd.sock $ export FILE=/var/tmp/nbd.img $ export BLOCK=/dev/loop2 $ src/nbdkit -f plugins/file/.libs/nbdkit-file-plugin.so file=$FILE -U $SOCK $ time qemu-img convert -n -f raw -O raw /var/tmp/fedora-27.img nbd:unix:$SOCK real 0m13.361s user 0m0.127s sys 0m0.668s $ src/nbdkit -f plugins/file/.libs/nbdkit-file-plugin.so file=$BLOCK -U $SOCK $ time qemu-img convert -n -f raw -O raw /var/tmp/fedora-27.img nbd:unix:$SOCK real 0m13.491s user 0m0.129s sys 0m0.612s Tested on Fedora 28 and RHEL 7.5. --- v2: - Include <linux/falloc.h> only on Linux v1 was here: https://www.redhat.com/archives/libguestfs/2018-July/msg00083.html plugins/file/file.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/file/file.c b/plugins/file/file.c index a8a6253..0345115 100644 --- a/plugins/file/file.c +++ b/plugins/file/file.c @@ -42,6 +42,10 @@ #include <sys/stat.h> #include <errno.h> +#if defined(__linux__) +#include <linux/falloc.h> /* For FALLOC_FL_* on RHEL, glibc < 2.18 */ +#endif + #include <nbdkit-plugin.h> #ifndef O_CLOEXEC -- 2.17.1
On 07/30/2018 01:04 PM, Nir Soffer wrote:
On RHEL 7.5 we need to include <linux/falloc.h> for FALLOC_FL_* macros.
Rather, on any Linux system that pre-dates glibc 2.18, where the flags were finally supported directly in <fcntl.h>
Without the macros, fallocate is never used and we fall back to manual zeroing.
+++ b/plugins/file/file.c @@ -42,6 +42,10 @@ #include <sys/stat.h> #include <errno.h>
+#if defined(__linux__) +#include <linux/falloc.h> /* For FALLOC_FL_* on RHEL, glibc < 2.18 */
Doesn't mention which version of RHEL, nor the fact that non-RHEL systems may also be impacted (it is the glibc version that matters here, rather than the distro).
+#endif +
ACK. Perhaps could be made tighter, with a less ambiguous comment, as: #if defined(__linux__) && !defined(FALLOC_FL_PUNCH_HOLE) # include <linux/falloc.h> /* For FALLOC_FL_*, on glibc < 2.18 */ #endif but let's see if Rich has any preference between the two. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Mon, Jul 30, 2018 at 9:16 PM Eric Blake <eblake@redhat.com> wrote:
On 07/30/2018 01:04 PM, Nir Soffer wrote:
On RHEL 7.5 we need to include <linux/falloc.h> for FALLOC_FL_* macros.
Rather, on any Linux system that pre-dates glibc 2.18, where the flags were finally supported directly in <fcntl.h>
Without the macros, fallocate is never used and we fall back to manual zeroing.
+++ b/plugins/file/file.c @@ -42,6 +42,10 @@ #include <sys/stat.h> #include <errno.h>
+#if defined(__linux__) +#include <linux/falloc.h> /* For FALLOC_FL_* on RHEL, glibc < 2.18 */
Doesn't mention which version of RHEL, nor the fact that non-RHEL systems may also be impacted (it is the glibc version that matters here, rather than the distro).
+#endif +
ACK.
Perhaps could be made tighter, with a less ambiguous comment, as:
#if defined(__linux__) && !defined(FALLOC_FL_PUNCH_HOLE) # include <linux/falloc.h> /* For FALLOC_FL_*, on glibc < 2.18 */ #endif
but let's see if Rich has any preference between the two.
Both changes are better, I'll post v3 if Rich prefer this. Nir
On Mon, Jul 30, 2018 at 01:16:10PM -0500, Eric Blake wrote:
On 07/30/2018 01:04 PM, Nir Soffer wrote:
On RHEL 7.5 we need to include <linux/falloc.h> for FALLOC_FL_* macros.
Rather, on any Linux system that pre-dates glibc 2.18, where the flags were finally supported directly in <fcntl.h>
Without the macros, fallocate is never used and we fall back to manual zeroing.
+++ b/plugins/file/file.c @@ -42,6 +42,10 @@ #include <sys/stat.h> #include <errno.h> +#if defined(__linux__) +#include <linux/falloc.h> /* For FALLOC_FL_* on RHEL, glibc < 2.18 */
Doesn't mention which version of RHEL, nor the fact that non-RHEL systems may also be impacted (it is the glibc version that matters here, rather than the distro).
+#endif +
ACK.
Perhaps could be made tighter, with a less ambiguous comment, as:
#if defined(__linux__) && !defined(FALLOC_FL_PUNCH_HOLE) # include <linux/falloc.h> /* For FALLOC_FL_*, on glibc < 2.18 */ #endif
but let's see if Rich has any preference between the two.
Your version is better. Upstream hat on we don't care about the weirdness of those enterprise distros :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
participants (4)
-
Eric Blake -
Nir Soffer -
Nir Soffer -
Richard W.M. Jones