Break up the large paragraph and split up some longer sentences to make it easier to read. --- filters/fua/nbdkit-fua-filter.pod | 51 ++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/filters/fua/nbdkit-fua-filter.pod b/filters/fua/nbdkit-fua-filter.pod index 0411f9f4..6e8f87e2 100644 --- a/filters/fua/nbdkit-fua-filter.pod +++ b/filters/fua/nbdkit-fua-filter.pod @@ -1,6 +1,6 @@ =head1 NAME -nbdkit-fua-filter - nbdkit FUA filter +nbdkit-fua-filter - modify nbdkit flush and Forced Unit Access (FUA) =head1 SYNOPSIS @@ -9,47 +9,60 @@ nbdkit-fua-filter - nbdkit FUA filter =head1 DESCRIPTION C<nbdkit-fua-filter> is a filter that intentionally modifies handling -of the 'Forced Unit Access' (FUA) flag across the NBD protocol. It is -mainly useful for testing client or server fallbacks, and for +of the S<“Forced Unit Access”> (FUA) flag across the NBD protocol. It +is mainly useful for testing client or server fallbacks, and for evaluating timing differences between proper use of FUA compared to a full flush. =head1 PARAMETERS +The C<fuamode> parameter is optional and controls which mode the +filter will use. + =over 4 =item B<fuamode=none> +FUA support is not advertised to the client. This is the default if +the C<fuamode> parameter is not specified. + =item B<fuamode=emulate> +The filter will emulate FUA support using the plugin’s C<.flush> +callback, regardless of whether the plugin itself supports more +efficient FUA. It refuses to load if the plugin does not support +flush. + =item B<fuamode=native> +The filter will advertise native FUA support to the client and earlier +filters in the chain. This is useful for comparing optimizations of +FUA handling when splitting large requests into sub-requests. It +refuses to load if the plugin’s C<.can_fua> callback returns +C<NBDKIT_FUA_NONE>. + =item B<fuamode=force> -Optional, controls which mode the filter will use. Mode B<none> -(default) means that FUA support is not advertised to the client. -Mode B<emulate> causes the filter to emulate FUA support using the -plugin's C<flush> callback, regardless of whether the plugin itself -supports more efficient FUA (or refuses to load if the plugin's -C<can_flush> callback returns false). Mode B<native> causes the -filter to advertise native FUA support to earlier filters, useful for -comparing optimizations of FUA handling when splitting large requests -into sub-requests (or refuses to load if the plugin's C<can_fua> -callback returns C<NBDKIT_FUA_NONE>). Mode B<force> causes the filter -to request FUA on all write transactions, even when the client did not -request it, and in turn treats client flush requests as a no-op (or -refuses to load if the plugin's C<can_fua> callback returns -C<NBDKIT_FUA_NONE>). +The filter will request FUA on all write transactions, even when the +client did not request it. In turn client flush requests become +no-ops. It refuses to load if the plugin's C<.can_fua> callback +returns C<NBDKIT_FUA_NONE>. =back =head1 EXAMPLES +=over 4 + +=item * + Serve the file F<disk.img>, but force the client to submit explicit flush requests instead of using C<NBD_CMD_FLAG_FUA>: nbdkit --filter=fua file disk.img +=item * + Observe that the blocksize filter optimizes its handling of the FUA flag based on whether it knows nbdkit will be emulating FUA with a flush, by comparing the log filter output on top of different fua @@ -60,12 +73,16 @@ filter modes: nbdkit --filter=blocksize --filter=log --filter=fua file disk.img \ maxlen=4k logfile=fua_native fuamode=native +=item * + Serve the file F<disk.img> in write-through mode, where all writes from the client are immediately flushed to disk as if the client had always requested FUA: nbdkit --filter=fua file fuamode=force disk.img +=back + =head1 FILES =over 4 -- 2.25.0

