rpms/nbdkit
6
0
Fork 0
Code Issues Pull Requests Releases Activity

Rebase to nbdkit 1.44.0

Browse Source 
resolves: RHEL-101180
This commit is contained in:
Richard W.M. Jones 2025-07-01 10:29:22 +01:00
parent 7a3edc9542
commit c0820a1886
13 changed files with 38 additions and 2007 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

254
0001-tests-Add-tests-of-file-plugin-cache-none.patch
View File

@ -1,254 +0,0 @@
From eca2b4034d446f1daaca4805c8a5380cb352a512 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 10565b43..7836f5dd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -901,6 +901,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 \
@@ -910,6 +912,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

238
0002-tests-Add-more-generic-tests-of-file-cache-none.patch
View File

@ -1,238 +0,0 @@
From 05348e4e97a034070878e0ef44392b5844d32b1f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 1 Apr 2025 12:45:43 +0100
Subject: [PATCH] tests: Add more generic tests of file cache=none
The tests added in commit 6017ba21ae require cachestats which means
they are almost always skipped in reality. Also we're interested in
whether the windowed eviction could ever corrupt data. Write a
simpler pair of tests for data integrity.
Updates: commit 6017ba21aeeb3d7ad85925e78dba85a005194dee
(cherry picked from commit 7b45f73c0668e56a9188249eeefc2f67aeb50af3)
---
tests/Makefile.am | 12 +++--
tests/test-file-cache-none-read-consistent.sh | 53 ++++++++++++++++++
...=> test-file-cache-none-read-effective.sh} | 9 ++--
.../test-file-cache-none-write-consistent.sh | 54 +++++++++++++++++++
...> test-file-cache-none-write-effective.sh} | 11 ++--
5 files changed, 126 insertions(+), 13 deletions(-)
create mode 100755 tests/test-file-cache-none-read-consistent.sh
rename tests/{test-file-cache-none-read.sh => test-file-cache-none-read-effective.sh} (93%)
create mode 100755 tests/test-file-cache-none-write-consistent.sh
rename tests/{test-file-cache-none-write.sh => test-file-cache-none-write-effective.sh} (92%)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7836f5dd..00cb1618 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -901,8 +901,10 @@ 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 \
+ test-file-cache-none-read-consistent.sh \
+ test-file-cache-none-read-effective.sh \
+ test-file-cache-none-write-consistent.sh \
+ test-file-cache-none-write-effective.sh \
$(NULL)
EXTRA_DIST += \
test-file.sh \
@@ -912,8 +914,10 @@ 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 \
+ test-file-cache-none-read-consistent.sh \
+ test-file-cache-none-read-effective.sh \
+ test-file-cache-none-write-consistent.sh \
+ test-file-cache-none-write-effective.sh \
$(NULL)
LIBGUESTFS_TESTS += test-file-block
LIBNBD_TESTS += test-file-block-nbd
diff --git a/tests/test-file-cache-none-read-consistent.sh b/tests/test-file-cache-none-read-consistent.sh
new file mode 100755
index 00000000..5f5794ee
--- /dev/null
+++ b/tests/test-file-cache-none-read-consistent.sh
@@ -0,0 +1,53 @@
+#!/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.
+#
+# Unlike test-file-cache-none-read-effective.sh, this test doesn't
+# require cachestats. It's more about testing integrity of reading.
+
+source ./functions.sh
+set -e
+set -x
+
+requires_plugin file
+requires_run
+requires_nbdcopy
+requires test -f disk
+requires md5sum --version
+
+out=file-cache-none-read-consistent.out
+cleanup_fn rm -f $out
+
+export out
+nbdkit file disk cache=none --run 'nbdcopy "$uri" "$out"'
+test "$(md5sum < disk)" = "$(md5sum < $out)"
diff --git a/tests/test-file-cache-none-read.sh b/tests/test-file-cache-none-read-effective.sh
similarity index 93%
rename from tests/test-file-cache-none-read.sh
rename to tests/test-file-cache-none-read-effective.sh
index c9831b43..efead224 100755
--- a/tests/test-file-cache-none-read.sh
+++ b/tests/test-file-cache-none-read-effective.sh
@@ -30,7 +30,8 @@
# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
# SUCH DAMAGE.
-# Test the file plugin, reading with cache=none.
+# Test the file plugin, reading with cache=none, is effective at
+# reducing page cache usage.
source ./functions.sh
set -e
@@ -52,9 +53,9 @@ requires $SED --version
requires type cachestats
requires type cachedel
-inp=file-cache-none-read.in
-stats1=file-cache-none-read.s1
-stats2=file-cache-none-read.s2
+inp=file-cache-none-read-effective.in
+stats1=file-cache-none-read-effective.s1
+stats2=file-cache-none-read-effective.s2
rm -f $inp $stats1 $stats2
cleanup_fn rm -f $inp $stats1 $stats2
diff --git a/tests/test-file-cache-none-write-consistent.sh b/tests/test-file-cache-none-write-consistent.sh
new file mode 100755
index 00000000..749805f1
--- /dev/null
+++ b/tests/test-file-cache-none-write-consistent.sh
@@ -0,0 +1,54 @@
+#!/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.
+#
+# Unlike test-file-cache-none-write-effective.sh, this test doesn't
+# require cachestats. It's more about testing integrity of writing.
+
+source ./functions.sh
+set -e
+set -x
+
+requires_plugin file
+requires_run
+requires_nbdcopy
+requires test -f disk
+requires md5sum --version
+requires $TRUNCATE --version
+
+out=file-cache-none-write-consistent.out
+cleanup_fn rm -f $out
+
+rm -f $out; $TRUNCATE -r disk $out
+nbdkit file $out cache=none --run 'nbdcopy disk "$uri"'
+test "$(md5sum < disk)" = "$(md5sum < $out)"
diff --git a/tests/test-file-cache-none-write.sh b/tests/test-file-cache-none-write-effective.sh
similarity index 92%
rename from tests/test-file-cache-none-write.sh
rename to tests/test-file-cache-none-write-effective.sh
index 2041a5cd..e0159242 100755
--- a/tests/test-file-cache-none-write.sh
+++ b/tests/test-file-cache-none-write-effective.sh
@@ -30,7 +30,8 @@
# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
# SUCH DAMAGE.
-# Test the file plugin, writing with cache=none.
+# Test the file plugin, writing with cache=none, is effective at
+# reducing page cache usage.
source ./functions.sh
set -e
@@ -51,10 +52,10 @@ requires $SED --version
# 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
+inp=file-cache-none-write-effective.in
+out=file-cache-none-write-effective.out
+stats1=file-cache-none-write-effective.s1
+stats2=file-cache-none-write-effective.s2
rm -f $inp $out $stats1 $stats2
cleanup_fn rm -f $inp $out $stats1 $stats2
--
2.47.1

41
0003-vddk-Cache-the-readonly-flag-from-the-.open-call-in-.patch
View File

@ -1,41 +0,0 @@
From cdb379e91341feeefa2e2cdf1d09b66ed387ec1c Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 30 May 2025 13:32:00 +0100
Subject: [PATCH] vddk: Cache the readonly flag from the .open call in the
handle
(cherry picked from commit 0d953ea644f44259edb19c97e3c7863794c0ca1c)
---
plugins/vddk/vddk.c | 1 +
plugins/vddk/vddk.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index b5fa28fb..d00abde7 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -715,6 +715,7 @@ vddk_open (int readonly)
nbdkit_error ("calloc: %m");
return NULL;
}
+ h->readonly = readonly;
h->commands = (command_queue) empty_vector;
pthread_mutex_init (&h->commands_lock, NULL);
pthread_cond_init (&h->commands_cond, NULL);
diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h
index 1d1069cc..3586c5da 100644
--- a/plugins/vddk/vddk.h
+++ b/plugins/vddk/vddk.h
@@ -166,6 +166,9 @@ struct vddk_handle {
pthread_cond_t commands_cond; /* condition (queue size 0 -> 1) */
uint64_t id; /* next command ID */
+ /* Cached readonly flag from the open call. */
+ int readonly;
+
/* Cached disk size in bytes (set in get_size()). */
uint64_t size;
};
--
2.47.1

