[Libguestfs] [nbdkit PATCH] nozero: Add notrim mode
Richard W.M. Jones
rjones at redhat.com
Mon May 13 07:44:37 UTC 2019
On Thu, May 09, 2019 at 07:14:52PM -0500, Eric Blake wrote:
> 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 at redhat.com>
> ---
> 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
ACK
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list