Make it possible for plugins to advertise caching support, and to implement their own caching algorithm. There are no universally common cache algorithms (calling .pread and ignoring the buffer works great for local files, but actually penalizes remote network access), so the code intentionally defaults .can_cache to NONE if .cache is missing. However, as both .pread (files) and a silent no-op (for plugins serving data that only resides in memory already) are common implementations, we allow .can_cache to return success even when .cache is missing (which is different from many of the other callbacks, where .can_FOO should not return true unless .FOO is present). On the other hand, if only .cache is present, defaulting .can_cache to NATIVE makes sense. Thus, .can_cache is a tri-state similar to .can_fua. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- docs/nbdkit-plugin.pod | 82 +++++++++++++++++++++++++++++++++++++++++ include/nbdkit-plugin.h | 2 + server/plugins.c | 18 +++++++-- 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 318be4c..7f83234 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -222,6 +222,37 @@ landed in persistent storage. =back +The following defines are valid as successful return values for +C<.can_cache>: + +=over 4 + +=item C<NBDKIT_CACHE_NONE> + +The server does not advertise caching support, and rejects any +client-requested caching. Any C<.cache> callback is ignored. + +=item C<NBDKIT_CACHE_EMULATE> + +The nbdkit server advertises cache support to the client, where the +client may request that the server cache a region of the export to +potentially speed up future read and/or write operations on that +region. The nbdkit server implements the caching by calling C<.pread> +and ignoring the results. This option exists to ease the +implementation of a common form of caching; any C<.cache> callback is +ignored. + +=item C<NBDKIT_CACHE_NATIVE> + +The nbdkit server advertises cache support to the client, where the +client may request that the server cache a region of the export to +potentially speed up future read and/or write operations on that +region. The nbdkit server calls the C<.cache> callback to perform the +caching; if that callback is missing, the client's cache request +succeeds without doing anything. + +=back + =head1 ERROR HANDLING If there is an error in the plugin, the plugin should call @@ -620,6 +651,29 @@ with an error message and return C<-1>. This callback is not required. If omitted, then we return false. +=head2 C<.can_cache> + + int can_cache (void *handle); + +This is called during the option negotiation phase to find out if the +plugin supports a cache operation. The nature of the caching is +unspecified (including whether there are limits on how much can be +cached at once, and whether writes to a cached region have +write-through or write-back semantics), but the command exists to let +clients issue a hint to the server that they will be accessing that +region of the export. + +If this returns C<NBDKIT_CACHE_NONE>, cache support is not advertised +to the guest; if this returns C<NBDKIT_CACHE_EMULATE>, caching is +emulated by the server calling C<.pread> and ignoring the results; if +this returns C<NBDKIT_CACHE_NATIVE>, then the C<.cache> callback will +be used. If there is an error, C<.can_cache> should call +C<nbdkit_error> with an error message and return C<-1>. + +This callback is not required. If omitted, then we return +C<NBDKIT_CACHE_NONE> if the C<.cache> callback is missing, or +C<NBDKIT_CACHE_NATIVE> if it is defined. + =head2 C<.pread> int pread (void *handle, void *buf, uint32_t count, uint64_t offset, @@ -814,6 +868,34 @@ C<nbdkit_extent_add> returns C<0> on success or C<-1> on failure. On failure C<nbdkit_error> and/or C<nbdkit_set_error> has already been called. C<errno> will be set to a suitable value. +=head2 C<.cache> + + int cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags); + +During the data serving phase, this callback is used to give the +plugin a hint that the client intends to make further accesses to the +given region of the export. The nature of caching is not specified +further by the NBD specification (for example, a server may place +limits on how much may be cached at once, and there is no way to +control if writes to a cached area have write-through or write-back +semantics). In fact, the cache command can always fail and still be +compliant, and success might not guarantee a performance gain. If +this callback is omitted, then the results of C<.can_cache> determine +whether nbdkit will reject cache requests, treat them as instant +success, or emulate caching by calling C<.pread> over the same region +and ignoring the results. + +This function will not be called if C<.can_cache> did not return +C<NBDKIT_CACHE_NATIVE>. The parameter C<flags> exists in case of +future NBD protocol extensions; at this time, it will be 0 on input. A +plugin must fail this function if C<flags> includes an unrecognized +flag, as that may indicate a requirement that the plugin comply must +with a specific caching semantic. + +If there is an error, C<.cache> should call C<nbdkit_error> with an +error message, and C<nbdkit_set_error> to record an appropriate error +(unless C<errno> is sufficient), then return C<-1>. + =head1 THREADS Each nbdkit plugin must declare its thread safety model by defining diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 54b4ce2..e9b1808 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -128,6 +128,8 @@ struct nbdkit_plugin { int (*can_extents) (void *handle); int (*extents) (void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents); + int (*can_cache) (void *handle); + int (*cache) (void *handle, uint32_t count, uint64_t offset, uint32_t flags); }; extern void nbdkit_set_error (int err); diff --git a/server/plugins.c b/server/plugins.c index cb9a50c..f293e6a 100644 --- a/server/plugins.c +++ b/server/plugins.c @@ -201,6 +201,8 @@ plugin_dump_fields (struct backend *b) HAS (can_multi_conn); HAS (can_extents); HAS (extents); + HAS (can_cache); + HAS (cache); #undef HAS /* Custom fields. */ @@ -451,11 +453,16 @@ plugin_can_multi_conn (struct backend *b, struct connection *conn) static int plugin_can_cache (struct backend *b, struct connection *conn) { + struct backend_plugin *p = container_of (b, struct backend_plugin, backend); + assert (connection_get_handle (conn, 0)); debug ("can_cache"); - /* FIXME: return default based on plugin->cache */ + if (p->plugin.can_cache) + return p->plugin.can_cache (connection_get_handle (conn, 0)); + if (p->plugin.cache) + return NBDKIT_CACHE_NATIVE; return NBDKIT_CACHE_NONE; } @@ -710,16 +717,19 @@ plugin_cache (struct backend *b, struct connection *conn, int *err) { struct backend_plugin *p = container_of (b, struct backend_plugin, backend); - int r = -1; + int r; assert (connection_get_handle (conn, 0)); assert (!flags); debug ("cache count=%" PRIu32 " offset=%" PRIu64, count, offset); - /* FIXME: assert plugin->cache and call it */ - assert (false); + /* A plugin may advertise caching but not provide .cache; in that + * case, caching is explicitly a no-op. */ + if (!p->plugin.cache) + return 0; + r = p->plugin.cache (connection_get_handle (conn, 0), count, offset, flags); if (r == -1) *err = get_error (p); return r; -- 2.20.1