This causes conflicts if a cleanup hook happens to use a variable with the same name. --- tests/functions.sh.in | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/functions.sh.in b/tests/functions.sh.in index e2ef9701..b5c7d88b 100644 --- a/tests/functions.sh.in +++ b/tests/functions.sh.in @@ -43,17 +43,17 @@ cleanup_fn () _run_cleanup_hooks () { - local status=$? i + local _status=$? _i set +e trap '' INT QUIT TERM EXIT ERR - echo $0: run cleanup hooks: exit code $status + echo $0: run cleanup hooks: exit code $_status - for (( i = 0; i < ${#_cleanup_hook[@]}; ++i )); do - ${_cleanup_hook[i]} + for (( _i = 0; _i < ${#_cleanup_hook[@]}; ++_i )); do + ${_cleanup_hook[_i]} done - exit $status + exit $_status } trap _run_cleanup_hooks INT QUIT TERM EXIT ERR -- 2.25.0

This drops all FUA and flush requests. --- filters/fua/nbdkit-fua-filter.pod | 27 +++++++++++++++++---- filters/fua/fua.c | 39 +++++++++++++++++++++++++++---- tests/test-fua.sh | 33 ++++++++++++++++---------- 3 files changed, 79 insertions(+), 20 deletions(-) diff --git a/filters/fua/nbdkit-fua-filter.pod b/filters/fua/nbdkit-fua-filter.pod index 3d20b56a..4f564fab 100644 --- a/filters/fua/nbdkit-fua-filter.pod +++ b/filters/fua/nbdkit-fua-filter.pod @@ -9,10 +9,12 @@ nbdkit-fua-filter - modify nbdkit flush and Forced Unit Access (FUA) =head1 DESCRIPTION C<nbdkit-fua-filter> is a filter that intentionally modifies handling -of the S<“Forced Unit Access”> (FUA) flag across the NBD protocol. It -is mainly useful for testing client or server fallbacks, and for -evaluating timing differences between proper use of FUA compared to a -full flush. +of the S<“Forced Unit Access”> (FUA) flag across the NBD protocol. + +This filter can be used to disable FUA and flush requests for speed +(although this is unsafe). Also it can be used to test client or +server fallbacks, and for evaluating timing differences between proper +use of FUA compared to a full flush. =head1 PARAMETERS @@ -53,6 +55,15 @@ returns C<NBDKIT_FUA_NONE>. Pass through FUA and flush requests unchanged. Turns the filter into a no-op. +=item B<fuamode=discard> + +The filter will discard FUA and flush requests. + +B<This mode is unsafe>: If the NBD disk contains a filesystem then you +will likely lose data in the event of a crash. It should only be used +for ephemeral data which you can easily recreate, such as caches, +builds, test data, etc. + =back =head1 EXAMPLES @@ -86,6 +97,14 @@ always requested FUA: nbdkit --filter=fua file fuamode=force disk.img +=item * + +Serve the file F<disk.img> discarding all FUA and flush requests. +This can greatly improve performance, but you will likely lose data if +there is a crash, so it is not safe. + + nbdkit --filter=discard file fuamode=force disk.img + =back =head1 FILES diff --git a/filters/fua/fua.c b/filters/fua/fua.c index 6bc62a02..229d83db 100644 --- a/filters/fua/fua.c +++ b/filters/fua/fua.c @@ -47,6 +47,7 @@ static enum FuaMode { NATIVE, FORCE, PASS, + DISCARD, } fuamode; static int @@ -64,6 +65,8 @@ fua_config (nbdkit_next_config *next, void *nxdata, fuamode = FORCE; else if (strcmp (value, "pass") == 0) fuamode = PASS; + else if (strcmp (value, "discard") == 0) + fuamode = DISCARD; else { nbdkit_error ("unknown fuamode '%s'", value); return -1; @@ -91,6 +94,7 @@ fua_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, switch (fuamode) { case NONE: case PASS: + case DISCARD: break; case EMULATE: r = next_ops->can_flush (nxdata); @@ -120,9 +124,17 @@ fua_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, static int fua_can_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) { - if (fuamode == FORCE) + switch (fuamode) { + case FORCE: + case DISCARD: return 1; /* Advertise our no-op flush, even if plugin lacks it */ - return next_ops->can_flush (nxdata); + case NONE: + case EMULATE: + case NATIVE: + case PASS: + return next_ops->can_flush (nxdata); + } + abort (); } /* Advertise desired fua mode. */ @@ -136,6 +148,7 @@ fua_can_fua (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) return NBDKIT_FUA_EMULATE; case NATIVE: case FORCE: + case DISCARD: return NBDKIT_FUA_NATIVE; case PASS: return next_ops->can_fua (nxdata); @@ -167,6 +180,9 @@ fua_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, case FORCE: flags |= NBDKIT_FLAG_FUA; break; + case DISCARD: + flags &= ~NBDKIT_FLAG_FUA; + break; } r = next_ops->pwrite (nxdata, buf, count, offs, flags, err); if (r != -1 && need_flush) @@ -178,9 +194,18 @@ static int fua_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t flags, int *err) { - if (fuamode == FORCE) + switch (fuamode) { + case FORCE: return 0; /* Nothing to flush, since all writes already used FUA */ - return next_ops->flush (nxdata, flags, err); + case DISCARD: + return 0; /* Drop flushes! */ + case NONE: + case EMULATE: + case NATIVE: + case PASS: + return next_ops->flush (nxdata, flags, err); + } + abort (); } static int @@ -207,6 +232,9 @@ fua_trim (struct nbdkit_next_ops *next_ops, void *nxdata, case FORCE: flags |= NBDKIT_FLAG_FUA; break; + case DISCARD: + flags &= ~NBDKIT_FLAG_FUA; + break; } r = next_ops->trim (nxdata, count, offs, flags, err); if (r != -1 && need_flush) @@ -238,6 +266,9 @@ fua_zero (struct nbdkit_next_ops *next_ops, void *nxdata, case FORCE: flags |= NBDKIT_FLAG_FUA; break; + case DISCARD: + flags &= ~NBDKIT_FLAG_FUA; + break; } r = next_ops->zero (nxdata, count, offs, flags, err); if (r != -1 && need_flush) diff --git a/tests/test-fua.sh b/tests/test-fua.sh index 5fb5e929..bf64e8da 100755 --- a/tests/test-fua.sh +++ b/tests/test-fua.sh @@ -40,7 +40,8 @@ files="fua.img fua2.log fua2.pid fua3.log fua3.pid fua4.log fua4.pid - fua5.log fua5.pid" + fua5.log fua5.pid + fua6.log fua6.pid" rm -f $files # Prep images, and check that qemu-io understands the actions we plan on @@ -56,16 +57,10 @@ fi # on exit. cleanup () { - echo "Log 1 file contents:" - cat fua1.log || : - echo "Log 2 file contents:" - cat fua2.log || : - echo "Log 3 file contents:" - cat fua3.log || : - echo "Log 4 file contents:" - cat fua4.log || : - echo "Log 5 file contents:" - cat fua5.log || : + for i in {1..6}; do + echo "Log $i file contents:" + cat fua$i.log || : + done rm -f $files rm -rf $sockdir } @@ -77,6 +72,7 @@ cleanup_fn cleanup # 3: fuamode=native: log shows that blocksize preserves fua # 4: fuamode=force: log shows that fua is always enabled # 5: fuamode=pass: fua flag and flush unchanged +# 6: fuamode=discard: discard all fua and flush start_nbdkit -P fua1.pid -U $sockdir/fua1.sock \ --filter=log --filter=fua \ file logfile=fua1.log fua.img @@ -92,10 +88,13 @@ start_nbdkit -P fua4.pid -U $sockdir/fua4.sock \ start_nbdkit -P fua5.pid -U $sockdir/fua5.sock \ --filter=fua --filter=log \ file logfile=fua5.log fua.img fuamode=pass +start_nbdkit -P fua6.pid -U $sockdir/fua6.sock \ + --filter=fua --filter=log \ + file logfile=fua6.log fua.img fuamode=discard # Perform a flush, write, and zero, first without then with FUA for f in '' -f; do - for i in {1..5}; do + for i in {1..6}; do qemu-io -f raw -t none -c flush -c "w $f 0 64k" -c "w -z $f 64k 64k" \ "nbd+unix://?socket=$sockdir/fua$i.sock" done @@ -139,3 +138,13 @@ grep 'connection=1 Write.*fua=0' fua5.log grep 'connection=2 Write.*fua=1' fua5.log grep 'connection=1 Zero.*fua=0' fua5.log grep 'connection=2 Zero.*fua=1' fua5.log + +# Test 6: Flush and fua=1 must not appear. +if grep 'Flush' fua6.log; then + echo "filter should have elided flush" + exit 1 +fi +if grep -E '(Write|Zero).*fua=1' fua6.log; then + echo "filter should have elided fua" + exit 1 +fi -- 2.25.0