Rebase to nbdkit 1.42.2

resolves: RHEL-78830
Add extra system call checking and debugging to nbdkit-file-plugin
resolves: RHEL-85515
This commit is contained in:
Richard W.M. Jones 2025-03-31 14:03:58 +01:00
parent 3bdae99cca
commit 9d635982f2
6 changed files with 388 additions and 99 deletions

View File

@ -0,0 +1,41 @@
From 96315050f6683c8834f8ac3ccaea33e8e58fb183 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 28 Mar 2025 20:32:46 +0000
Subject: [PATCH] file: Add debugging if sync_file_range / posix_fadvise fails
These are advisory hints, but add some debugging so we know if these
calls are failing. The errors are ignored so there is no behaviour
change.
Thanks: Eric Sandeen
(cherry picked from commit feb22cfc85b0750a4e4bfbc5fe56636883599feb)
---
plugins/file/file.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 52396bb2..77de1cc8 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -125,11 +125,13 @@ evict_writes (int fd, uint64_t offset, size_t len)
/* Evict the oldest window from the page cache. */
if (window[0].len > 0) {
- sync_file_range (window[0].fd, window[0].offset, window[0].len,
- SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE|
- SYNC_FILE_RANGE_WAIT_AFTER);
- posix_fadvise (window[0].fd, window[0].offset, window[0].len,
- POSIX_FADV_DONTNEED);
+ if (sync_file_range (window[0].fd, window[0].offset, window[0].len,
+ SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE|
+ SYNC_FILE_RANGE_WAIT_AFTER) == -1)
+ nbdkit_debug ("sync_file_range: wait: (ignored): %m");
+ if (posix_fadvise (window[0].fd, window[0].offset, window[0].len,
+ POSIX_FADV_DONTNEED) == -1)
+ nbdkit_debug ("posix_fadvise: POSIX_FADV_DONTNEED: (ignored): %m");
}
/* Move the Nth window to N-1. */
--
2.47.1

View File

@ -1,92 +0,0 @@
From 19c46bafab94716141a8e2ec9ade2f7dd47a6384 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 5 Mar 2025 11:31:10 +0000
Subject: [PATCH] rust: Disable clippy warnings unless --enable-gcc-warnings is
used
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Turn ./configure --enable-gcc-warnings into a general "enable compiler
warnings" option (we'll probably need to change the name of it later).
Then use this option to control whether Rust clippy warnings are
enabled or not.
Note that CI generally enables this option so we should still see Rust
errors in CI.
Reported-by: Thomas Weißschuh
(cherry picked from commit aa6b3a9954418659c5d27aae3ef3fe9005961510)
---
configure.ac | 14 +++++++++-----
plugins/rust/Makefile.am | 5 ++++-
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/configure.ac b/configure.ac
index c3ef3aff..ff4a4049 100644
--- a/configure.ac
+++ b/configure.ac
@@ -225,17 +225,20 @@ AS_IF([$CXX --version >&AS_MESSAGE_LOG_FD 2>&1],[have_cxx=yes],[have_cxx=no])
AC_MSG_RESULT([$have_cxx])
AM_CONDITIONAL([HAVE_CXX], [test "$have_cxx" = "yes"])
+dnl If --enable-gcc-warnings is used, then compiler warnings are
+dnl turned on. Despite the name this applies to GCC, Clang and
+dnl Rust (clippy) warnings.
AC_ARG_ENABLE([gcc-warnings],
[AS_HELP_STRING([--enable-gcc-warnings],
- [turn on lots of GCC warnings (for developers)])],
+ [turn on compiler warnings (for developers)])],
[case $enableval in
yes|no) ;;
*) AC_MSG_ERROR([bad value $enableval for gcc-warnings option]) ;;
esac
- gcc_warnings=$enableval],
- [gcc_warnings=no]
+ compiler_warnings=$enableval],
+ [compiler_warnings=no]
)
-if test "x$gcc_warnings" = "xyes"; then
+if test "x$compiler_warnings" = "xyes"; then
WARNINGS_CFLAGS="-Wall -Wshadow -Wvla -Werror"
AC_SUBST([WARNINGS_CFLAGS])
@@ -251,8 +254,8 @@ if test "x$gcc_warnings" = "xyes"; then
[AC_MSG_RESULT([no])]
)
AC_SUBST([WARNINGS_MODULE_CXXFLAGS])
-
fi
+AM_CONDITIONAL([COMPILER_WARNINGS], [test "$compiler_warnings" = "yes"])
dnl Check if the compiler supports -std=c90 flag. This is only used
dnl during a test. OpenBSD GCC does not support this flag so we skip
@@ -1818,6 +1821,7 @@ echo
echo "Other optional features:"
echo
feature "allocator=zstd" test "x$HAVE_LIBZSTD_TRUE" = "x"
+feature "compiler warnings" test "x$COMPILER_WARNINGS_TRUE" = "x"
feature "tests using libguestfs" \
test "x$HAVE_LIBGUESTFS_TRUE" = "x" && \
test "x$USE_LIBGUESTFS_FOR_TESTS_TRUE" = "x"
diff --git a/plugins/rust/Makefile.am b/plugins/rust/Makefile.am
index dc774e11..33f13107 100644
--- a/plugins/rust/Makefile.am
+++ b/plugins/rust/Makefile.am
@@ -81,9 +81,12 @@ endif
TESTS = \
cargo-tests.sh \
test-ramdisk.sh \
- clippy.sh \
$(NULL)
+if COMPILER_WARNINGS
+TESTS += clippy.sh
+endif
+
if HAVE_POD
man_MANS = nbdkit-rust-plugin.3
--
2.47.1