For our plugins which are reading from one or more local files, calling .pread is likely to populate the kernel's file cache to our advantage; these plugins are complicated enough that there is nothing better like posix_fadvise() that we can try. Implementing .can_cache is sufficient to let nbdkit do the desired work on our behalf. Full list of plugins changed: ext2, floppy, iso, linuxdisk, partitioning Note that the tar plugin would likewise probably benefit from .pread treatment; but for that, we'd first have to wire up .can_cache to the perl language binding. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/ext2/ext2.c | 10 +++++++++- plugins/floppy/floppy.c | 11 ++++++++++- plugins/iso/iso.c | 10 +++++++++- plugins/linuxdisk/linuxdisk.c | 9 +++++++++ plugins/partitioning/partitioning.c | 11 ++++++++++- 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/plugins/ext2/ext2.c b/plugins/ext2/ext2.c index 17f88fe..6698d99 100644 --- a/plugins/ext2/ext2.c +++ b/plugins/ext2/ext2.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2017-2018 Red Hat Inc. + * Copyright (C) 2017-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -212,6 +212,13 @@ ext2_can_fua (void *handle) return NBDKIT_FUA_NATIVE; } +static int +ext2_can_cache (void *handle) +{ + /* Let nbdkit call pread to populate the file system cache. */ + return NBDKIT_CACHE_EMULATE; +} + /* It might be possible to relax this, but it's complicated. * * It's desirable for ‘nbdkit -r’ to behave the same way as @@ -345,6 +352,7 @@ static struct nbdkit_plugin plugin = { .open = ext2_open, .close = ext2_close, .can_fua = ext2_can_fua, + .can_cache = ext2_can_cache, .get_size = ext2_get_size, .pread = ext2_pread, .pwrite = ext2_pwrite, diff --git a/plugins/floppy/floppy.c b/plugins/floppy/floppy.c index ebdea5b..41a2364 100644 --- a/plugins/floppy/floppy.c +++ b/plugins/floppy/floppy.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -128,6 +128,14 @@ floppy_can_multi_conn (void *handle) return 1; } +/* Cache. */ +static int +floppy_can_cache (void *handle) +{ + /* Let nbdkit call pread to populate the file system cache. */ + return NBDKIT_CACHE_EMULATE; +} + /* Read data from the file. */ static int floppy_pread (void *handle, void *buf, uint32_t count, uint64_t offset) @@ -199,6 +207,7 @@ static struct nbdkit_plugin plugin = { .open = floppy_open, .get_size = floppy_get_size, .can_multi_conn = floppy_can_multi_conn, + .can_cache = floppy_can_cache, .pread = floppy_pread, .errno_is_preserved = 1, }; diff --git a/plugins/iso/iso.c b/plugins/iso/iso.c index 586f1f9..4728ff3 100644 --- a/plugins/iso/iso.c +++ b/plugins/iso/iso.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -215,6 +215,13 @@ iso_can_multi_conn (void *handle) return 1; } +static int +iso_can_cache (void *handle) +{ + /* Let nbdkit call pread to populate the file system cache. */ + return NBDKIT_CACHE_EMULATE; +} + /* Read data from the file. */ static int iso_pread (void *handle, void *buf, uint32_t count, uint64_t offset) @@ -249,6 +256,7 @@ static struct nbdkit_plugin plugin = { .open = iso_open, .get_size = iso_get_size, .can_multi_conn = iso_can_multi_conn, + .can_cache = iso_can_cache, .pread = iso_pread, .errno_is_preserved = 1, }; diff --git a/plugins/linuxdisk/linuxdisk.c b/plugins/linuxdisk/linuxdisk.c index 1ba7114..99dbc99 100644 --- a/plugins/linuxdisk/linuxdisk.c +++ b/plugins/linuxdisk/linuxdisk.c @@ -159,6 +159,14 @@ linuxdisk_can_multi_conn (void *handle) return 1; } +/* Cache. */ +static int +linuxdisk_can_cache (void *handle) +{ + /* Let nbdkit call pread to populate the file system cache. */ + return NBDKIT_CACHE_EMULATE; +} + /* Read data from the virtual disk. */ static int linuxdisk_pread (void *handle, void *buf, uint32_t count, uint64_t offset, @@ -221,6 +229,7 @@ static struct nbdkit_plugin plugin = { .open = linuxdisk_open, .get_size = linuxdisk_get_size, .can_multi_conn = linuxdisk_can_multi_conn, + .can_cache = linuxdisk_can_cache, .pread = linuxdisk_pread, .errno_is_preserved = 1, }; diff --git a/plugins/partitioning/partitioning.c b/plugins/partitioning/partitioning.c index 630c6d2..90333bf 100644 --- a/plugins/partitioning/partitioning.c +++ b/plugins/partitioning/partitioning.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -297,6 +297,14 @@ partitioning_can_multi_conn (void *handle) return 1; } +/* Cache. */ +static int +partitioning_can_cache (void *handle) +{ + /* Let nbdkit call pread to populate the file system cache. */ + return NBDKIT_CACHE_EMULATE; +} + /* Read data. */ static int partitioning_pread (void *handle, void *buf, uint32_t count, uint64_t offset) @@ -426,6 +434,7 @@ static struct nbdkit_plugin plugin = { .open = partitioning_open, .get_size = partitioning_get_size, .can_multi_conn = partitioning_can_multi_conn, + .can_cache = partitioning_can_cache, .pread = partitioning_pread, .pwrite = partitioning_pwrite, .flush = partitioning_flush, -- 2.20.1

In the nbd plugin, we expose caching to the client only if the server exposed it to us. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- plugins/nbd/nbd.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index 821f256..df7d366 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -1151,6 +1151,16 @@ nbd_can_multi_conn (void *handle) return h->flags & NBD_FLAG_CAN_MULTI_CONN; } +static int +nbd_can_cache (void *handle) +{ + struct handle *h = handle; + + if (h->flags & NBD_FLAG_SEND_CACHE) + return NBDKIT_CACHE_NATIVE; + return NBDKIT_CACHE_NONE; +} + static int nbd_can_extents (void *handle) { @@ -1245,6 +1255,18 @@ nbd_extents (void *handle, uint32_t count, uint64_t offset, return c < 0 ? c : nbd_reply (h, c); } +/* Cache a portion of the file. */ +static int +nbd_cache (void *handle, uint32_t count, uint64_t offset, uint32_t flags) +{ + struct handle *h = handle; + int c; + + assert (!flags); + c = nbd_request (h, 0, NBD_CMD_CACHE, offset, count); + return c < 0 ? c : nbd_reply (h, c); +} + static struct nbdkit_plugin plugin = { .name = "nbd", .longname = "nbdkit nbd plugin", @@ -1264,12 +1286,14 @@ static struct nbdkit_plugin plugin = { .can_fua = nbd_can_fua, .can_multi_conn = nbd_can_multi_conn, .can_extents = nbd_can_extents, + .can_cache = nbd_can_cache, .pread = nbd_pread, .pwrite = nbd_pwrite, .zero = nbd_zero, .flush = nbd_flush, .trim = nbd_trim, .extents = nbd_extents, + .cache = nbd_cache, .errno_is_preserved = 1, }; -- 2.20.1

On Wed, May 15, 2019 at 10:57:57PM -0500, Eric Blake wrote: I wonder if you meant to add C<cache> to this sentence? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v

This was a bit harder than sh, but still a lot of copy-and-paste. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- Note: I'm not sure how to actually test this beyond compilation. --- plugins/ocaml/ocaml.c | 51 ++++++++++++++++++++++++++++++++++++++++ plugins/ocaml/NBDKit.ml | 16 ++++++++++++- plugins/ocaml/NBDKit.mli | 5 ++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c index 4447d7f..f664a7f 100644 --- a/plugins/ocaml/ocaml.c +++ b/plugins/ocaml/ocaml.c @@ -128,6 +128,9 @@ static value can_multi_conn_fn; static value can_extents_fn; static value extents_fn; +static value can_cache_fn; +static value cache_fn; + /*----------------------------------------------------------------------*/ /* Wrapper functions that translate calls from C (ie. nbdkit) to OCaml. */ @@ -638,6 +641,48 @@ extents_wrapper (void *h, uint32_t count, uint64_t offset, uint32_t flags, CAMLreturnT (int, 0); } +static int +can_cache_wrapper (void *h) +{ + CAMLparam0 (); + CAMLlocal1 (rv); + + caml_leave_blocking_section (); + + rv = caml_callback_exn (can_cache_fn, *(value *) h); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + caml_enter_blocking_section (); + CAMLreturnT (int, -1); + } + + caml_enter_blocking_section (); + CAMLreturnT (int, Int_val (rv)); +} + +static int +cache_wrapper (void *h, uint32_t count, uint64_t offset, uint32_t flags) +{ + CAMLparam0 (); + CAMLlocal4 (rv, countv, offsetv, flagsv); + + caml_leave_blocking_section (); + + countv = caml_copy_int32 (count); + offsetv = caml_copy_int32 (offset); + flagsv = Val_flags (flags); + + value args[] = { *(value *) h, countv, offsetv, flagsv }; + rv = caml_callbackN_exn (cache_fn, sizeof args / sizeof args[0], args); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + CAMLreturnT (int, -1); + } + + caml_enter_blocking_section (); + CAMLreturnT (int, 0); +} + /*----------------------------------------------------------------------*/ /* set_* functions called from OCaml code at load time to initialize * fields in the plugin struct. @@ -727,6 +772,9 @@ SET(can_multi_conn) SET(can_extents) SET(extents) +SET(can_cache) +SET(cache) + #undef SET static void @@ -766,6 +814,9 @@ remove_roots (void) REMOVE (can_extents); REMOVE (extents); + REMOVE (can_cache); + REMOVE (cache); + #undef REMOVE } diff --git a/plugins/ocaml/NBDKit.ml b/plugins/ocaml/NBDKit.ml index 7aca8c8..02aa200 100644 --- a/plugins/ocaml/NBDKit.ml +++ b/plugins/ocaml/NBDKit.ml @@ -37,6 +37,8 @@ and flag = May_trim | FUA | Req_one type fua_flag = FuaNone | FuaEmulate | FuaNative +type cache_flag = CacheNone | CacheEmulate | CacheNop + type extent = { offset : int64; length : int64; @@ -82,6 +84,9 @@ type 'a plugin = { can_extents : ('a -> bool) option; extents : ('a -> int32 -> int64 -> flags -> extent list) option; + + can_cache : ('a -> cache_flag) option; + cache : ('a -> int32 -> int64 -> flags -> unit) option; } let default_callbacks = { @@ -122,6 +127,9 @@ let default_callbacks = { can_extents = None; extents = None; + + can_cache = None; + cache = None; } type thread_model = @@ -170,6 +178,9 @@ external set_can_multi_conn : ('a -> bool) -> unit = "ocaml_nbdkit_set_can_multi external set_can_extents : ('a -> bool) -> unit = "ocaml_nbdkit_set_can_extents" external set_extents : ('a -> int32 -> int64 -> flags -> extent list) -> unit = "ocaml_nbdkit_set_extents" +external set_can_cache : ('a -> cache_flag) -> unit = "ocaml_nbdkit_set_can_cache" +external set_cache : ('a -> int32 -> int64 -> flags -> unit) -> unit = "ocaml_nbdkit_set_cache" + let may f = function None -> () | Some a -> f a let register_plugin thread_model plugin = @@ -229,7 +240,10 @@ let register_plugin thread_model plugin = may set_can_multi_conn plugin.can_multi_conn; may set_can_extents plugin.can_extents; - may set_extents plugin.extents + may set_extents plugin.extents; + + may set_can_cache plugin.can_cache; + may set_cache plugin.cache external _set_error : int -> unit = "ocaml_nbdkit_set_error" "noalloc" diff --git a/plugins/ocaml/NBDKit.mli b/plugins/ocaml/NBDKit.mli index da110fe..bab8f7f 100644 --- a/plugins/ocaml/NBDKit.mli +++ b/plugins/ocaml/NBDKit.mli @@ -40,6 +40,8 @@ and flag = May_trim | FUA | Req_one type fua_flag = FuaNone | FuaEmulate | FuaNative +type cache_flag = CacheNone | CacheEmulate | CacheNop + type extent = { offset : int64; length : int64; @@ -86,6 +88,9 @@ type 'a plugin = { can_extents : ('a -> bool) option; extents : ('a -> int32 -> int64 -> flags -> extent list) option; + + can_cache : ('a -> cache_flag) option; + cache : ('a -> int32 -> int64 -> flags -> unit) option; } (** The plugin fields and callbacks. ['a] is the handle type. *) -- 2.20.1

Make it possible for filters to adjust the behavior for NBD_CMD_CACHE. To avoid any 'git bisect' breakage, this patch leaves .can_cache as NBDKIT_CACHE_NONE if a filter does not provide an override, rather than passthrough to the plugin; it will be flipped later after all necessary filters have been patched first. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- docs/nbdkit-filter.pod | 42 ++++++++++++++++++++++++++++++++++------- include/nbdkit-filter.h | 8 ++++++++ server/filters.c | 37 ++++++++++++++++++++++++++++++++---- 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 6aeaa7b..857f241 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -356,6 +356,8 @@ calls. =head2 C<.can_multi_conn> +=head2 C<.can_cache> + int (*can_write) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); int (*can_flush) (struct nbdkit_next_ops *next_ops, void *nxdata, @@ -373,6 +375,8 @@ calls. void *handle); int (*can_multi_conn) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); + int (*can_cache) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle); These intercept the corresponding plugin methods, and control feature bits advertised to the client. @@ -385,12 +389,14 @@ the plugin's own C<.can_zero> callback returned false, because nbdkit implements a fallback to C<.pwrite> at the plugin layer. Remember that most of the feature check functions return merely a -boolean success value, while C<.can_fua> has three success values. -The difference between values may affect choices made in the filter: -when splitting a write request that requested FUA from the client, if -C<next_ops-E<gt>can_fua> returns C<NBDKIT_FUA_NATIVE>, then the filter -should pass the FUA flag on to each sub-request; while if it is known -that FUA is emulated by a flush because of a return of +boolean success value, while C<.can_fua> and C<.can_cache> have three +success values. + +The difference between C<.can_fua> values may affect choices made in +the filter: when splitting a write request that requested FUA from the +client, if C<next_ops-E<gt>can_fua> returns C<NBDKIT_FUA_NATIVE>, then +the filter should pass the FUA flag on to each sub-request; while if +it is known that FUA is emulated by a flush because of a return of C<NBDKIT_FUA_EMULATE>, it is more efficient to only flush once after all sub-requests have completed (often by passing C<NBDKIT_FLAG_FUA> on to only the final sub-request, or by dropping the flag and ending @@ -597,6 +603,28 @@ Returns the number of extents in the list. Returns a copy of the C<i>'th extent. +=head2 C<.cache> + + int (*cache) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, + uint32_t flags, int *err); + +This intercepts the plugin C<.cache> method and can be used to modify +cache requests. + +This function will not be called if C<.can_cache> returned +C<NBDKIT_CACHE_NONE> or C<NBDKIT_CACHE_EMULATE>; in turn, the filter +should not call C<next_ops-E<gt>cache> unless +C<next_ops-E<gt>can_cache> returned C<NBDKIT_CACHE_NATIVE>. + +The parameter C<flags> exists in case of future NBD protocol +extensions; at this time, it will be 0 on input, and the filter should +not pass any flags to C<next_ops-E<gt>cache>. + +If there is an error, C<.cache> should call C<nbdkit_error> with an +error message B<and> return -1 with C<err> set to the positive errno +value to return to the client. + =head1 ERROR HANDLING If there is an error in the filter itself, the filter should call @@ -708,4 +736,4 @@ Richard W.M. Jones =head1 COPYRIGHT -Copyright (C) 2013-2018 Red Hat Inc. +Copyright (C) 2013-2019 Red Hat Inc. diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index 9b6cd6e..5893dd8 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.h @@ -74,6 +74,7 @@ struct nbdkit_next_ops { int (*can_extents) (void *nxdata); int (*can_fua) (void *nxdata); int (*can_multi_conn) (void *nxdata); + int (*can_cache) (void *nxdata); int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); @@ -87,6 +88,8 @@ struct nbdkit_next_ops { int *err); int (*extents) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err); + int (*cache) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, + int *err); }; struct nbdkit_filter { @@ -142,6 +145,8 @@ struct nbdkit_filter { void *handle); int (*can_multi_conn) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); + int (*can_cache) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle); int (*pread) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, void *buf, uint32_t count, uint64_t offset, @@ -161,6 +166,9 @@ struct nbdkit_filter { int (*extents) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err); + int (*cache) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, uint32_t flags, + int *err); }; #define NBDKIT_REGISTER_FILTER(filter) \ diff --git a/server/filters.c b/server/filters.c index e456fbf..430d515 100644 --- a/server/filters.c +++ b/server/filters.c @@ -329,6 +329,13 @@ next_can_multi_conn (void *nxdata) return b_conn->b->can_multi_conn (b_conn->b, b_conn->conn); } +static int +next_can_cache (void *nxdata) +{ + struct b_conn *b_conn = nxdata; + return b_conn->b->can_cache (b_conn->b, b_conn->conn); +} + static int next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) @@ -379,6 +386,15 @@ next_extents (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, extents, err); } +static int +next_cache (void *nxdata, uint32_t count, uint64_t offset, + uint32_t flags, int *err) +{ + struct b_conn *b_conn = nxdata; + return b_conn->b->cache (b_conn->b, b_conn->conn, count, offset, flags, + err); +} + static struct nbdkit_next_ops next_ops = { .get_size = next_get_size, .can_write = next_can_write, @@ -389,12 +405,14 @@ static struct nbdkit_next_ops next_ops = { .can_extents = next_can_extents, .can_fua = next_can_fua, .can_multi_conn = next_can_multi_conn, + .can_cache = next_can_cache, .pread = next_pread, .pwrite = next_pwrite, .flush = next_flush, .trim = next_trim, .zero = next_zero, .extents = next_extents, + .cache = next_cache, }; static int @@ -577,12 +595,18 @@ static int filter_can_cache (struct backend *b, struct connection *conn) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + void *handle = connection_get_handle (conn, f->backend.i); + struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; debug ("%s: can_cache", f->name); + if (f->filter.can_cache) + return f->filter.can_cache (&next_ops, &nxdata, handle); /* FIXME: Default to f->backend.next->can_cache, once all filters have been audited */ - return NBDKIT_CACHE_NONE; + else + return NBDKIT_CACHE_NONE; + return f->backend.next->can_cache (f->backend.next, conn); } static int @@ -720,15 +744,20 @@ filter_cache (struct backend *b, struct connection *conn, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + void *handle = connection_get_handle (conn, f->backend.i); + struct b_conn nxdata = { .b = f->backend.next, .conn = conn }; assert (flags == 0); debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64 " flags=0x%" PRIx32, f->name, count, offset, flags); - /* FIXME: Allow filter to rewrite request */ - return f->backend.next->cache (f->backend.next, conn, - count, offset, flags, err); + if (f->filter.cache) + return f->filter.cache (&next_ops, &nxdata, handle, + count, offset, flags, err); + else + return f->backend.next->cache (f->backend.next, conn, + count, offset, flags, err); } static struct backend filter_functions = { -- 2.20.1

