On 1/5/19 8:50 AM, Richard W.M. Jones wrote:
I examined each filter to see which ones implement a cache and do
not
properly consider consistency across clients for flush requests. For
these filters we should force .can_multi_conn to return false.
I believe only one filter (blocksize) needs to be updated and all the
other ones are safe.
---
filters/blocksize/blocksize.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
index 34f58c9..9e0e713 100644
--- a/filters/blocksize/blocksize.c
+++ b/filters/blocksize/blocksize.c
@@ -164,6 +164,17 @@ blocksize_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
return size == -1 ? size : size & ~(minblock - 1);
}
+static int
+blocksize_can_multi_conn (struct nbdkit_next_ops *next_ops, void *nxdata,
+ void *handle)
+{
+ /* Although we are serializing all requests anyway so this likely
+ * doesn't make a difference, return false because the bounce buffer
+ * is not consistent for flush.
+ */
+ return 0;
+}
I think we could get away with returning 1, _because_ we are serializing
all requests. Even though we make multiple calls into the plugin
(worst-case being read-modify-write-flush on a client write-with-FUA),
no other operation can compete with a client flush. But I also agree
that if we were to fix the bounce buffer to be thread-safe and change
the filter to be more parallel, then you are also right that unless we
add a .flush callback and add locking so that all calls into the plugin
are guarded within the same lock, then returning 0 is appropriate, even
if the plugin itself returns 1. So I agree with adding this callback,
and could go either way with you returning 0 or 1 for now.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org