Re: [Libguestfs] [nbdkit PATCH] nozero: Add notrim mode

It may be useful to test whether the client's use of NBD_CMD_FLAG_NO_HOLE makes a difference; do this by adding a mode to --filter=nozero to force a non-trimming zero write. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/nozero/nbdkit-nozero-filter.pod | 19 +++++++---- filters/nozero/nozero.c | 45 ++++++++++++++++++++----- tests/test-nozero.sh | 27 +++++++++++++-- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/filters/nozero/nbdkit-nozero-filter.pod b/filters/nozero/nbdkit-nozero-filter.pod index 7e06570..8e694bb 100644 --- a/filters/nozero/nbdkit-nozero-filter.pod +++ b/filters/nozero/nbdkit-nozero-filter.pod @@ -18,14 +18,19 @@ testing client or server fallbacks. =over 4 -=item B<zeromode=none|emulate> +=item B<zeromode=none|emulate|notrim> Optional, controls which mode the filter will use. Mode B<none> -(default) means that zero support is not advertised to the client; -mode B<emulate> means that zero support is emulated by the filter -using the plugin's C<pwrite> callback, regardless of whether the -plugin itself implemented the C<zero> callback with a more efficient -way to write zeroes. +(default) means that zero support is not advertised to the +client. Mode B<emulate> means that zero support is emulated by the +filter using the plugin's C<pwrite> callback, regardless of whether +the plugin itself implemented the C<zero> callback with a more +efficient way to write zeros. Since nbdkit E<ge> 1.13.4, mode +B<notrim> means that zero requests are forwarded on to the plugin, +except that the plugin will never see the NBDKIT_MAY_TRIM flag, to +determine if the client permitting trimming during zero operations +makes a difference (it is an error to request this mode if the plugin +does not support the C<zero> callback). =back @@ -55,4 +60,4 @@ Eric Blake =head1 COPYRIGHT -Copyright (C) 2018 Red Hat Inc. +Copyright (C) 2018-2019 Red Hat Inc. diff --git a/filters/nozero/nozero.c b/filters/nozero/nozero.c index 3ec6346..6e0ffa9 100644 --- a/filters/nozero/nozero.c +++ b/filters/nozero/nozero.c @@ -47,8 +47,12 @@ #define MAX_WRITE (64 * 1024 * 1024) -static char buffer[MAX_WRITE]; -static bool emulate; +static const char buffer[MAX_WRITE]; +static enum ZeroMode { + NONE, + EMULATE, + NOTRIM, +} zeromode; static int nozero_config (nbdkit_next_config *next, void *nxdata, @@ -56,7 +60,9 @@ nozero_config (nbdkit_next_config *next, void *nxdata, { if (strcmp (key, "zeromode") == 0) { if (strcmp (value, "emulate") == 0) - emulate = true; + zeromode = EMULATE; + else if (strcmp (value, "notrim") == 0) + zeromode = NOTRIM; else if (strcmp (value, "none") != 0) { nbdkit_error ("unknown zeromode '%s'", value); return -1; @@ -67,13 +73,31 @@ nozero_config (nbdkit_next_config *next, void *nxdata, } #define nozero_config_help \ - "zeromode=<MODE> Either 'none' (default) or 'emulate'.\n" \ + "zeromode=<MODE> Either 'none' (default), 'emulate', or 'notrim'.\n" \ + +/* Check that desired mode is supported by plugin. */ +static int +nozero_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) +{ + int r; + + if (zeromode == NOTRIM) { + r = next_ops->can_zero (nxdata); + if (r == -1) + return -1; + if (!r) { + nbdkit_error ("zeromode 'notrim' requires plugin zero support"); + return -1; + } + } + return 0; +} /* Advertise desired WRITE_ZEROES mode. */ static int nozero_can_zero (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) { - return emulate; + return zeromode != NONE; } static int @@ -81,11 +105,15 @@ nozero_zero (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offs, uint32_t flags, int *err) { - assert (emulate); + assert (zeromode != NONE); + flags &= ~NBDKIT_FLAG_MAY_TRIM; + + if (zeromode == NOTRIM) + return next_ops->zero (nxdata, count, offs, flags, err); + while (count) { uint32_t size = MIN (count, MAX_WRITE); - if (next_ops->pwrite (nxdata, buffer, size, offs, - flags & ~NBDKIT_FLAG_MAY_TRIM, err) == -1) + if (next_ops->pwrite (nxdata, buffer, size, offs, flags, err) == -1) return -1; offs += size; count -= size; @@ -99,6 +127,7 @@ static struct nbdkit_filter filter = { .version = PACKAGE_VERSION, .config = nozero_config, .config_help = nozero_config_help, + .prepare = nozero_prepare, .can_zero = nozero_can_zero, .zero = nozero_zero, }; diff --git a/tests/test-nozero.sh b/tests/test-nozero.sh index 1282586..fc22420 100755 --- a/tests/test-nozero.sh +++ b/tests/test-nozero.sh @@ -39,12 +39,14 @@ sock3=`mktemp -u` sock4=`mktemp -u` sock5a=`mktemp -u` sock5b=`mktemp -u` +sock6=`mktemp -u` files="nozero1.img nozero1.log $sock1 nozero1.pid nozero2.img nozero2.log $sock2 nozero2.pid nozero3.img nozero3.log $sock3 nozero3.pid nozero4.img nozero4.log $sock4 nozero4.pid nozero5.img nozero5a.log nozero5b.log $sock5a $sock5b - nozero5a.pid nozero5b.pid" + nozero5a.pid nozero5b.pid + nozero6.img nozero6.log $sock6 nozero6.pid" rm -f $files # Prep images, and check that qemu-io understands the actions we plan on @@ -54,6 +56,7 @@ cp nozero1.img nozero2.img cp nozero1.img nozero3.img cp nozero1.img nozero4.img cp nozero1.img nozero5.img +cp nozero1.img nozero6.img if ! qemu-io -f raw -d unmap -c 'w -z -u 0 1M' nozero1.img; then echo "$0: missing or broken qemu-io" rm nozero?.img @@ -82,17 +85,20 @@ cleanup () cat nozero5a.log || : echo "Log 5b file contents:" cat nozero5b.log || : + echo "Log 6 file contents:" + cat nozero6.log || : rm -f $files } cleanup_fn cleanup -# Run four parallel nbdkit; to compare the logs and see what changes. +# Run several parallel nbdkit; to compare the logs and see what changes. # 1: unfiltered, to check that qemu-io sends ZERO request and plugin trims # 2: log before filter with zeromode=none (default), to ensure no ZERO request # 3: log before filter with zeromode=emulate, to ensure ZERO from client # 4: log after filter with zeromode=emulate, to ensure no ZERO to plugin # 5a/b: both sides of nbd plugin: even though server side does not advertise # ZERO, the client side still exposes it, and just skips calling nbd's .zero +# 6: log after filter with zeromode=notrim, to ensure plugin does not trim start_nbdkit -P nozero1.pid -U $sock1 --filter=log \ file logfile=nozero1.log nozero1.img start_nbdkit -P nozero2.pid -U $sock2 --filter=log --filter=nozero \ @@ -105,6 +111,8 @@ start_nbdkit -P nozero5a.pid -U $sock5a --filter=log --filter=nozero \ file logfile=nozero5a.log nozero5.img start_nbdkit -P nozero5b.pid -U $sock5b --filter=log \ nbd logfile=nozero5b.log socket=$sock5a +start_nbdkit -P nozero6.pid -U $sock6 --filter=nozero --filter=log \ + file logfile=nozero6.log nozero6.img zeromode=notrim # Perform the zero write. qemu-io -f raw -c 'w -z -u 0 1M' "nbd+unix://?socket=$sock1" @@ -112,6 +120,7 @@ qemu-io -f raw -c 'w -z -u 0 1M' "nbd+unix://?socket=$sock2" qemu-io -f raw -c 'w -z -u 0 1M' "nbd+unix://?socket=$sock3" qemu-io -f raw -c 'w -z -u 0 1M' "nbd+unix://?socket=$sock4" qemu-io -f raw -c 'w -z -u 0 1M' "nbd+unix://?socket=$sock5b" +qemu-io -f raw -c 'w -z -u 0 1M' "nbd+unix://?socket=$sock6" # Check for expected ZERO vs. WRITE results grep 'connection=1 Zero' nozero1.log @@ -129,9 +138,23 @@ if grep 'connection=1 Zero' nozero5a.log; then echo "nbdkit should have converted zero into write before nbd plugin" exit 1 fi +grep 'connection=1 Zero' nozero6.log # Sanity check on contents - all 5 files should read identically cmp nozero1.img nozero2.img cmp nozero2.img nozero3.img cmp nozero3.img nozero4.img cmp nozero4.img nozero5.img +cmp nozero5.img nozero6.img + +# Sanity check on sparseness; only image 1 should be sparse +if test "$(stat -c %b nozero1.img)" = "$(stat -c %b nozero2.img)"; then + echo "nozero1.img was not trimmed" + exit 1 +fi +for i in 3 4 5 6; do + if test "$(stat -c %b nozero2.img)" != "$(stat -c %b nozero$i.img)"; then + echo "nozero$i.img was trimmed by mistake" + exit 1 + fi +done -- 2.20.1