Richard W.M. Jones
2020-May-22 21:32 UTC
[Libguestfs] [PATCH nbdkit 0/4] Add fuamode=pass and fuamode=discard
Two hopefully useful additions to the fua filter. The second one is kind of like cache=unsafe in qemu, in that it exchanges correctness for speed. Useful for data which is easily recreated in the event of a crash or for people who like living on the edge and have good backups. Rich.
Richard W.M. Jones
2020-May-22 21:32 UTC
[Libguestfs] [PATCH nbdkit 1/4] fua: Revise and clarify the nbdkit-fua-filter manual page.
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
Richard W.M. Jones
2020-May-22 21:32 UTC
[Libguestfs] [PATCH nbdkit 2/4] fua: Add fuamode=pass.
Passes FUA and flush requests unchanged. A convenient way to turn the filter into a no-op without rewriting the whole nbdkit command line. --- filters/fua/nbdkit-fua-filter.pod | 5 +++++ filters/fua/fua.c | 12 +++++++++++- tests/test-fua.sh | 21 ++++++++++++++++++--- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/filters/fua/nbdkit-fua-filter.pod b/filters/fua/nbdkit-fua-filter.pod index 6e8f87e2..3d20b56a 100644 --- a/filters/fua/nbdkit-fua-filter.pod +++ b/filters/fua/nbdkit-fua-filter.pod @@ -48,6 +48,11 @@ 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>. +=item B<fuamode=pass> + +Pass through FUA and flush requests unchanged. Turns the filter into +a no-op. + =back =head1 EXAMPLES diff --git a/filters/fua/fua.c b/filters/fua/fua.c index cee05eed..6bc62a02 100644 --- a/filters/fua/fua.c +++ b/filters/fua/fua.c @@ -46,6 +46,7 @@ static enum FuaMode { EMULATE, NATIVE, FORCE, + PASS, } fuamode; static int @@ -61,6 +62,8 @@ fua_config (nbdkit_next_config *next, void *nxdata, fuamode = NATIVE; else if (strcmp (value, "force") == 0) fuamode = FORCE; + else if (strcmp (value, "pass") == 0) + fuamode = PASS; else { nbdkit_error ("unknown fuamode '%s'", value); return -1; @@ -71,7 +74,8 @@ fua_config (nbdkit_next_config *next, void *nxdata, } #define fua_config_help \ - "fuamode=<MODE> One of 'none' (default), 'emulate', 'native', 'force'.\n" \ + "fuamode=<MODE> One of 'none' (default), 'emulate', 'native',\n" \ + " 'force', 'pass'." /* Check that desired mode is supported by plugin. */ static int @@ -86,6 +90,7 @@ fua_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, switch (fuamode) { case NONE: + case PASS: break; case EMULATE: r = next_ops->can_flush (nxdata); @@ -132,6 +137,8 @@ fua_can_fua (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) case NATIVE: case FORCE: return NBDKIT_FUA_NATIVE; + case PASS: + return next_ops->can_fua (nxdata); } abort (); } @@ -155,6 +162,7 @@ fua_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, } break; case NATIVE: + case PASS: break; case FORCE: flags |= NBDKIT_FLAG_FUA; @@ -194,6 +202,7 @@ fua_trim (struct nbdkit_next_ops *next_ops, void *nxdata, } break; case NATIVE: + case PASS: break; case FORCE: flags |= NBDKIT_FLAG_FUA; @@ -224,6 +233,7 @@ fua_zero (struct nbdkit_next_ops *next_ops, void *nxdata, } break; case NATIVE: + case PASS: break; case FORCE: flags |= NBDKIT_FLAG_FUA; diff --git a/tests/test-fua.sh b/tests/test-fua.sh index 1c869e96..5fb5e929 100755 --- a/tests/test-fua.sh +++ b/tests/test-fua.sh @@ -39,7 +39,8 @@ files="fua.img fua1.log fua1.pid fua2.log fua2.pid fua3.log fua3.pid - fua4.log fua4.pid" + fua4.log fua4.pid + fua5.log fua5.pid" rm -f $files # Prep images, and check that qemu-io understands the actions we plan on @@ -63,16 +64,19 @@ cleanup () cat fua3.log || : echo "Log 4 file contents:" cat fua4.log || : + echo "Log 5 file contents:" + cat fua5.log || : rm -f $files rm -rf $sockdir } cleanup_fn cleanup -# Run four parallel nbdkit; to compare the logs and see what changes. +# Run parallel nbdkit; to compare the logs and see what changes. # 1: fuamode=none (default): client should send flush instead # 2: fuamode=emulate: log shows that blocksize optimizes fua to flush # 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 start_nbdkit -P fua1.pid -U $sockdir/fua1.sock \ --filter=log --filter=fua \ file logfile=fua1.log fua.img @@ -85,10 +89,13 @@ start_nbdkit -P fua3.pid -U $sockdir/fua3.sock \ start_nbdkit -P fua4.pid -U $sockdir/fua4.sock \ --filter=fua --filter=log \ file logfile=fua4.log fua.img fuamode=force +start_nbdkit -P fua5.pid -U $sockdir/fua5.sock \ + --filter=fua --filter=log \ + file logfile=fua5.log fua.img fuamode=pass # Perform a flush, write, and zero, first without then with FUA for f in '' -f; do - for i in {1..4}; do + for i in {1..5}; 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 @@ -124,3 +131,11 @@ if grep 'Flush' fua4.log; then echo "filter should have elided flush" exit 1 fi + +# Test 5: Flush should be passed through. +# There should also be one set of fua=0 and a second set of fua=1. +grep 'Flush' fua5.log +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 -- 2.25.0
Richard W.M. Jones
2020-May-22 21:32 UTC
[Libguestfs] [PATCH nbdkit 3/4] tests/functions.sh.in: Rename variables $status and $i when running cleanups.
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
Richard W.M. Jones
2020-May-22 21:32 UTC
[Libguestfs] [PATCH nbdkit 4/4] fua: Add unsafe fuamode=discard.
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
Eric Blake
2020-May-26 15:25 UTC
Re: [Libguestfs] [PATCH nbdkit 3/4] tests/functions.sh.in: Rename variables $status and $i when running cleanups.
On 5/22/20 4:32 PM, Richard W.M. Jones wrote:> This causes conflicts if a cleanup hook happens to use a variable with > the same name.I don't envy the debugging you used to finally spot this. Using 'local' in the cleanup hook helps, but so does this rename, so I like this approach.> --- > 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 > >-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Eric Blake
2020-May-26 15:30 UTC
Re: [Libguestfs] [PATCH nbdkit 4/4] fua: Add unsafe fuamode=discard.
On 5/22/20 4:32 PM, Richard W.M. Jones wrote:> 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(-)Overall, the series looks good to me. I don't know if there are any other man pages where we should call attention to the fact about the new mode.> > 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.Also, we may want to mention when this mode was introduced (since if you don't have 1.22 installed, you can't use it yet). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Reasonably Related Threads
- [PATCH nbdkit 4/4] tests: Add a helper function which waits for nbdkit to start up.
- [PATCH nbdkit 0/4] Add fuamode=pass and fuamode=discard
- [PATCH v2 nbdkit 5/5] tests: Add a helper function which waits for nbdkit to start up.
- [PATCH nbdkit 3/6] file: Make the file= parameter into a magic config key.
- [PATCH nbdkit v2 3/6] file: Make the file= parameter into a magic config key.