250
0004-vddk-Move-minimum-version-of-VDDK-to-6.7.patch
View File

@ -1,250 +0,0 @@
From ed73435936467ecc9d585764f7fa468ef3626eaa Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 30 May 2025 13:49:10 +0100
Subject: [PATCH] vddk: Move minimum version of VDDK to 6.7
We can now assume that QueryAllocatedBlocks exists, as well as a
couple of other functions, simplifying the code.
This was released in 2018 (7 years ago) so there's been plenty of time
to upgrade.
(cherry picked from commit 7c90116664b1b1a3c3756d6a79d6d483bc6062dd)
---
plugins/vddk/nbdkit-vddk-plugin.pod | 8 ++---
plugins/vddk/vddk-stubs.h | 32 +++++++++++---------
plugins/vddk/vddk.c | 43 ++++++---------------------
plugins/vddk/worker.c | 14 ++-------
tests/dummy-vddk.c | 45 +++++++++++++++++++++++++++--
5 files changed, 76 insertions(+), 66 deletions(-)
diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod
index 4810bceb..7cf938de 100644
--- a/plugins/vddk/nbdkit-vddk-plugin.pod
+++ b/plugins/vddk/nbdkit-vddk-plugin.pod
@@ -396,15 +396,13 @@ is I<not> recommended.
=over 4
-=item VDDK E<ge> 6.5 (released Nov 2016)
+=item VDDK E<ge> 6.7 (released Apr 2018)
This is the minimum version of VDDK supported. Older versions will
not work.
-=item VDDK 6.7 (released Apr 2018)
-
-This is the first version that supported the
-C<VixDiskLib_QueryAllocatedBlocks> API. This is required to provide
+This is also the first version that supported the
+C<VixDiskLib_QueryAllocatedBlocks> API. This is used to provide
sparseness (extent) information over NBD.
=item VDDK 9.0.0.0 (released Jun 2025)
diff --git a/plugins/vddk/vddk-stubs.h b/plugins/vddk/vddk-stubs.h
index 8e5afdb8..d768c51a 100644
--- a/plugins/vddk/vddk-stubs.h
+++ b/plugins/vddk/vddk-stubs.h
@@ -132,16 +132,22 @@ STUB (VixDiskLib_Wait,
VixError,
(VixDiskLibHandle handle));
-/* Added in VDDK 6.7, these will be NULL for earlier versions: */
-OPTIONAL_STUB (VixDiskLib_QueryAllocatedBlocks,
- VixError,
- (VixDiskLibHandle diskHandle,
- uint64_t start_sector, uint64_t nr_sectors,
- uint64_t chunk_size,
- VixDiskLibBlockList **block_list));
-OPTIONAL_STUB (VixDiskLib_FreeBlockList,
- VixError,
- (VixDiskLibBlockList *block_list));
-OPTIONAL_STUB (VixDiskLib_AllocateConnectParams,
- VixDiskLibConnectParams *,
- (void));
+/* Added in VDDK 6.7. */
+STUB (VixDiskLib_QueryAllocatedBlocks,
+ VixError,
+ (VixDiskLibHandle diskHandle,
+ uint64_t start_sector, uint64_t nr_sectors,
+ uint64_t chunk_size,
+ VixDiskLibBlockList **block_list));
+STUB (VixDiskLib_FreeBlockList,
+ VixError,
+ (VixDiskLibBlockList *block_list));
+STUB (VixDiskLib_AllocateConnectParams,
+ VixDiskLibConnectParams *,
+ (void));
+
+/* OPTIONAL_STUB can be used to add new APIs which are not supported
+ * by older versions of VDDK, and to make them NULL if not present.
+ * However VDDK >= 6.7 has everything we need for now so we are no
+ * longer using this macro.
+ */
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index d00abde7..3c8bb03a 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -668,37 +668,6 @@ vddk_dump_plugin (void)
/* Lock protecting open/close calls - see above. */
static pthread_mutex_t open_close_lock = PTHREAD_MUTEX_INITIALIZER;
-static inline VixDiskLibConnectParams *
-allocate_connect_params (void)
-{
- VixDiskLibConnectParams *ret;
-
- if (VixDiskLib_AllocateConnectParams != NULL) {
- VDDK_CALL_START (VixDiskLib_AllocateConnectParams, "")
- ret = VixDiskLib_AllocateConnectParams ();
- VDDK_CALL_END (VixDiskLib_AllocateConnectParams, 0);
- }
- else
- ret = calloc (1, sizeof (VixDiskLibConnectParams));
-
- return ret;
-}
-
-static inline void
-free_connect_params (VixDiskLibConnectParams *params)
-{
- /* Only use FreeConnectParams if AllocateConnectParams was
- * originally called. Otherwise use free.
- */
- if (VixDiskLib_AllocateConnectParams != NULL) {
- VDDK_CALL_START (VixDiskLib_FreeConnectParams, "params")
- VixDiskLib_FreeConnectParams (params);
- VDDK_CALL_END (VixDiskLib_FreeConnectParams, 0);
- }
- else
- free (params);
-}
-
/* Create the per-connection handle. */
static void *
vddk_open (int readonly)
@@ -720,7 +689,9 @@ vddk_open (int readonly)
pthread_mutex_init (&h->commands_lock, NULL);
pthread_cond_init (&h->commands_cond, NULL);
- h->params = allocate_connect_params ();
+ VDDK_CALL_START (VixDiskLib_AllocateConnectParams, "")
+ h->params = VixDiskLib_AllocateConnectParams ();
+ VDDK_CALL_END (VixDiskLib_AllocateConnectParams, 0);
if (h->params == NULL) {
nbdkit_error ("allocate VixDiskLibConnectParams: %m");
goto err0;
@@ -853,7 +824,9 @@ vddk_open (int readonly)
VixDiskLib_Disconnect (h->connection);
VDDK_CALL_END (VixDiskLib_Disconnect, 0);
err1:
- free_connect_params (h->params);
+ VDDK_CALL_START (VixDiskLib_FreeConnectParams, "params")
+ VixDiskLib_FreeConnectParams (h->params);
+ VDDK_CALL_END (VixDiskLib_FreeConnectParams, 0);
err0:
pthread_mutex_destroy (&h->commands_lock);
pthread_cond_destroy (&h->commands_cond);
@@ -879,7 +852,9 @@ vddk_close (void *handle)
VixDiskLib_Disconnect (h->connection);
VDDK_CALL_END (VixDiskLib_Disconnect, 0);
- free_connect_params (h->params);
+ VDDK_CALL_START (VixDiskLib_FreeConnectParams, "params")
+ VixDiskLib_FreeConnectParams (h->params);
+ VDDK_CALL_END (VixDiskLib_FreeConnectParams, 0);
pthread_mutex_destroy (&h->commands_lock);
pthread_cond_destroy (&h->commands_cond);
command_queue_reset (&h->commands);
diff --git a/plugins/vddk/worker.c b/plugins/vddk/worker.c
index 8c602ac7..e51da8c0 100644
--- a/plugins/vddk/worker.c
+++ b/plugins/vddk/worker.c
@@ -309,23 +309,13 @@ do_can_extents (struct command *cmd, struct vddk_handle *h)
VixError err;
VixDiskLibBlockList *block_list;
- /* This call was added in VDDK 6.7. In earlier versions the
- * function pointer will be NULL and we cannot query extents.
- */
- if (VixDiskLib_QueryAllocatedBlocks == NULL) {
- nbdkit_debug ("can_extents: VixDiskLib_QueryAllocatedBlocks == NULL, "
- "probably this is VDDK < 6.7");
- return 0;
- }
-
/* Suppress errors around this call. See:
* https://bugzilla.redhat.com/show_bug.cgi?id=1709211#c7
*/
error_suppression = 1;
- /* However even when the call is available it rarely works well so
- * the best thing we can do here is to try the call and if it's
- * non-functional return false.
+ /* Try the QueryAllocatedBlocks call and if it's non-functional
+ * return false.
*/
VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks,
"handle, 0, %d sectors, %d sectors",
diff --git a/tests/dummy-vddk.c b/tests/dummy-vddk.c
index 0e69012a..01284f38 100644
--- a/tests/dummy-vddk.c
+++ b/tests/dummy-vddk.c
@@ -116,11 +116,52 @@ VixDiskLib_FreeErrorText (char *text)
free (text);
}
+NBDKIT_DLL_PUBLIC VixError
+VixDiskLib_QueryAllocatedBlocks (VixDiskLibHandle diskHandle,
+ uint64_t start_sector, uint64_t nr_sectors,
+ uint64_t chunk_size,
+ VixDiskLibBlockList **block_list)
+{
+ VixDiskLibBlockList *ret;
+
+ /* This is safe because ret->blocks is a 1-sized array and we only
+ * use 1 entry here.
+ */
+ ret = calloc (1, sizeof *ret);
+ if (ret == NULL)
+ abort ();
+
+ /* Pretend it's all allocated. */
+ ret->numBlocks = 1;
+ ret->blocks[0].offset = start_sector;
+ ret->blocks[0].length = nr_sectors;
+
+ *block_list = ret;
+ return VIX_OK;
+}
+
+NBDKIT_DLL_PUBLIC VixError
+VixDiskLib_FreeBlockList (VixDiskLibBlockList *block_list)
+{
+ free (block_list);
+ return VIX_OK;
+}
+
+NBDKIT_DLL_PUBLIC VixDiskLibConnectParams *
+VixDiskLib_AllocateConnectParams (void)
+{
+ VixDiskLibConnectParams *ret;
+
+ ret = calloc (1, sizeof *ret);
+ if (ret == NULL)
+ abort ();
+ return ret;
+}
+
NBDKIT_DLL_PUBLIC void
VixDiskLib_FreeConnectParams (VixDiskLibConnectParams *params)
{
- /* never called since we don't define optional AllocateConnectParams */
- abort ();
+ free (params);
}
NBDKIT_DLL_PUBLIC VixError
--
2.47.1