View File

@ -0,0 +1,82 @@
From 93e3804d42b5e6f4afcd7524eb07d776b56c883d Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 28 Mar 2025 20:35:54 +0000
Subject: [PATCH] file: If sync_file_range fails to start, don't add window to
list
When starting the process of writing out the newest range, we
previously didn't check if sync_file_range succeeded or failed. But
if it failed there's no reason to add it to the list of windows. Also
debug any failures.
This shouldn't change any semantics as far as we're aware.
Thanks: Eric Sandeen
(cherry picked from commit d35f0636373e6a58c8f3fcfcf505af248e27c574)
---
plugins/file/file.c | 48 ++++++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 77de1cc8..0271a56b 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -123,27 +123,35 @@ evict_writes (int fd, uint64_t offset, size_t len)
{
ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&window_lock);
- /* Evict the oldest window from the page cache. */
- if (window[0].len > 0) {
- if (sync_file_range (window[0].fd, window[0].offset, window[0].len,
- SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE|
- SYNC_FILE_RANGE_WAIT_AFTER) == -1)
- nbdkit_debug ("sync_file_range: wait: (ignored): %m");
- if (posix_fadvise (window[0].fd, window[0].offset, window[0].len,
- POSIX_FADV_DONTNEED) == -1)
- nbdkit_debug ("posix_fadvise: POSIX_FADV_DONTNEED: (ignored): %m");
- }
-
- /* Move the Nth window to N-1. */
- memmove (&window[0], &window[1], sizeof window[0] * (NR_WINDOWS-1));
-
- /* Set up the current window and tell Linux to start writing it out
- * to disk (asynchronously).
+ /* Tell Linux to start writing the current range out to disk
+ * (asynchronously).
*/
- sync_file_range (fd, offset, len, SYNC_FILE_RANGE_WRITE);
- window[NR_WINDOWS-1].fd = fd;
- window[NR_WINDOWS-1].offset = offset;
- window[NR_WINDOWS-1].len = len;
+ if (sync_file_range (fd, offset, len, SYNC_FILE_RANGE_WRITE) == -1) {
+ /* sync_file_range to start the write failed */
+ nbdkit_debug ("sync_file_range: write: (ignored): %m");
+ }
+ else {
+ /* sync_file_range to start the write succeeded, so
+ * evict the oldest window from the page cache.
+ */
+ if (window[0].len > 0) {
+ if (sync_file_range (window[0].fd, window[0].offset, window[0].len,
+ SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE|
+ SYNC_FILE_RANGE_WAIT_AFTER) == -1)
+ nbdkit_debug ("sync_file_range: wait: (ignored): %m");
+ if (posix_fadvise (window[0].fd, window[0].offset, window[0].len,
+ POSIX_FADV_DONTNEED) == -1)
+ nbdkit_debug ("posix_fadvise: POSIX_FADV_DONTNEED: (ignored): %m");
+ }
+
+ /* Move the Nth window to N-1. */
+ memmove (&window[0], &window[1], sizeof window[0] * (NR_WINDOWS-1));
+
+ /* Add the range to the newest end of the list of windows. */
+ window[NR_WINDOWS-1].fd = fd;
+ window[NR_WINDOWS-1].offset = offset;
+ window[NR_WINDOWS-1].len = len;
+ }
}
/* When we close the handle we must remove any windows which are still
--
2.47.1

View File

@ -0,0 +1,254 @@
From ecd719d5ab101e54cab496085b827a485c9398e6 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sat, 29 Mar 2025 10:01:05 +0000
Subject: [PATCH] tests: Add tests of file plugin cache=none
These are based on the tests which were done in the commit message
when the feature was first added in commit aa5a2183a6, but cleaned up
and modernised a little.
It is likely that these tests would fail under memory pressure (since
they test that the page cache is used vs not used). Making them more
robust left as an exercise for the future.
(cherry picked from commit 6017ba21aeeb3d7ad85925e78dba85a005194dee)
---
README.md | 1 +
tests/Makefile.am | 4 ++
tests/test-file-cache-none-read.sh | 90 ++++++++++++++++++++++++++++
tests/test-file-cache-none-write.sh | 92 +++++++++++++++++++++++++++++
4 files changed, 187 insertions(+)
create mode 100755 tests/test-file-cache-none-read.sh
create mode 100755 tests/test-file-cache-none-write.sh
diff --git a/README.md b/README.md
index 5d61e786..2f5123c9 100644
--- a/README.md
+++ b/README.md
@@ -179,6 +179,7 @@ To test for memory leaks (`make check-valgrind`):
For non-essential enhancements to the test suite:
+* cachedel, cachestats (from https://github.com/Feh/nocache)
* expect
* fdisk, sfdisk (from util-linux)
* flake8
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7376c948..e8003c1c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -899,6 +899,8 @@ TESTS += \
test-file-extents.sh \
test-file-dir.sh \
test-file-dirfd.sh \
+ test-file-cache-none-read.sh \
+ test-file-cache-none-write.sh \
$(NULL)
EXTRA_DIST += \
test-file.sh \
@@ -908,6 +910,8 @@ EXTRA_DIST += \
test-file-extents.sh \
test-file-dir.sh \
test-file-dirfd.sh \
+ test-file-cache-none-read.sh \
+ test-file-cache-none-write.sh \
$(NULL)
LIBGUESTFS_TESTS += test-file-block
LIBNBD_TESTS += test-file-block-nbd
diff --git a/tests/test-file-cache-none-read.sh b/tests/test-file-cache-none-read.sh
new file mode 100755
index 00000000..c9831b43
--- /dev/null
+++ b/tests/test-file-cache-none-read.sh
@@ -0,0 +1,90 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright Red Hat
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Test the file plugin, reading with cache=none.
+
+source ./functions.sh
+set -e
+set -x
+
+# Makes no sense to run this test under valgrind.
+skip_if_valgrind
+
+requires_plugin file
+requires_run
+requires_nbdcopy
+requires $TRUNCATE --version
+requires test -r /dev/urandom
+requires dd --version
+requires $SED --version
+
+# Requires the cachestats tool from https://github.com/Feh/nocache
+# It doesn't support --version or --help, so use 'type' instead.
+requires type cachestats
+requires type cachedel
+
+inp=file-cache-none-read.in
+stats1=file-cache-none-read.s1
+stats2=file-cache-none-read.s2
+rm -f $inp $stats1 $stats2
+cleanup_fn rm -f $inp $stats1 $stats2
+
+# Create a large random file as input.
+dd if=/dev/urandom of=$inp bs=1024k count=1024
+
+# Drop the input file from the page cache and read it out with nbdkit.
+# We expect to see the input file mostly or completely in cache after.
+cachedel $inp
+nbdkit file $inp --run 'nbdcopy "$uri" null:'
+cachestats $inp > $stats1
+cat $stats1
+
+# The same, with cache=none.
+# We expect to see the input file not cached after.
+cachedel $inp
+nbdkit file $inp --run 'nbdcopy "$uri" null:' cache=none
+cachestats $inp > $stats2
+cat $stats2
+
+# The output of cachestats looks like this:
+# pages in cache: 262144/262144 (100.0%) [filesize=1048576.0K, pagesize=4K]
+# We want to check that % pages in cache using cache=none is much
+# lower than the default case.
+pic1="$($SED 's,pages in cache: [0-9/]* (\([0-9]*\)\.[0-9]*%).*,\1,' \
+ < $stats1)"
+pic2="$($SED 's,pages in cache: [0-9/]* (\([0-9]*\)\.[0-9]*%).*,\1,' \
+ < $stats2)"
+
+# Test before is > 10%
+test "$pic1" -gt 10
+# Test after is < 10%
+test "$pic2" -lt 10
diff --git a/tests/test-file-cache-none-write.sh b/tests/test-file-cache-none-write.sh
new file mode 100755
index 00000000..2041a5cd
--- /dev/null
+++ b/tests/test-file-cache-none-write.sh
@@ -0,0 +1,92 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright Red Hat
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Test the file plugin, writing with cache=none.
+
+source ./functions.sh
+set -e
+set -x
+
+# Makes no sense to run this test under valgrind.
+skip_if_valgrind
+
+requires_plugin file
+requires_run
+requires_nbdcopy
+requires $TRUNCATE --version
+requires test -r /dev/urandom
+requires dd --version
+requires $SED --version
+
+# Requires the cachestats tool from https://github.com/Feh/nocache
+# It doesn't support --version or --help, so use 'type' instead.
+requires type cachestats
+
+inp=file-cache-none-write.in
+out=file-cache-none-write.out
+stats1=file-cache-none-write.s1
+stats2=file-cache-none-write.s2
+rm -f $inp $out $stats1 $stats2
+cleanup_fn rm -f $inp $out $stats1 $stats2
+
+# Create a large random file as input.
+dd if=/dev/urandom of=$inp bs=1024k count=1024
+
+# Copy to output using cache=default and collect the stats.
+# We expect to see the output file mostly or completely in cache after.
+rm -f $out; truncate -r $inp $out
+export inp
+nbdkit file $out --run 'nbdcopy $inp "$uri"' cache=default
+cachestats $out > $stats1
+cat $stats1
+
+# The same, with cache=none.
+# We expect to see the output file not cached after.
+rm -f $out; truncate -r $inp $out
+export inp
+nbdkit file $out --run 'nbdcopy $inp "$uri"' cache=none
+cachestats $out > $stats2
+cat $stats2
+
+# The output of cachestats looks like this:
+# pages in cache: 262144/262144 (100.0%) [filesize=1048576.0K, pagesize=4K]
+# We want to check that % pages in cache using cache=none is much
+# lower than the default case.
+pic1="$($SED 's,pages in cache: [0-9/]* (\([0-9]*\)\.[0-9]*%).*,\1,' \
+ < $stats1)"
+pic2="$($SED 's,pages in cache: [0-9/]* (\([0-9]*\)\.[0-9]*%).*,\1,' \
+ < $stats2)"
+
+# Test before is > 10%
+test "$pic1" -gt 10
+# Test after is < 10%
+test "$pic2" -lt 10
--
2.47.1

View File

@ -54,7 +54,7 @@
%global source_directory 1.42-stable
Name: nbdkit
Version: 1.42.1
Version: 1.42.2
Release: 1%{?dist}
Summary: NBD server
@ -80,7 +80,9 @@ Source3: copy-patches.sh
# https://gitlab.com/nbdkit/nbdkit/-/commits/rhel-10.1/
# Patches.
Patch0001: 0001-rust-Disable-clippy-warnings-unless-enable-gcc-warni.patch
Patch0001: 0001-file-Add-debugging-if-sync_file_range-posix_fadvise-.patch
Patch0002: 0002-file-If-sync_file_range-fails-to-start-don-t-add-win.patch
Patch0003: 0003-tests-Add-tests-of-file-plugin-cache-none.patch
# For automatic RPM Provides generation.
# See: https://rpm-software-management.github.io/rpm/manual/dependency_generators.html
@ -1523,12 +1525,14 @@ fi
%changelog
* Mon Mar 10 2025 Richard W.M. Jones <rjones@redhat.com> - 1.42.1-1
- Rebase to nbdkit 1.42.1
* Mon Mar 31 2025 Richard W.M. Jones <rjones@redhat.com> - 1.42.2-1
- Rebase to nbdkit 1.42.2
- Synch the spec file with Fedora Rawhide.
- nbdkit-ondemand-plugin moves into a new subpackage.
- New nbdkit-time-limit-filter.
- resolves: RHEL-78830
resolves: RHEL-78830
- Add extra system call checking and debugging to nbdkit-file-plugin
resolves: RHEL-85515
* Mon Jan 06 2025 Richard W.M. Jones <rjones@redhat.com> - 1.40.4-3
- vddk: Avoid reading partial chunk beyond the end of the disk

View File

@ -1,2 +1,2 @@
SHA512 (nbdkit-1.42.1.tar.gz) = ff6b171c6e73eb9bec70ef0f565dc16add72dfbef29db147fa639fd0b443a9fe0f72d5f832b1eb8f397d62d66660fb825e29b0314ec9d6670aa1a7aa9c1dbc3d
SHA512 (nbdkit-1.42.1.tar.gz.sig) = 20e1059cdb0d53aeac066e81ee92c49c58cec65dce928f3e11c0213c4e7a9700fa73f2b990efa974be5afc877d2082f85e40e511001ba5e971faa51c9247ec2b
SHA512 (nbdkit-1.42.2.tar.gz) = 007040b1a6e4653353305646af784aece194d6e8b66ce645774015a72ffb8863e70919fc13ec2b66bb762d7c963d5cbed02ac26b803b1202eff1a4183470ada3
SHA512 (nbdkit-1.42.2.tar.gz.sig) = 8d356481c1005dc9d566e3de1ff9b695ae1c50182ebf4913476a6a15e7ad6ae2f529caef712b2a4156eda539236ba8f20896c335de27c91ef734d83ccc132050