Libguestfs - Feb 2021 - [nbdkit PATCH] readahead: Fix multi-conn inconsistency

The readahead filter was blindly passing through the plugin's
.can_multi_conn, but does not kill the readahead buffer on a flush.
This is a bug in the following scenario, when the plugin advertised
multi-conn:

connA              connB
write
  wipes our buffer
  plugin caches the write
                   read
		     reads stale data into buffer
flush
  plugin finally flushes the write
                   read
		     data in buffer is still stale

In order to preserve the plugin's multi-conn status, we MUST drop our
cache any time we flush, so that our next read picks up whatever got
flushed from another connection.

But when the server lacks multi-conn, we don't have to kill our
readahead buffer on flush, because the client already has no guarantee
that a flush from one connection will affect the read seen by another
(killing on write was already good enough to maintain semantics).
---
 filters/readahead/readahead.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/filters/readahead/readahead.c b/filters/readahead/readahead.c
index 8163c4fb..a97c5da5 100644
--- a/filters/readahead/readahead.c
+++ b/filters/readahead/readahead.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2019 Red Hat Inc.
+ * Copyright (C) 2019-2021 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -244,6 +244,15 @@ readahead_zero (struct nbdkit_next_ops *next_ops, void
*nxdata,
   return next_ops->zero (nxdata, count, offset, flags, err);
 }

+static int
+readahead_flush (struct nbdkit_next_ops *next_ops, void *nxdata,
+                 void *handle, uint32_t flags, int *err)
+{
+  if (next_ops->can_multi_conn(nxdata) != 1)
+    kill_readahead ();
+  return next_ops->flush (nxdata, flags, err);
+}
+
 static struct nbdkit_filter filter = {
   .name              = "readahead",
   .longname          = "nbdkit readahead filter",
@@ -255,6 +264,7 @@ static struct nbdkit_filter filter = {
   .pwrite            = readahead_pwrite,
   .trim              = readahead_trim,
   .zero              = readahead_zero,
+  .flush             = readahead_flush,
 };

 NBDKIT_REGISTER_FILTER(filter)
-- 
2.30.1

On 2/19/21 3:03 PM, Eric Blake wrote:
> The readahead filter was blindly passing through the plugin's
> .can_multi_conn, but does not kill the readahead buffer on a flush.
> This is a bug in the following scenario, when the plugin advertised
> multi-conn:
> 
> In order to preserve the plugin's multi-conn status, we MUST drop our
> cache any time we flush, so that our next read picks up whatever got
> flushed from another connection.
> 
> But when the server lacks multi-conn, we don't have to kill our
> readahead buffer on flush, because the client already has no guarantee
> that a flush from one connection will affect the read seen by another
> (killing on write was already good enough to maintain semantics).
> ---
> +static int
> +readahead_flush (struct nbdkit_next_ops *next_ops, void *nxdata,
> +                 void *handle, uint32_t flags, int *err)
> +{
> +  if (next_ops->can_multi_conn(nxdata) != 1)
That should be ==, to match my commit message.
> +    kill_readahead ();
> +  return next_ops->flush (nxdata, flags, err);
> +}
> +
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org