78
0005-vddk-Unconditionally-test-QueryAllocatedBlocks.patch
View File

@ -1,78 +0,0 @@
From 084dfd7e9037e12a2d0ef7051f7d0d29ff9dd67a Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 30 May 2025 13:59:28 +0100
Subject: [PATCH] vddk: Unconditionally test QueryAllocatedBlocks
Except in rare cases like a suddenly dropped connection, we always
call vddk_can_extents and therefore do this test. We might as well do
it unconditionally when the worker thread starts up.
This simplifies following commits where we may do more work based on
this flag when the worker thread starts up.
(cherry picked from commit 787db3e8ecfd81b683f541065daee75665ab47e0)
---
plugins/vddk/worker.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/plugins/vddk/worker.c b/plugins/vddk/worker.c
index e51da8c0..b3a5180e 100644
--- a/plugins/vddk/worker.c
+++ b/plugins/vddk/worker.c
@@ -303,8 +303,13 @@ do_flush (struct command *cmd, struct vddk_handle *h)
return 0;
}
-static int
-do_can_extents (struct command *cmd, struct vddk_handle *h)
+/* Try the QueryAllocatedBlocks call and if it's non-functional return
+ * false. At some point in future, perhaps when we move to baseline
+ * VDDK >= 7, we can just assume it works and remove this test
+ * entirely.
+ */
+static bool
+test_can_extents (struct vddk_handle *h)
{
VixError err;
VixDiskLibBlockList *block_list;
@@ -314,9 +319,6 @@ do_can_extents (struct command *cmd, struct vddk_handle *h)
*/
error_suppression = 1;
- /* Try the QueryAllocatedBlocks call and if it's non-functional
- * return false.
- */
VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks,
"handle, 0, %d sectors, %d sectors",
VIXDISKLIB_MIN_CHUNK_SIZE, VIXDISKLIB_MIN_CHUNK_SIZE)
@@ -505,6 +507,10 @@ vddk_worker_thread (void *handle)
{
struct vddk_handle *h = handle;
bool stop = false;
+ bool can_extents;
+
+ /* Test if QueryAllocatedBlocks will work. */
+ can_extents = test_can_extents (h);
while (!stop) {
struct command *cmd;
@@ -547,12 +553,13 @@ vddk_worker_thread (void *handle)
break;
case CAN_EXTENTS:
- r = do_can_extents (cmd, h);
- if (r >= 0)
- *(int *)cmd->ptr = r;
+ *(int *)cmd->ptr = can_extents;
+ r = 0;
break;
case EXTENTS:
+ /* If we returned false above, we should never be called here. */
+ assert (can_extents);
r = do_extents (cmd, h);
break;
--
2.47.1

349
0006-vddk-Pre-cache-the-extents-for-readonly-connections.patch
View File

@ -1,349 +0,0 @@
From f93b05e7e6f7edd03cc73e154d53386e66216070 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 30 May 2025 15:06:07 +0100
Subject: [PATCH] vddk: Pre-cache the extents for readonly connections
As explained in detail in the code comment, QueryAllocatedBlocks has
very poor performance. We can partially work around this by
pre-caching extents when the first NBD_BLOCK_STATUS request is made.
We only do this for readonly connections, since it would be very
complex to do it for writable connections where the extent information
could change under us. And we only do it on the first
NBD_BLOCK_STATUS request, so we know that the caller is interested in
extents.
Benchmarking
------------
This improves performance, dramatically in some cases:
Size Used% [ni] [nm] [nc] [qi] [qm] [qc] [dd]
before 64G 16% 17 21 354 6 62 180
after " " 18 13 59 6 7 66 57
nc=178 MBytes/s dd=1150 MBytes/sec
before 128G 5.5% 17 29 646 6 50 151
after " " 17 14 68 6 8 71
nc=106 MBytes/s
before 128G 45% 17 30 1073 6 52 578
after " " 17 14 457 6 8 506 143
nc=128 MBytes/s dd=409 MBytes/sec
(all times in seconds)
[ni] nbdinfo $uri
Note: Makes two connections, unlike qemu-img info.
[nm] nbdinfo --map --totals $uri
Note: Slower than it ought to be, needs investigation.
[nc] nbdcopy -p $uri null:
[qi] qemu-img info $uri
[qm] qemu-img map $uri
[qc] qemu-img convert -p -n -f raw $uri \
-O raw 'json:{"file.driver":"null-co","file.size":"1E"}'
Note: Requests the map up front, which is why it performs better
than nbdcopy on the "before" version since reads are not being
serialized by concurrent calls to QueryAllocatedBlocks.
[dd] dd if=*-flat.vmdk of=/dev/null bs=16777216
Note: This command was run on the ESXi server where the storage
is assumed to be local. ESXi has very limited tools available
(eg. no "fio" etc). Also the cp command is from Busybox and is
notably slower than dd. To get the accurate copying rate I
assumed that this command copies all data on disk to /dev/null,
skipping reading holes where thin provisioned. IOW using the "ls
-s" output as the number of blocks read.
It should be noted that the after/[nc] numbers are not very stable.
In the last test where [nc] = 457, I see a deviation of as much as 10%
either side over multiple runs.
The network used in the test is 25 Gbps and clearly we are nowhere
near able to reach that. A more likely upper limit is the speed of
reading from the disks ([dd]). There is also a large gap between our
performance and that number. VMware is thought to impose a
per-connection limit of around 1 Gbps on NFC connections, and there
are other limitations
(https://knowledge.broadcom.com/external/article/307001/nfc-performance-is-slow-resulting-in-mig.html).
Tuning NFC makes no observable difference
-----------------------------------------
Further tuning of NFC is possible
(https://techdocs.broadcom.com/us/en/vmware-cis/vsphere/vsphere-supervisor/7-0/best-practices-for-nbd-transport.html).
Using compression (nbdkit vddk plugin 'compression' option) is
possible but in my test it makes things much slower. This is using
the first VM from the tests above:
[nc]
(no compression) 59 (178 MBytes/sec)
compression=zlib 323 ( 33 MBytes/sec)
VMware documentation also suggests using a configuration file
containing the entries below (the configuration file is placed
somewhere on the client, and referenced using the
config=/path/to/config.ini parameter):
vixDiskLib.nfcAio.Session.BufSizeIn64KB=32
vixDiskLib.nfcAio.Session.BufCount=16
This made no difference for me, at least when testing a single
conversion. Separate tests done by the MTV team suggest it may
improve performance if you are converting multiple disks / VMs in
parallel
(https://docs.redhat.com/en/documentation/migration_toolkit_for_virtualization/2.7/html/installing_and_using_the_migration_toolkit_for_virtualization/mtv-performance-recommendation_mtv#mtv-aio-buffer-key-findings_mtv)
Some VMware documentation also suggests:
config.nfc.useSSL=false
but this also made no difference.
Some VMware documentation suggests using unbuffered I/O
(unbuffered=true) but in my test this caused a large slow down.
Continue to disable multi-conn
------------------------------
We have recommended against using multi-conn with the VDDK plugin,
because we observed some slow down. This commit makes no difference
to this advice. The same amount of slow down is still observed. (In
virt-v2v we use --filter=multi-conn multi-conn-mode=disable to ensure
it is never used.)
(cherry picked from commit 5a882e74cae3dbaa09bf3b942a02f9947b12f6e5)
---
plugins/vddk/vddk.c | 2 +
plugins/vddk/vddk.h | 3 +
plugins/vddk/worker.c | 166 +++++++++++++++++++++++++++++++++++++++++-
3 files changed, 170 insertions(+), 1 deletion(-)
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index 3c8bb03a..a13698a3 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -845,6 +845,8 @@ vddk_close (void *handle)
send_command_and_wait (h, &stop_cmd);
pthread_join (h->thread, NULL);
+ nbdkit_extents_free (h->extents);
+
VDDK_CALL_START (VixDiskLib_Close, "handle")
VixDiskLib_Close (h->handle);
VDDK_CALL_END (VixDiskLib_Close, 0);
diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h
index 3586c5da..461fb528 100644
--- a/plugins/vddk/vddk.h
+++ b/plugins/vddk/vddk.h
@@ -171,6 +171,9 @@ struct vddk_handle {
/* Cached disk size in bytes (set in get_size()). */
uint64_t size;
+
+ /* Cached extents for readonly disks. */
+ struct nbdkit_extents *extents;
};
/* reexec.c */
diff --git a/plugins/vddk/worker.c b/plugins/vddk/worker.c
index b3a5180e..27f56e37 100644
--- a/plugins/vddk/worker.c
+++ b/plugins/vddk/worker.c
@@ -37,6 +37,7 @@
#include <stdbool.h>
#include <stdint.h>
#include <inttypes.h>
+#include <time.h>
#include <pthread.h>
@@ -380,7 +381,7 @@ add_extent (struct nbdkit_extents *extents,
}
static int
-do_extents (struct command *cmd, struct vddk_handle *h)
+get_extents_slow (struct command *cmd, struct vddk_handle *h)
{
const uint32_t count = cmd->count;
const uint64_t offset = cmd->offset;
@@ -499,6 +500,169 @@ do_extents (struct command *cmd, struct vddk_handle *h)
return 0;
}
+static int
+pre_cache_extents (struct vddk_handle *h)
+{
+ struct nbdkit_extents *extents;
+ uint64_t start_sector = 0;
+ uint64_t nr_chunks_remaining =
+ h->size / VIXDISKLIB_MIN_CHUNK_SIZE / VIXDISKLIB_SECTOR_SIZE;
+ uint64_t position = 0;
+
+ extents = nbdkit_extents_new (0, h->size);
+ if (extents == NULL)
+ return -1;
+
+ /* Scan through the disk reading whole "chunks" (32 GB), the most
+ * efficient way to use QueryAllocatedBlocks.
+ */
+ while (nr_chunks_remaining > 0) {
+ VixError err;
+ uint32_t i;
+ uint64_t nr_chunks, nr_sectors;
+ VixDiskLibBlockList *block_list;
+
+ assert (IS_ALIGNED (start_sector, VIXDISKLIB_MIN_CHUNK_SIZE));
+
+ nr_chunks = MIN (nr_chunks_remaining, VIXDISKLIB_MAX_CHUNK_NUMBER);
+ nr_sectors = nr_chunks * VIXDISKLIB_MIN_CHUNK_SIZE;
+
+ VDDK_CALL_START (VixDiskLib_QueryAllocatedBlocks,
+ "handle, %" PRIu64 " sectors, %" PRIu64 " sectors, "
+ "%d sectors",
+ start_sector, nr_sectors, VIXDISKLIB_MIN_CHUNK_SIZE)
+ err = VixDiskLib_QueryAllocatedBlocks (h->handle,
+ start_sector, nr_sectors,
+ VIXDISKLIB_MIN_CHUNK_SIZE,
+ &block_list);
+ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0);
+ if (err != VIX_OK) {
+ VDDK_ERROR (err, "VixDiskLib_QueryAllocatedBlocks");
+ nbdkit_extents_free (extents);
+ return -1;
+ }
+
+ for (i = 0; i < block_list->numBlocks; ++i) {
+ uint64_t blk_offset, blk_length;
+
+ blk_offset = block_list->blocks[i].offset * VIXDISKLIB_SECTOR_SIZE;
+ blk_length = block_list->blocks[i].length * VIXDISKLIB_SECTOR_SIZE;
+
+ /* The query returns allocated blocks. We must insert holes
+ * between the blocks as necessary.
+ */
+ if ((position < blk_offset &&
+ add_extent (extents, &position, blk_offset, true) == -1) ||
+ (add_extent (extents,
+ &position, blk_offset + blk_length, false) == -1)) {
+ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list")
+ VixDiskLib_FreeBlockList (block_list);
+ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0);
+ nbdkit_extents_free (extents);
+ return -1;
+ }
+ }
+ VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list")
+ VixDiskLib_FreeBlockList (block_list);
+ VDDK_CALL_END (VixDiskLib_FreeBlockList, 0);
+
+ /* There's an implicit hole after the returned list of blocks,
+ * up to the end of the QueryAllocatedBlocks request.
+ */
+ if (add_extent (extents,
+ &position,
+ (start_sector + nr_sectors) * VIXDISKLIB_SECTOR_SIZE,
+ true) == -1) {
+ nbdkit_extents_free (extents);
+ return -1;
+ }
+
+ start_sector += nr_sectors;
+ nr_chunks_remaining -= nr_chunks;
+ }
+
+ /* Add the allocated unaligned bit at the end. */
+ if (position < h->size) {
+ if (add_extent (extents, &position, h->size, false) == -1) {
+ nbdkit_extents_free (extents);
+ return -1;
+ }
+ }
+
+ /* Save the pre-cached extents in the handle. */
+ h->extents = extents;
+ return 0;
+}
+
+static int
+get_extents_from_cache (struct command *cmd, struct vddk_handle *h)
+{
+ struct nbdkit_extents *rextents = cmd->ptr;
+ struct nbdkit_extent e;
+ size_t i;
+
+ /* We can just copy from the pre-cached extents in the handle which
+ * cover the entire disk, into the returned extents, because
+ * nbdkit_add_extent does the right thing.
+ */
+ for (i = 0; i < nbdkit_extents_count (h->extents); ++i) {
+ e = nbdkit_get_extent (h->extents, i);
+ if (nbdkit_add_extent (rextents, e.offset, e.length, e.type) == -1)
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Handle extents.
+ *
+ * Oh QueryAllocatedBlocks, how much I hate you. The API has two
+ * enormous problems: (a) It's slow, taking about 1 second per
+ * invocation regardless of how much or little data you request. (b)
+ * It serialises all other requests to the disk, like concurrent
+ * reads.
+ *
+ * NBD / nbdkit doesn't help much either by having a 4GB - 1 byte
+ * limit on the size of extent requests. This means that for each 4GB
+ * of disk, we will need to run QueryAllocatedBlocks twice. For a 1TB
+ * virtual disk, about 500 seconds would be used directly in the API
+ * calls, and much more time is lost because of serialization.
+ *
+ * To work around these problems, in the readonly case (used by
+ * virt-v2v), when the first NBD_BLOCK_STATUS request is received, we
+ * will read over the whole disk and cache the extents. We will read
+ * in 32 GB chunks (the maximum possible for the underlying
+ * QueryAllocatedBlocks API). For a 1TB disk this will take ~ 30
+ * seconds, but avoids all the overheads above. The cached extents
+ * are stored in the handle, and subsequent NBD_BLOCK_STATUS will use
+ * the cache only.
+ *
+ * For writable disks we can't easily do any caching so don't attempt
+ * it.
+ */
+static int
+do_extents (struct command *cmd, struct vddk_handle *h)
+{
+ if (h->readonly && !h->extents) {
+ time_t start_t, end_t;
+
+ time (&start_t);
+ nbdkit_debug ("vddk: pre-caching extents");
+
+ if (pre_cache_extents (h) == -1)
+ return -1;
+
+ time (&end_t);
+ nbdkit_debug ("vddk: finished pre-caching extents in %d second(s)",
+ (int) (end_t - start_t));
+ }
+
+ if (h->extents)
+ return get_extents_from_cache (cmd, h);
+ else
+ return get_extents_slow (cmd, h);
+}
+
/* Background worker thread, one per connection, which is where the
* VDDK commands are issued.
*/
--
2.47.1

263
0007-file-Save-the-filename-or-equivalent-in-the-file-han.patch
View File

@ -1,263 +0,0 @@
From f048b5dd411ab4ada0f4b57fb27fee84908a6d7f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 4 Jun 2025 21:00:47 +0100
Subject: [PATCH] file: Save the filename or equivalent in the file handle
This will allow us to improve error messages.
To make this change easier by reducing duplication, I also implemented
a common error path in the file_open function.
(cherry picked from commit 83ad77d28d598e4b85809e7225e754e64314f802)
---
plugins/file/file.c | 114 +++++++++++++++++++++++++-------------------
1 file changed, 65 insertions(+), 49 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 94adc43d..13898af1 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -500,6 +500,7 @@ file_list_exports (int readonly, int default_only,
/* The per-connection handle. */
struct handle {
int fd;
+ char *name; /* Filename or equivalent. */
struct stat statbuf;
bool is_block_device;
int sector_size;
@@ -557,7 +558,6 @@ static void *
file_open (int readonly)
{
struct handle *h;
- const char *file;
h = malloc (sizeof *h);
if (h == NULL) {
@@ -566,36 +566,44 @@ file_open (int readonly)
}
h->can_write = !readonly;
h->fd = -1;
+ h->name = NULL;
switch (mode) {
case mode_filename:
- file = filename;
- if (open_file_by_name (h, readonly, -1, file) == -1) {
- free (h);
- return NULL;
+ h->name = strdup (filename);
+ if (h->name == NULL) {
+ nbdkit_error ("strdup: %m");
+ goto err;
}
+ if (open_file_by_name (h, readonly, -1, h->name) == -1)
+ goto err;
break;
case mode_directory: {
int dfd;
+ const char *export_name = nbdkit_export_name ();
- file = nbdkit_export_name ();
- if (strchr (file, '/')) {
- nbdkit_error ("exportname cannot contain /");
- free (h);
+ if (export_name == NULL)
+ goto err;
+
+ h->name = strdup (export_name);
+ if (h->name == NULL) {
+ nbdkit_error ("strdup: %m");
+ goto err;
+ }
+ if (strchr (h->name, '/')) {
+ nbdkit_error ("%s: exportname cannot contain /", h->name);
errno = EINVAL;
- return NULL;
+ goto err;
}
dfd = open (directory, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
if (dfd == -1) {
nbdkit_error ("open %s: %m", directory);
- free (h);
- return NULL;
+ goto err;
}
- if (open_file_by_name (h, readonly, dfd, file) == -1) {
- free (h);
+ if (open_file_by_name (h, readonly, dfd, h->name) == -1) {
close (dfd);
- return NULL;
+ goto err;
}
close (dfd);
break;
@@ -604,14 +612,14 @@ file_open (int readonly)
case mode_fd: {
int r;
- /* This is needed for error messages. */
- file = "<file descriptor>";
-
+ if (asprintf (&h->name, "fd=%d", filedesc) == -1) {
+ nbdkit_error ("asprintf: %m");
+ goto err;
+ }
h->fd = dup (filedesc);
if (h->fd == -1) {
- nbdkit_error ("dup fd=%d: %m", filedesc);
- free (h);
- return NULL;
+ nbdkit_error ("dup: %s: %m", h->name);
+ goto err;
}
/* If the file descriptor is readonly then we should not advertise
@@ -619,10 +627,8 @@ file_open (int readonly)
*/
r = fcntl (h->fd, F_GETFL);
if (r == -1) {
- nbdkit_error ("fcntl: F_GETFL: %m");
- close (h->fd);
- free (h);
- return NULL;
+ nbdkit_error ("fcntl: %s: F_GETFL: %m", h->name);
+ goto err;
}
r &= O_ACCMODE;
if (r == O_RDONLY)
@@ -636,25 +642,30 @@ file_open (int readonly)
case mode_dirfd: {
int dfd;
+ const char *export_name = nbdkit_export_name ();
- file = nbdkit_export_name ();
- if (strchr (file, '/')) {
- nbdkit_error ("exportname cannot contain /");
- free (h);
+ if (export_name == NULL)
+ goto err;
+
+ h->name = strdup (export_name);
+ if (h->name == NULL) {
+ nbdkit_error ("strdup: %m");
+ goto err;
+ }
+ if (strchr (h->name, '/')) {
+ nbdkit_error ("%s: exportname cannot contain /", h->name);
errno = EINVAL;
- return NULL;
+ goto err;
}
/* We don't fork, so no need to worry about FD_CLOEXEC on the directory */
dfd = dup (filedesc);
if (dfd == -1) {
- nbdkit_error ("dup dirfd=%d: %m", filedesc);
- free (h);
- return NULL;
+ nbdkit_error ("dup: dirfd=%d: %m", filedesc);
+ goto err;
}
- if (open_file_by_name (h, readonly, dfd, file) == -1) {
- free (h);
+ if (open_file_by_name (h, readonly, dfd, h->name) == -1) {
close (dfd);
- return NULL;
+ goto err;
}
close (dfd);
break;
@@ -665,12 +676,11 @@ file_open (int readonly)
}
assert (h->fd >= 0);
+ assert (h->name != NULL);
if (fstat (h->fd, &h->statbuf) == -1) {
- nbdkit_error ("fstat: %s: %m", file);
- close (h->fd);
- free (h);
- return NULL;
+ nbdkit_error ("fstat: %s: %m", h->name);
+ goto err;
}
if (fadvise_mode != -1) {
@@ -678,7 +688,7 @@ file_open (int readonly)
#ifdef HAVE_POSIX_FADVISE
int r = posix_fadvise (h->fd, 0, 0, fadvise_mode);
if (r == -1)
- nbdkit_debug ("posix_fadvise: %s: %m (ignored)", file);
+ nbdkit_debug ("posix_fadvise: %s: %m (ignored)", h->name);
#else
nbdkit_debug ("fadvise is not supported");
#endif
@@ -689,17 +699,15 @@ file_open (int readonly)
else if (S_ISREG (h->statbuf.st_mode))
h->is_block_device = false;
else {
- nbdkit_error ("file is not regular or block device: %s", file);
- close (h->fd);
- free (h);
- return NULL;
+ nbdkit_error ("file is not regular or block device: %s", h->name);
+ goto err;
}
h->sector_size = 4096; /* Start with safe guess */
#ifdef BLKSSZGET
if (h->is_block_device) {
if (ioctl (h->fd, BLKSSZGET, &h->sector_size) == -1)
- nbdkit_debug ("cannot get sector size: %s: %m", file);
+ nbdkit_debug ("cannot get sector size: %s: %m", h->name);
}
#endif
@@ -707,7 +715,7 @@ file_open (int readonly)
#ifdef BLKROTATIONAL
if (h->is_block_device) {
if (ioctl (h->fd, BLKROTATIONAL, &h->rotational) == -1)
- nbdkit_debug ("cannot get rotational property: %s: %m", file);
+ nbdkit_debug ("cannot get rotational property: %s: %m", h->name);
}
#endif
@@ -717,10 +725,10 @@ file_open (int readonly)
unsigned int minimum_io_size = 0, optimal_io_size = 0;
if (ioctl (h->fd, BLKIOMIN, &minimum_io_size) == -1)
- nbdkit_debug ("cannot get BLKIOMIN: %s: %m", file);
+ nbdkit_debug ("cannot get BLKIOMIN: %s: %m", h->name);
if (ioctl (h->fd, BLKIOOPT, &optimal_io_size) == -1)
- nbdkit_debug ("cannot get BLKIOOPT: %s: %m", file);
+ nbdkit_debug ("cannot get BLKIOOPT: %s: %m", h->name);
else if (optimal_io_size == 0)
/* All devices in the Linux kernel except for MD report optimal
* as 0. In that case guess a good value.
@@ -754,6 +762,13 @@ file_open (int readonly)
h->can_blkdiscard = h->is_block_device;
return h;
+
+ err:
+ if (h->fd >= 0)
+ close (h->fd);
+ free (h->name);
+ free (h);
+ return NULL;
}
/* Free up the per-connection handle. */
@@ -766,6 +781,7 @@ file_close (void *handle)
remove_fd_from_window (h->fd);
#endif
close (h->fd);
+ free (h->name);
free (h);
}
--
2.47.1

143
0008-file-Add-the-filename-or-equivalent-to-error-message.patch
View File

@ -1,143 +0,0 @@
From 33e8a5b15a4edf5ea03d3f339e1580855c746fcf Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 4 Jun 2025 21:03:58 +0100
Subject: [PATCH] file: Add the filename (or equivalent) to error messages
It helps to have this information when there's an error. It
particularly helps in two cases: (1) You don't have access to the
nbdkit command line. (2) The filename is derived from an export name
passed by the client.
(cherry picked from commit 7273bd829fc0d8e492786f8908d8ddc3d5399e06)
---
plugins/file/file.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 13898af1..2bba7e77 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -797,7 +797,7 @@ file_get_size (void *handle)
r = device_size (h->fd, &h->statbuf);
if (r == -1) {
- nbdkit_error ("device_size: %m");
+ nbdkit_error ("device_size: %s: %m", h->name);
return -1;
}
return r;
@@ -883,7 +883,7 @@ file_flush (void *handle, uint32_t flags)
struct handle *h = handle;
if (fdatasync (h->fd) == -1) {
- nbdkit_error ("fdatasync: %m");
+ nbdkit_error ("fdatasync: %s: %m", h->name);
return -1;
}
@@ -904,11 +904,11 @@ file_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pread (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pread: %m");
+ nbdkit_error ("pread: %s: %m", h->name);
return -1;
}
if (r == 0) {
- nbdkit_error ("pread: unexpected end of file");
+ nbdkit_error ("pread: %s: unexpected end of file", h->name);
return -1;
}
buf += r;
@@ -939,7 +939,7 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pwrite (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pwrite: %m");
+ nbdkit_error ("pwrite: %s: %m", h->name);
return -1;
}
buf += r;
@@ -1021,7 +1021,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1057,7 +1057,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1065,7 +1065,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1087,7 +1087,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1115,14 +1115,14 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
h->can_fallocate = false;
} else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1146,7 +1146,7 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (errno != ENOTTY) {
- nbdkit_error ("zero: %m");
+ nbdkit_error ("zero: %s: %m", h->name);
return -1;
}
@@ -1183,7 +1183,7 @@ file_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
offset, count);
if (r == -1) {
if (!is_enotsup (errno)) {
- nbdkit_error ("fallocate: %m");
+ nbdkit_error ("fallocate: %s: %m", h->name);
return -1;
}
@@ -1322,7 +1322,7 @@ file_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
r = posix_fadvise (h->fd, offset, count, POSIX_FADV_WILLNEED);
if (r) {
errno = r;
- nbdkit_error ("posix_fadvise: %m");
+ nbdkit_error ("posix_fadvise: %s: %m", h->name);
return -1;
}
return 0;
--
2.47.1

132
0009-file-Add-offset-count-to-error-messages.patch
View File

@ -1,132 +0,0 @@
From 84362587e5bb53e113c65d9876cf5b3c39174bab Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 4 Jun 2025 21:15:26 +0100
Subject: [PATCH] file: Add offset/count to error messages
Although the same information is available in debug logs, if verbose
messages (-v) are not used then the information is lost. As it might
be useful occasionally, ensure it is captured in errors.
(cherry picked from commit 253574354252f4a77e2df0769b721e76f1777651)
---
plugins/file/file.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 2bba7e77..ef40b400 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -904,7 +904,8 @@ file_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pread (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pread: %s: %m", h->name);
+ nbdkit_error ("pread: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
if (r == 0) {
@@ -939,7 +940,8 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
while (count > 0) {
ssize_t r = pwrite (h->fd, buf, count, offset);
if (r == -1) {
- nbdkit_error ("pwrite: %s: %m", h->name);
+ nbdkit_error ("pwrite: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
buf += r;
@@ -1021,7 +1023,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1057,7 +1060,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1065,7 +1069,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1087,7 +1092,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1115,14 +1121,16 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
h->can_fallocate = false;
} else {
if (!is_enotsup (errno)) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ":%m",
+ h->name, offset, count);
return -1;
}
@@ -1146,7 +1154,8 @@ file_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
}
if (errno != ENOTTY) {
- nbdkit_error ("zero: %s: %m", h->name);
+ nbdkit_error ("zero: %s: offset=%" PRIu64 ", count=%" PRIu32 ": %m",
+ h->name, offset, count);
return -1;
}
@@ -1183,7 +1192,9 @@ file_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
offset, count);
if (r == -1) {
if (!is_enotsup (errno)) {
- nbdkit_error ("fallocate: %s: %m", h->name);
+ nbdkit_error ("fallocate: %s: offset=%" PRIu64 ", count=%" PRIu32 ":"
+ " %m",
+ h->name, offset, count);
return -1;
}
@@ -1322,7 +1333,9 @@ file_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags)
r = posix_fadvise (h->fd, offset, count, POSIX_FADV_WILLNEED);
if (r) {
errno = r;
- nbdkit_error ("posix_fadvise: %s: %m", h->name);
+ nbdkit_error ("posix_fadvise: %s: offset=%" PRIu64 ", count=%" PRIu32 ":"
+ " %m",
+ h->name, offset, count);
return -1;
}
return 0;
--
2.47.1

61
0010-vddk-stats-Record-the-byte-count-of-each-QueryAlloca.patch
View File

@ -1,61 +0,0 @@
From f6cbb39d3db9b138b98e9d5af3f50e430520302f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sun, 8 Jun 2025 10:40:26 +0100
Subject: [PATCH] vddk: stats: Record the byte count of each
QueryAllocatedBlocks call
In -D vddk.stats=1 output, keep track of the size of each call to
QueryAllocatedBlocks, and display that at the end.
After this change, we see the number of bytes scanned in the "bytes"
column (previously this column was empty for this call):
nbdkit: debug: VixDiskLib_... us calls bytes
nbdkit: debug: Open 27051497 2
nbdkit: debug: GetInfo 12538428 4
nbdkit: debug: ConnectEx 6944819 2
nbdkit: debug: QueryAllocatedBlocks 4563503 3 21474967552
nbdkit: debug: Close 1440271 2
nbdkit: debug: Exit 1001407 1
(cherry picked from commit 2db1ede27bb529b36b0075eab337f0c585d1a7ec)
---
plugins/vddk/worker.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/plugins/vddk/worker.c b/plugins/vddk/worker.c
index 27f56e37..bb088418 100644
--- a/plugins/vddk/worker.c
+++ b/plugins/vddk/worker.c
@@ -327,7 +327,8 @@ test_can_extents (struct vddk_handle *h)
0, VIXDISKLIB_MIN_CHUNK_SIZE,
VIXDISKLIB_MIN_CHUNK_SIZE,
&block_list);
- VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0);
+ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks,
+ VIXDISKLIB_MIN_CHUNK_SIZE * VIXDISKLIB_SECTOR_SIZE);
error_suppression = 0;
if (err == VIX_OK) {
VDDK_CALL_START (VixDiskLib_FreeBlockList, "block_list")
@@ -435,7 +436,8 @@ get_extents_slow (struct command *cmd, struct vddk_handle *h)
start_sector, nr_sectors,
VIXDISKLIB_MIN_CHUNK_SIZE,
&block_list);
- VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0);
+ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks,
+ nr_sectors * VIXDISKLIB_SECTOR_SIZE);
if (err != VIX_OK) {
VDDK_ERROR (err, "VixDiskLib_QueryAllocatedBlocks");
return -1;
@@ -535,7 +537,8 @@ pre_cache_extents (struct vddk_handle *h)
start_sector, nr_sectors,
VIXDISKLIB_MIN_CHUNK_SIZE,
&block_list);
- VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks, 0);
+ VDDK_CALL_END (VixDiskLib_QueryAllocatedBlocks,
+ nr_sectors * VIXDISKLIB_SECTOR_SIZE);
if (err != VIX_OK) {
VDDK_ERROR (err, "VixDiskLib_QueryAllocatedBlocks");
nbdkit_extents_free (extents);
--
2.47.1

178
0011-vddk-stats-Collect-elapsed-time-for-ReadAsync-and-Wr.patch
View File

@ -1,178 +0,0 @@
From bd7a0bbbdc36844662f4018d37f05c40452c6763 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sun, 8 Jun 2025 10:59:57 +0100
Subject: [PATCH] vddk: stats: Collect elapsed time for ReadAsync and
WriteAsync
A major existing problem with -D vddk.stats=1 is that it did not
accurately account for the time taken in these asynchronous calls.
This change records the elapsed time. (It's not possible to collect
the actual time spent "working on" these calls, we just add up the
total elapsed time, which is fine.)
In the remote case, here's how the output changes from before this
change:
nbdkit: debug: VixDiskLib_... us calls bytes
nbdkit: debug: ReadAsync 18 1 8192
to after this change:
nbdkit: debug: VixDiskLib_... us calls bytes
nbdkit: debug: ReadAsync 243745 1 8192
An interesting thing about this is that in the local case this
actually reduces the apparent elapsed time, I think because VDDK calls
complete_command directly from the Read/WriteAsync function. It only
increases the apparent elapsed time in the remote case.
(cherry picked from commit a224d7fd8d839287c63cb4a063569283dd974b48)
---
plugins/vddk/nbdkit-vddk-plugin.pod | 3 +--
plugins/vddk/vddk.h | 37 ++++++++++++++++++++++-------
plugins/vddk/worker.c | 13 ++++++++--
3 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/plugins/vddk/nbdkit-vddk-plugin.pod b/plugins/vddk/nbdkit-vddk-plugin.pod
index 7cf938de..18486d75 100644
--- a/plugins/vddk/nbdkit-vddk-plugin.pod
+++ b/plugins/vddk/nbdkit-vddk-plugin.pod
@@ -723,8 +723,7 @@ Same as above, but for writing and flushing writes.
=item C<WriteAsync>
Same as above, but for asynchronous read and write calls introduced in
-nbdkit 1.30. Unfortunately at the moment the amount of time spent in
-these calls is not accounted for correctly.
+S<nbdkit 1.30>.
=item C<QueryAllocatedBlocks>
diff --git a/plugins/vddk/vddk.h b/plugins/vddk/vddk.h
index 461fb528..5ff02649 100644
--- a/plugins/vddk/vddk.h
+++ b/plugins/vddk/vddk.h
@@ -39,6 +39,7 @@
#include <pthread.h>
+#include "cleanup.h"
#include "isaligned.h"
#include "tvdiff.h"
#include "vector.h"
@@ -91,7 +92,7 @@ extern int vddk_debug_stats;
*/
#define VDDK_CALL_START(fn, fs, ...) \
do { \
- struct timeval start_t, end_t; \
+ struct timeval start_t; \
/* GCC can optimize this away at compile time: */ \
const bool datapath = \
strcmp (#fn, "VixDiskLib_Read") == 0 || \
@@ -105,13 +106,13 @@ extern int vddk_debug_stats;
do
#define VDDK_CALL_END(fn, bytes_) \
while (0); \
- if (vddk_debug_stats) { \
- gettimeofday (&end_t, NULL); \
- ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock); \
- stats_##fn.usecs += tvdiff_usec (&start_t, &end_t); \
- stats_##fn.calls++; \
- stats_##fn.bytes += bytes_; \
- } \
+ update_stats (&start_t, bytes_, &stats_##fn); \
+ } while (0)
+/* Same as VDDK_CALL_END, but for {Read,Write}Async, where we will
+ * update the stats ourself.
+ */
+#define VDDK_CALL_END_ASYNC() \
+ while (0); \
} while (0)
/* Print VDDK errors. */
@@ -141,6 +142,7 @@ struct command {
uint64_t id; /* serial number */
/* These fields are used by the internal implementation. */
+ struct timeval start_t; /* start time */
pthread_mutex_t mutex; /* completion mutex */
pthread_cond_t cond; /* completion condition */
enum { SUBMITTED, SUCCEEDED, FAILED } status;
@@ -197,6 +199,25 @@ extern pthread_mutex_t stats_lock;
#undef OPTIONAL_STUB
extern void display_stats (void);
+static inline void
+update_stats (const struct timeval *start_t, uint64_t bytes,
+ struct vddk_stat *stat)
+{
+ if (vddk_debug_stats) {
+ struct timeval end_t;
+ int64_t usecs;
+
+ gettimeofday (&end_t, NULL);
+ usecs = tvdiff_usec (start_t, &end_t);
+
+ /* Keep this critical section as small as possible. */
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&stats_lock);
+ stat->usecs += usecs;
+ stat->calls++;
+ stat->bytes += bytes;
+ }
+}
+
/* utils.c */
extern void trim (char *str);
diff --git a/plugins/vddk/worker.c b/plugins/vddk/worker.c
index bb088418..d26574ef 100644
--- a/plugins/vddk/worker.c
+++ b/plugins/vddk/worker.c
@@ -120,6 +120,11 @@ complete_command (void *vp, VixError result)
nbdkit_debug ("command %" PRIu64 " (%s) completed",
cmd->id, command_type_string (cmd->type));
+ /* Update the stats for this asynchronous call. */
+ update_stats (&cmd->start_t, cmd->count,
+ cmd->type == READ ? &stats_VixDiskLib_ReadAsync :
+ &stats_VixDiskLib_WriteAsync);
+
ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&cmd->mutex);
if (result == VIX_OK) {
@@ -219,13 +224,15 @@ do_read (struct command *cmd, struct vddk_handle *h)
offset /= VIXDISKLIB_SECTOR_SIZE;
count /= VIXDISKLIB_SECTOR_SIZE;
+ gettimeofday (&cmd->start_t, NULL);
+
VDDK_CALL_START (VixDiskLib_ReadAsync,
"handle, %" PRIu64 " sectors, "
"%" PRIu32 " sectors, buffer, callback, %" PRIu64,
offset, count, cmd->id)
err = VixDiskLib_ReadAsync (h->handle, offset, count, buf,
complete_command, cmd);
- VDDK_CALL_END (VixDiskLib_ReadAsync, count * VIXDISKLIB_SECTOR_SIZE);
+ VDDK_CALL_END_ASYNC ();
if (err != VIX_ASYNC) {
VDDK_ERROR (err, "VixDiskLib_ReadAsync");
return -1;
@@ -256,13 +263,15 @@ do_write (struct command *cmd, struct vddk_handle *h)
offset /= VIXDISKLIB_SECTOR_SIZE;
count /= VIXDISKLIB_SECTOR_SIZE;
+ gettimeofday (&cmd->start_t, NULL);
+
VDDK_CALL_START (VixDiskLib_WriteAsync,
"handle, %" PRIu64 " sectors, "
"%" PRIu32 " sectors, buffer, callback, %" PRIu64,
offset, count, cmd->id)
err = VixDiskLib_WriteAsync (h->handle, offset, count, buf,
complete_command, cmd);
- VDDK_CALL_END (VixDiskLib_WriteAsync, count * VIXDISKLIB_SECTOR_SIZE);
+ VDDK_CALL_END_ASYNC ();
if (err != VIX_ASYNC) {
VDDK_ERROR (err, "VixDiskLib_WriteAsync");
return -1;
--
2.47.1

54
nbdkit.spec
View File

@ -51,10 +51,10 @@
%global verify_tarball_signature 1
# The source directory.
%global source_directory 1.42-stable
%global source_directory 1.44-stable
Name: nbdkit
Version: 1.42.5
Version: 1.44.0
Release: 1%{?dist}
Summary: NBD server
@ -80,17 +80,7 @@ Source3: copy-patches.sh
# https://gitlab.com/nbdkit/nbdkit/-/commits/rhel-10.1/
# Patches.
Patch0001: 0001-tests-Add-tests-of-file-plugin-cache-none.patch
Patch0002: 0002-tests-Add-more-generic-tests-of-file-cache-none.patch
Patch0003: 0003-vddk-Cache-the-readonly-flag-from-the-.open-call-in-.patch
Patch0004: 0004-vddk-Move-minimum-version-of-VDDK-to-6.7.patch
Patch0005: 0005-vddk-Unconditionally-test-QueryAllocatedBlocks.patch
Patch0006: 0006-vddk-Pre-cache-the-extents-for-readonly-connections.patch
Patch0007: 0007-file-Save-the-filename-or-equivalent-in-the-file-han.patch
Patch0008: 0008-file-Add-the-filename-or-equivalent-to-error-message.patch
Patch0009: 0009-file-Add-offset-count-to-error-messages.patch
Patch0010: 0010-vddk-stats-Record-the-byte-count-of-each-QueryAlloca.patch
Patch0011: 0011-vddk-stats-Collect-elapsed-time-for-ReadAsync-and-Wr.patch
#(none)
# For automatic RPM Provides generation.
# See: https://rpm-software-management.github.io/rpm/manual/dependency_generators.html
@ -127,6 +117,10 @@ BuildRequires: pkgconfig(bzip2)
BuildRequires: pkgconfig(libzstd)
BuildRequires: pkgconfig(libcurl)
BuildRequires: pkgconfig(libnbd)
%if !0%{?rhel}
# We require libnfs >= 6, but the internal version is >= 16
BuildRequires: pkgconfig(libnfs) >= 16
%endif
BuildRequires: pkgconfig(libssh)
BuildRequires: e2fsprogs
BuildRequires: pkgconfig(ext2fs)
@ -451,6 +445,16 @@ This package lets you forward NBD connections from %{name}
to another NBD server.
%if !0%{?rhel}
%package nfs-plugin
Summary: NFS (Network File Server) plugin for %{name}
Requires: %{name}-server%{?_isa} = %{version}-%{release}
%description nfs-plugin
This package contains Network File Server (NFS) support for %{name}.
%endif
%if !0%{?rhel} && 0%{?have_ocaml}
%package ocaml-plugin
Summary: OCaml plugin for %{name}
@ -570,7 +574,7 @@ This package is a BitTorrent plugin for %{name}.
Summary: VMware VDDK plugin for %{name}
Requires: %{name}-server%{?_isa} = %{version}-%{release}
# https://bugzilla.redhat.com/show_bug.cgi?id=1931818
Requires: libxcrypt-compat
Requires: libxcrypt-compat%{?_isa}
%description vddk-plugin
This package is a plugin for %{name} which connects to
@ -643,6 +647,8 @@ nbdkit-nozero-filter Adjust handling of zero requests by plugins.
nbdkit-offset-filter Serve an offset and range.
nbdkit-openonce-filter Open the underlying plugin once.
nbdkit-partition-filter Serve a single partition.
nbdkit-pause-filter Pause NBD requests.
@ -763,7 +769,7 @@ BuildRequires: selinux-policy-devel
%{?selinux_requires}
%description selinux
%{nbdkit} SELinux policy module.
%{name} SELinux policy module.
%endif
@ -1239,6 +1245,15 @@ fi
%{_mandir}/man1/nbdkit-nbd-plugin.1*
%if !0%{?rhel}
%files nfs-plugin
%doc README.md
%license LICENSE
%{_libdir}/%{name}/plugins/nbdkit-nfs-plugin.so
%{_mandir}/man1/nbdkit-nfs-plugin.1*
%endif
%if !0%{?rhel} && 0%{?have_ocaml}
%files ocaml-plugin
%doc README.md
@ -1356,6 +1371,7 @@ fi
%{_libdir}/%{name}/filters/nbdkit-noparallel-filter.so
%{_libdir}/%{name}/filters/nbdkit-nozero-filter.so
%{_libdir}/%{name}/filters/nbdkit-offset-filter.so
%{_libdir}/%{name}/filters/nbdkit-openonce-filter.so
%{_libdir}/%{name}/filters/nbdkit-partition-filter.so
%{_libdir}/%{name}/filters/nbdkit-pause-filter.so
%{_libdir}/%{name}/filters/nbdkit-protect-filter.so
@ -1401,6 +1417,7 @@ fi
%{_mandir}/man1/nbdkit-noparallel-filter.1*
%{_mandir}/man1/nbdkit-nozero-filter.1*
%{_mandir}/man1/nbdkit-offset-filter.1*
%{_mandir}/man1/nbdkit-openonce-filter.1*
%{_mandir}/man1/nbdkit-partition-filter.1*
%{_mandir}/man1/nbdkit-pause-filter.1*
%{_mandir}/man1/nbdkit-protect-filter.1*
@ -1489,6 +1506,7 @@ fi
%{_mandir}/man3/nbdkit-plugin.3*
%{_mandir}/man3/nbdkit_*.3*
%{_mandir}/man1/nbdkit-release-notes-1.*.1*
%{_mandir}/man3/nbdkit-tracing.3*
%{_libdir}/pkgconfig/nbdkit.pc
@ -1533,9 +1551,9 @@ fi
%changelog
* Mon Jun 23 2025 Richard W.M. Jones <rjones@redhat.com> - 1.42.5-1
- Rebase to nbdkit 1.42.5
resolves: RHEL-78830
* Mon Jun 23 2025 Richard W.M. Jones <rjones@redhat.com> - 1.44.0-1
- Rebase to nbdkit 1.44.0
resolves: RHEL-78830, RHEL-101180
- Synch the spec file with Fedora Rawhide.
- nbdkit-ondemand-plugin moves into a new subpackage.
- New nbdkit-time-limit-filter.

4
sources
View File

@ -1,2 +1,2 @@
SHA512 (nbdkit-1.42.5.tar.gz) = 71cb0a287430b451aa200b0be5f440077c37b42c060db6c5ccbff34c2e6971c6fc411c0ff4921c8cb16b39c6091da24667524dccff5c578d3103a4ca9972e4d5
SHA512 (nbdkit-1.42.5.tar.gz.sig) = bb2df86013951b86da60e65d313954ba76d72d82534f9fcff6d2808d40480ce81fbc8b65bc609155b3a8518bdeabb69ee4190b2e3668f52e8b40fc49027b7954
SHA512 (nbdkit-1.44.0.tar.gz) = 87d91a8203fadd96829888d0fe60e0d8c06de2eb2fcc715e710ebf6709befa176bb96d316e4ad8412ea6d8da46f61807d75f6e22eaa8ce986e2341056e4220f5
SHA512 (nbdkit-1.44.0.tar.gz.sig) = c21fc3c8c51fe89eaec9f979d2a6e61c86568302bad744669e116c26450d7d0e8c82477d7e7bb7d82980c1b887fcf40b6609eae5896396bd70a14a2885ebe7bd