The sh bindings make this one easy to test. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- tests/test-eflags.sh | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test-eflags.sh b/tests/test-eflags.sh index 6a90b87..eaaaae0 100755 --- a/tests/test-eflags.sh +++ b/tests/test-eflags.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # nbdkit -# Copyright (C) 2018 Red Hat Inc. +# Copyright (C) 2018-2019 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -286,3 +286,18 @@ EOF [ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|CAN_MULTI_CONN )) ] || fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|CAN_MULTI_CONN" + +#---------------------------------------------------------------------- +# -r +# can_cache=true + +do_nbdkit -r <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_cache) echo "emulate" ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] || + fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE" -- 2.20.1

The whole point of the cache filter is to avoid visiting the plugin more than once for a range of data; as such, passing cache requests through to the plugin is wrong, and a client request for caching a range of the file obviously means we want the data locally. Our filter defaults to cache_on_read=false, where we normally only cache data that has been written but not yet flushed; but it can be presumed that an explicit cache request should always pull data locally, regardless of the cache_on_read setting. And even when cache_on_read=true, we can implement caching more efficiently than discarding the buffer of a naive pread. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/cache/nbdkit-cache-filter.pod | 6 +-- filters/cache/blk.h | 10 ++++- filters/cache/blk.c | 50 ++++++++++++++++++++++++- filters/cache/cache.c | 53 +++++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 6 deletions(-) diff --git a/filters/cache/nbdkit-cache-filter.pod b/filters/cache/nbdkit-cache-filter.pod index 5993831..8b50231 100644 --- a/filters/cache/nbdkit-cache-filter.pod +++ b/filters/cache/nbdkit-cache-filter.pod @@ -70,9 +70,9 @@ Limit the size of the cache to C<SIZE>. See L</CACHE MAXIMUM SIZE> below. =item B<cache-on-read=true> -Cache read requests as well as write requests. Any time a block is -read from the plugin, it is saved in the cache (if there is sufficient -space) so the same data can be served more quickly later. +Cache read requests as well as write and cache requests. Any time a +block is read from the plugin, it is saved in the cache (if there is +sufficient space) so the same data can be served more quickly later. Note that if the underlying data served by the plugin can be modified by some other means (eg. something else can write to a file which is diff --git a/filters/cache/blk.h b/filters/cache/blk.h index 974a118..0d84f74 100644 --- a/filters/cache/blk.h +++ b/filters/cache/blk.h @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -49,11 +49,17 @@ extern void blk_free (void); /* Allocate or resize the cache file and bitmap. */ extern int blk_set_size (uint64_t new_size); -/* Read a single block from the cache or plugin. */ +/* Read a single block from the cache or plugin. If cache_on_read is set, + * also ensure it is cached. */ extern int blk_read (struct nbdkit_next_ops *next_ops, void *nxdata, uint64_t blknum, uint8_t *block, int *err) __attribute__((__nonnull__ (1, 4, 5))); +/* If a single block is not cached, copy it from the plugin. */ +extern int blk_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + uint64_t blknum, uint8_t *block, int *err) + __attribute__((__nonnull__ (1, 4, 5))); + /* Write to the cache and the plugin. */ extern int blk_writethrough (struct nbdkit_next_ops *next_ops, void *nxdata, uint64_t blknum, const uint8_t *block, diff --git a/filters/cache/blk.c b/filters/cache/blk.c index acbed61..cf7145d 100644 --- a/filters/cache/blk.c +++ b/filters/cache/blk.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -217,6 +217,54 @@ blk_read (struct nbdkit_next_ops *next_ops, void *nxdata, } } +int +blk_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + uint64_t blknum, uint8_t *block, int *err) +{ + off_t offset = blknum * blksize; + enum bm_entry state = bitmap_get_blk (&bm, blknum, BLOCK_NOT_CACHED); + + reclaim (fd, &bm); + + nbdkit_debug ("cache: blk_cache block %" PRIu64 " (offset %" PRIu64 ") is %s", + blknum, (uint64_t) offset, + state == BLOCK_NOT_CACHED ? "not cached" : + state == BLOCK_CLEAN ? "clean" : + state == BLOCK_DIRTY ? "dirty" : + "unknown"); + + if (state == BLOCK_NOT_CACHED) { + off_t offset = blknum * blksize; + + /* Read underlying plugin, copy to cache regardless of cache-on-read. */ + if (next_ops->pread (nxdata, block, blksize, offset, 0, err) == -1) + return -1; + + nbdkit_debug ("cache: cache block %" PRIu64 " (offset %" PRIu64 ")", + blknum, (uint64_t) offset); + + if (pwrite (fd, block, blksize, offset) == -1) { + *err = errno; + nbdkit_error ("pwrite: %m"); + return -1; + } + bitmap_set_blk (&bm, blknum, BLOCK_CLEAN); + lru_set_recently_accessed (blknum); + } + else { +#if HAVE_POSIX_FADVISE + int r = posix_fadvise (fd, offset, blksize, POSIX_FADV_WILLNEED); + if (r) { + errno = r; + nbdkit_error ("posix_fadvise: %m"); + return -1; + } +#endif + lru_set_recently_accessed (blknum); + } + return 0; +} + int blk_writethrough (struct nbdkit_next_ops *next_ops, void *nxdata, uint64_t blknum, const uint8_t *block, uint32_t flags, diff --git a/filters/cache/cache.c b/filters/cache/cache.c index e215cac..2d2f39d 100644 --- a/filters/cache/cache.c +++ b/filters/cache/cache.c @@ -230,6 +230,13 @@ cache_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, return 0; } +/* Override the plugin's .can_cache, because we are caching here instead */ +static int +cache_can_cache (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) +{ + return NBDKIT_CACHE_NATIVE; +} + /* Read data. */ static int cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata, @@ -548,6 +555,50 @@ flush_dirty_block (uint64_t blknum, void *datav) return 0; /* continue scanning and flushing. */ } +/* Cache data. */ +static int +cache_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, + uint32_t flags, int *err) +{ + CLEANUP_FREE uint8_t *block = NULL; + uint64_t blknum, blkoffs; + int r; + uint64_t remaining = count; /* Rounding out could exceed 32 bits */ + + assert (!flags); + block = malloc (blksize); + if (block == NULL) { + *err = errno; + nbdkit_error ("malloc: %m"); + return -1; + } + + blknum = offset / blksize; /* block number */ + blkoffs = offset % blksize; /* offset within the block */ + + /* Unaligned head */ + remaining += blkoffs; + offset -= blkoffs; + + /* Unaligned tail */ + remaining = ROUND_UP (remaining, blksize); + + /* Aligned body */ + while (remaining) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + r = blk_cache (next_ops, nxdata, blknum, block, err); + if (r == -1) + return -1; + + remaining -= blksize; + offset += blksize; + blknum++; + } + + return 0; +} + static struct nbdkit_filter filter = { .name = "cache", .longname = "nbdkit caching filter", @@ -558,10 +609,12 @@ static struct nbdkit_filter filter = { .config_complete = cache_config_complete, .prepare = cache_prepare, .get_size = cache_get_size, + .can_cache = cache_can_cache, .pread = cache_pread, .pwrite = cache_pwrite, .zero = cache_zero, .flush = cache_flush, + .cache = cache_cache, }; NBDKIT_REGISTER_FILTER(filter) -- 2.20.1

In the delay filter, all we need to do is pass through cache operations, but with an added delay (well, technically, this isn't enabled until a later patch flips the default for .can_cache to pass-through). Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/delay/nbdkit-delay-filter.pod | 9 ++++++++- filters/delay/delay.c | 27 ++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/filters/delay/nbdkit-delay-filter.pod b/filters/delay/nbdkit-delay-filter.pod index 2e2ac74..730cea4 100644 --- a/filters/delay/nbdkit-delay-filter.pod +++ b/filters/delay/nbdkit-delay-filter.pod @@ -11,7 +11,7 @@ nbdkit-delay-filter - nbdkit delay filter nbdkit --filter=delay plugin [plugin-args ...] delay-read=(SECS|NNms) delay-write=(SECS|NNms) delay-zero=(SECS|NNms) delay-trim=(SECS|NNms) - delay-extents=(SECS|NNms) + delay-extents=(SECS|NNms) delay-cache=(SECS|NNms) =head1 DESCRIPTION @@ -71,6 +71,13 @@ Delay trim/discard operations by C<SECS> seconds or C<NN> milliseconds. Delay block status (extents) operations by C<SECS> seconds or C<NN> milliseconds. +=item B<delay-cache=>SECS + +=item B<delay-cache=>NNB<ms> + +Delay advisory cache operations by C<SECS> seconds or C<NN> +milliseconds. + =item B<wdelay=>SECS =item B<wdelay=>NNB<ms> diff --git a/filters/delay/delay.c b/filters/delay/delay.c index 862af93..486a24e 100644 --- a/filters/delay/delay.c +++ b/filters/delay/delay.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -47,6 +47,7 @@ static int delay_write_ms = 0; /* write delay (milliseconds) */ static int delay_zero_ms = 0; /* zero delay (milliseconds) */ static int delay_trim_ms = 0; /* trim delay (milliseconds) */ static int delay_extents_ms = 0;/* extents delay (milliseconds) */ +static int delay_cache_ms = 0; /* cache delay (milliseconds) */ static int parse_delay (const char *key, const char *value) @@ -116,6 +117,12 @@ extents_delay (void) delay (delay_extents_ms); } +static void +cache_delay (void) +{ + delay (delay_cache_ms); +} + /* Called for each key=value passed on the command line. */ static int delay_config (nbdkit_next_config *next, void *nxdata, @@ -167,6 +174,12 @@ delay_config (nbdkit_next_config *next, void *nxdata, return -1; return 0; } + else if (strcmp (key, "delay-cache") == 0) { + delay_cache_ms = parse_delay (key, value); + if (delay_cache_ms == -1) + return -1; + return 0; + } else return next (nxdata, key, value); } @@ -178,6 +191,7 @@ delay_config (nbdkit_next_config *next, void *nxdata, "delay-zero=<NN>[ms] Zero delay in seconds/milliseconds.\n" \ "delay-trim=<NN>[ms] Trim delay in seconds/milliseconds.\n" \ "delay-extents=<NN>[ms] Extents delay in seconds/milliseconds.\n" \ + "delay-cache=<NN>[ms] Cache delay in seconds/milliseconds.\n" \ "wdelay=<NN>[ms] Write, zero and trim delay in secs/msecs." /* Read data. */ @@ -231,6 +245,16 @@ delay_extents (struct nbdkit_next_ops *next_ops, void *nxdata, return next_ops->extents (nxdata, count, offset, flags, extents, err); } +/* Cache. */ +static int +delay_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, uint32_t flags, + int *err) +{ + cache_delay (); + return next_ops->cache (nxdata, count, offset, flags, err); +} + static struct nbdkit_filter filter = { .name = "delay", .longname = "nbdkit delay filter", @@ -242,6 +266,7 @@ static struct nbdkit_filter filter = { .zero = delay_zero, .trim = delay_trim, .extents = delay_extents, + .cache = delay_cache, }; NBDKIT_REGISTER_FILTER(filter) -- 2.20.1

In the log filter, all we need to do is copy-and-paste existing per-command logging to add the initial value of advertised cache support, as well as logging actual cache commands (well, technically, this isn't enabled until a later patch flips the default for .can_cache to pass-through). Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/log/nbdkit-log-filter.pod | 10 +++++----- filters/log/log.c | 29 +++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/filters/log/nbdkit-log-filter.pod b/filters/log/nbdkit-log-filter.pod index 973024b..9e102bc 100644 --- a/filters/log/nbdkit-log-filter.pod +++ b/filters/log/nbdkit-log-filter.pod @@ -46,11 +46,11 @@ the impact of the caching. This filter writes to the file specified by the C<logfile=FILE> parameter. All lines include a timestamp, a connection counter, then details about the command. The following actions are logged: Connect, -Read, Write, Zero, Trim, Extents, Flush, and Disconnect. Except for -Connect and Disconnect, an event is logged across two lines for call -and return value, to allow tracking duration and tracing any parallel -execution, using id for correlation (incremented per action on the -connection). +Read, Write, Zero, Trim, Extents, Cache, Flush, and Disconnect. +Except for Connect and Disconnect, an event is logged across two lines +for call and return value, to allow tracking duration and tracing any +parallel execution, using id for correlation (incremented per action +on the connection). An example logging session of a client that performs a single successful read is: diff --git a/filters/log/log.c b/filters/log/log.c index 466160e..133e352 100644 --- a/filters/log/log.c +++ b/filters/log/log.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018 Red Hat Inc. + * Copyright (C) 2018-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -243,13 +243,15 @@ log_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) int z = next_ops->can_zero (nxdata); int F = next_ops->can_fua (nxdata); int e = next_ops->can_extents (nxdata); + int c = next_ops->can_cache (nxdata); - if (size < 0 || w < 0 || f < 0 || r < 0 || t < 0 || z < 0 || F < 0 || e < 0) + if (size < 0 || w < 0 || f < 0 || r < 0 || t < 0 || z < 0 || F < 0 || + e < 0 || c < 0) return -1; output (h, "Connect", 0, "size=0x%" PRIx64 " write=%d flush=%d " - "rotational=%d trim=%d zero=%d fua=%d extents=%d", - size, w, f, r, t, z, F, e); + "rotational=%d trim=%d zero=%d fua=%d extents=%d cache=%d", + size, w, f, r, t, z, F, e, c); return 0; } @@ -396,6 +398,24 @@ log_extents (struct nbdkit_next_ops *next_ops, void *nxdata, return r; } +/* Cache data. */ +static int +log_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + struct handle *h = handle; + uint64_t id = get_id (h); + int r; + + assert (!flags); + output (h, "Cache", id, "offset=0x%" PRIx64 " count=0x%x ...", + offs, count); + r = next_ops->cache (nxdata, count, offs, flags, err); + output_return (h, "...Cache", id, r, err); + return r; +} + static struct nbdkit_filter filter = { .name = "log", .longname = "nbdkit log filter", @@ -414,6 +434,7 @@ static struct nbdkit_filter filter = { .trim = log_trim, .zero = log_zero, .extents = log_extents, + .cache = log_cache, }; NBDKIT_REGISTER_FILTER(filter) -- 2.20.1

The readahead and xz filters serve as a limited cache, so we want to advertise caching regardless of what the underlying plugin provides. But it turns out that we are best serviced by relying on nbdkit's ability to emulate caching by .pread requests. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/readahead/readahead.c | 13 +++++++++++++ filters/xz/xz.c | 15 ++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/filters/readahead/readahead.c b/filters/readahead/readahead.c index dc27bae..95bda0e 100644 --- a/filters/readahead/readahead.c +++ b/filters/readahead/readahead.c @@ -109,6 +109,18 @@ readahead_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, return r; } +/* Cache */ +static int +readahead_can_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + /* We are already operating as a cache regardless of the plugin's + * underlying .can_cache, but it's easiest to just rely on nbdkit's + * behavior of calling .pread for caching. + */ + return NBDKIT_CACHE_EMULATE; +} + /* Read data. */ static int @@ -241,6 +253,7 @@ static struct nbdkit_filter filter = { .unload = readahead_unload, .prepare = readahead_prepare, .get_size = readahead_get_size, + .can_cache = readahead_can_cache, .pread = readahead_pread, .pwrite = readahead_pwrite, .trim = readahead_trim, diff --git a/filters/xz/xz.c b/filters/xz/xz.c index 366000b..8ada294 100644 --- a/filters/xz/xz.c +++ b/filters/xz/xz.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2018 Red Hat Inc. + * Copyright (C) 2013-2019 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -194,6 +194,18 @@ xz_can_extents (struct nbdkit_next_ops *next_ops, void *nxdata, return 0; } +/* Cache */ +static int +xz_can_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + /* We are already operating as a cache regardless of the plugin's + * underlying .can_cache, but it's easiest to just rely on nbdkit's + * behavior of calling .pread for caching. + */ + return NBDKIT_CACHE_EMULATE; +} + /* Read data from the file. */ static int xz_pread (struct nbdkit_next_ops *next_ops, void *nxdata, @@ -248,6 +260,7 @@ static struct nbdkit_filter filter = { .get_size = xz_get_size, .can_write = xz_can_write, .can_extents = xz_can_extents, + .can_cache = xz_can_cache, .pread = xz_pread, }; -- 2.20.1

In the truncate filter, all we need to do is copy-and-paste existing per-command count rewriting (well, technically, this isn't enabled until a later patch flips the default for .can_cache to pass-through). Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/truncate/truncate.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/filters/truncate/truncate.c b/filters/truncate/truncate.c index 6408c35..38b1cd9 100644 --- a/filters/truncate/truncate.c +++ b/filters/truncate/truncate.c @@ -354,6 +354,29 @@ truncate_extents (struct nbdkit_next_ops *next_ops, void *nxdata, return 0; } +/* Cache. */ +static int +truncate_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offset, + uint32_t flags, int *err) +{ + int r; + uint32_t n; + struct handle *h = handle; + + if (offset < h->real_size) { + if (offset + count <= h->real_size) + n = count; + else + n = h->real_size - offset; + r = next_ops->cache (nxdata, n, offset, flags, err); + if (r == -1) + return -1; + } + + return 0; +} + static struct nbdkit_filter filter = { .name = "truncate", .longname = "nbdkit truncate filter", @@ -369,6 +392,7 @@ static struct nbdkit_filter filter = { .trim = truncate_trim, .zero = truncate_zero, .extents = truncate_extents, + .cache = truncate_cache, }; NBDKIT_REGISTER_FILTER(filter) -- 2.20.1

Similar to the existing fua, nozero and noextents filters, add a filter to make it easy to override the basic caching functionality, in part to facilitate timing tests of whether a plugin's cache implementation is worthwhile. A worthwhile test to add to the testsuite would connect the log filter both before and after the nocache filter, to prove how caching requests are altered. However, until we use libnbd in the nbdkit testsuite, there isn't really a clean way to trigger NBD_CMD_CACHE calls to test this (qemu-nbd can't do it). But I can at least exploit test-eflags to use 'qemu-nbd --list' to demonstrate that the advertised flag changes, with a little tweak to allow the test to run filters with arguments. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/fua/nbdkit-fua-filter.pod | 5 +- filters/nocache/nbdkit-nocache-filter.pod | 69 +++++++++++ filters/noextents/nbdkit-noextents-filter.pod | 2 + filters/nozero/nbdkit-nozero-filter.pod | 2 + configure.ac | 2 + filters/nocache/nocache.c | 113 ++++++++++++++++++ filters/nocache/Makefile.am | 61 ++++++++++ tests/test-eflags.sh | 21 +++- 8 files changed, 272 insertions(+), 3 deletions(-) create mode 100644 filters/nocache/nbdkit-nocache-filter.pod create mode 100644 filters/nocache/nocache.c create mode 100644 filters/nocache/Makefile.am diff --git a/filters/fua/nbdkit-fua-filter.pod b/filters/fua/nbdkit-fua-filter.pod index bd08a8c..b76917b 100644 --- a/filters/fua/nbdkit-fua-filter.pod +++ b/filters/fua/nbdkit-fua-filter.pod @@ -66,7 +66,10 @@ L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-blocksize-filter(1)>, -L<nbdkit-log-filter(1)>. +L<nbdkit-log-filter(1)>, +L<nbdkit-nocache-filter(1)>, +L<nbdkit-noextents-filter(1)>, +L<nbdkit-nozero-filter(1)>. =head1 AUTHORS diff --git a/filters/nocache/nbdkit-nocache-filter.pod b/filters/nocache/nbdkit-nocache-filter.pod new file mode 100644 index 0000000..0f43433 --- /dev/null +++ b/filters/nocache/nbdkit-nocache-filter.pod @@ -0,0 +1,69 @@ +=head1 NAME + +nbdkit-nocache-filter - nbdkit nocache filter + +=head1 SYNOPSIS + + nbdkit --filter=nocache plugin [cachemode=MODE] [plugin-args...] + +=head1 DESCRIPTION + +C<nbdkit-nocache-filter> is a filter that intentionally disables +efficient handling of advisory client cache requests across the NBD +protocol. It is mainly useful for evaluating timing differences to +determine the impact of caching requests. + +Note that the effects of this filter (in crippling handling of client +cache requests) is somewhat orthogonal from that of the +L<nbdkit-cache-filter(1)> (adding local caching of client read/write +requests); the two filters can be run together to experiment with +timings. + +=head1 PARAMETERS + +=over 4 + +=item B<cachemode=none|emulate|nop> + +Optional, controls which mode the filter will use. Mode B<none> +(default) means that cache support is not advertised to the +client. Mode B<emulate> means that cache support is emulated by the +filter using the plugin's C<pread> callback, regardless of whether the +plugin itself implemented the C<cache> callback. Mode B<nop> means +that cache requests are always accepted and immediately ignored, +rather than having any actual impact. + +=back + +=head1 EXAMPLES + +Serve the file F<disk.img>, but prevent C<NBD_CMD_CACHE> requests +altogether, to get a baseline timing of behavior when the client is +unable to make cache requests: + + nbdkit --filter=nocache file disk.img + +Serve the file F<disk.img>, but with cache requests silently ignored, +rather than being forwarded on to the file plugin (which attempts to +use L<posix_fadvise(3)>), to compare against the timings without the +filter and determine whether the file plugin caching was worthwhile: + + nbdkit --filter=nocache file disk.img cachemode=nop + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-file-plugin(1)>, +L<nbdkit-filter(3)>, +L<nbdkit-cache-filter(1)>, +L<nbdkit-fua-filter(1)>, +L<nbdkit-noextents-filter(1)>, +L<nbdkit-nozero-filter(1)>. + +=head1 AUTHORS + +Eric Blake + +=head1 COPYRIGHT + +Copyright (C) 2019 Red Hat Inc. diff --git a/filters/noextents/nbdkit-noextents-filter.pod b/filters/noextents/nbdkit-noextents-filter.pod index 46f6bdb..24519a0 100644 --- a/filters/noextents/nbdkit-noextents-filter.pod +++ b/filters/noextents/nbdkit-noextents-filter.pod @@ -27,6 +27,8 @@ plugin in the normal way. L<nbdkit(1)>, L<nbdkit-filter(3)>, +L<nbdkit-fua-filter(1)>, +L<nbdkit-nocache-filter(1)>, L<nbdkit-nozero-filter(1)>, L<nbdkit-file-plugin(1)>. diff --git a/filters/nozero/nbdkit-nozero-filter.pod b/filters/nozero/nbdkit-nozero-filter.pod index 8e694bb..d94dd8d 100644 --- a/filters/nozero/nbdkit-nozero-filter.pod +++ b/filters/nozero/nbdkit-nozero-filter.pod @@ -52,6 +52,8 @@ the data to be written explicitly rather than punching any holes: L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, +L<nbdkit-fua-filter(1)>, +L<nbdkit-nocache-filter(1)>, L<nbdkit-noextents-filter(1)>. =head1 AUTHORS diff --git a/configure.ac b/configure.ac index 06124c5..2163930 100644 --- a/configure.ac +++ b/configure.ac @@ -831,6 +831,7 @@ filters="\ error \ fua \ log \ + nocache \ noextents \ nozero \ offset \ @@ -906,6 +907,7 @@ AC_CONFIG_FILES([Makefile filters/error/Makefile filters/fua/Makefile filters/log/Makefile + filters/nocache/Makefile filters/noextents/Makefile filters/nozero/Makefile filters/offset/Makefile diff --git a/filters/nocache/nocache.c b/filters/nocache/nocache.c new file mode 100644 index 0000000..abb042e --- /dev/null +++ b/filters/nocache/nocache.c @@ -0,0 +1,113 @@ +/* nbdkit + * Copyright (C) 2018-2019 Red Hat Inc. + * + * 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. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <string.h> +#include <stdbool.h> +#include <assert.h> + +#include <nbdkit-filter.h> + +#include "minmax.h" + +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL + +static enum CacheMode { + NONE, + EMULATE, + NOP, +} cachemode; + +static int +nocache_config (nbdkit_next_config *next, void *nxdata, + const char *key, const char *value) +{ + if (strcmp (key, "cachemode") == 0) { + if (strcmp (value, "emulate") == 0) + cachemode = EMULATE; + else if (strcmp (value, "nop") == 0 || + strcmp (value, "no-op") == 0) + cachemode = NOP; + else if (strcmp (value, "none") != 0) { + nbdkit_error ("unknown cachemode '%s'", value); + return -1; + } + return 0; + } + return next (nxdata, key, value); +} + +#define nocache_config_help \ + "cachemode=<MODE> Either 'none' (default), 'emulate', or 'nop'.\n" \ + +/* Advertise desired FLAG_SEND_CACHE mode. */ +static int +nocache_can_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + switch (cachemode) { + case NONE: + return NBDKIT_CACHE_NONE; + case EMULATE: + return NBDKIT_CACHE_EMULATE; + case NOP: + return NBDKIT_CACHE_NATIVE; + } + abort (); +} + +static int +nocache_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + assert (cachemode == NOP); + assert (!flags); + + return 0; +} + +static struct nbdkit_filter filter = { + .name = "nocache", + .longname = "nbdkit nocache filter", + .version = PACKAGE_VERSION, + .config = nocache_config, + .config_help = nocache_config_help, + .can_cache = nocache_can_cache, + .cache = nocache_cache, +}; + +NBDKIT_REGISTER_FILTER(filter) diff --git a/filters/nocache/Makefile.am b/filters/nocache/Makefile.am new file mode 100644 index 0000000..aa83937 --- /dev/null +++ b/filters/nocache/Makefile.am @@ -0,0 +1,61 @@ +# nbdkit +# Copyright (C) 2018-2019 Red Hat Inc. +# +# 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. + +include $(top_srcdir)/common-rules.mk + +EXTRA_DIST = nbdkit-nocache-filter.pod + +filter_LTLIBRARIES = nbdkit-nocache-filter.la + +nbdkit_nocache_filter_la_SOURCES = \ + nocache.c \ + $(top_srcdir)/include/nbdkit-filter.h + +nbdkit_nocache_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/include +nbdkit_nocache_filter_la_CFLAGS = \ + $(WARNINGS_CFLAGS) +nbdkit_nocache_filter_la_LDFLAGS = \ + -module -avoid-version -shared \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms + +if HAVE_POD + +man_MANS = nbdkit-nocache-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-nocache-filter.1: nbdkit-nocache-filter.pod + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/tests/test-eflags.sh b/tests/test-eflags.sh index eaaaae0..14a0099 100755 --- a/tests/test-eflags.sh +++ b/tests/test-eflags.sh @@ -51,6 +51,7 @@ if ! qemu-nbd --help | grep -sq -- --list; then fi files="eflags.out" +late_args= rm -f $files cleanup_fn rm -f $files @@ -70,7 +71,7 @@ SEND_CACHE=$(( 1 << 10 )) do_nbdkit () { - nbdkit -v -U - "$@" sh - --run 'qemu-nbd --list -k $unixsocket' | + nbdkit -v -U - "$@" sh - $late_args --run 'qemu-nbd --list -k $unixsocket' | grep -E "flags: 0x" | grep -Eoi '0x[a-f0-9]+' > eflags.out echo -n eflags=; cat eflags.out @@ -289,7 +290,7 @@ EOF #---------------------------------------------------------------------- # -r -# can_cache=true +# can_cache=native do_nbdkit -r <<'EOF' case "$1" in @@ -301,3 +302,19 @@ EOF [ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE )) ] || fail "expected HAS_FLAGS|READ_ONLY|SEND_DF|SEND_CACHE" + +#---------------------------------------------------------------------- +# -r +# --filter=nocache cachemode=none +# can_cache=native + +late_args="cachemode=none" do_nbdkit -r --filter=nocache <<'EOF' +case "$1" in + get_size) echo 1M ;; + can_cache) echo "emulate" ;; + *) exit 2 ;; +esac +EOF + +[ $eflags -eq $(( HAS_FLAGS|READ_ONLY|SEND_DF )) ] || + fail "expected HAS_FLAGS|READ_ONLY|SEND_DF" -- 2.20.1