From 0ef9cfedf2e5dcdf4f4182aed92b402738ab00ca Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Fri, 30 Aug 2019 17:45:34 -0500
Subject: [PATCH] server: Remember .open(readonly) status

The previous patch argued that globally affecting .can_write based on
'-r' (the global 'readonly') prevents the ability for a filter to call
next_open(nxdata, false) to purposefully write to the plugin, even
while advertising .can_write=0 to the client.  But it also
demonstrated that when a filter passes readonly=true to next_open, we
were still making needless probes into the plugin, for something the
filter won't be using.

This patch improves things to prime the .can_write cache according to
.open.  The same script from the previous patch now fails a lot faster
under -r (since nozero is not a filter that changes readonly=false to
true, the plugin now fails .can_zero quickly):

$ /bin/time -f %e ./nbdkit -U - -r --filter=nozero sh script zeromode=notrim \
  --run 'qemu-nbd --list -k $unixsocket >/dev/null'
nbdkit: sh[1]: error: zeromode 'notrim' requires plugin zero support
qemu-nbd: Failed to read initial magic: Unexpected end-of-file before all bytes were read
Command exited with non-zero status 1
0.06

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/backend.c     | 20 +++++++++++++++-----
 server/connections.c |  2 +-
 server/filters.c     |  6 ++----
 server/internal.h    |  3 +++
 server/plugins.c     |  2 --
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/server/backend.c b/server/backend.c
index 749b1f15..ebdef63a 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -167,6 +167,20 @@ backend_unload (struct backend *b, void (*unload) (void))
   free (b->name);
 }
 
+int
+backend_open (struct backend *b, struct connection *conn, int readonly)
+{
+  struct b_conn_handle *h = &conn->handles[b->i];
+
+  debug ("%s: open readonly=%d", b->name, readonly);
+
+  assert (h->handle == NULL);
+  assert (h->can_write == -1);
+  if (readonly)
+    h->can_write = 0;
+  return b->open (b, conn, readonly);
+}
+
 void
 backend_set_handle (struct backend *b, struct connection *conn, void *handle)
 {
@@ -195,12 +209,8 @@ backend_can_write (struct backend *b, struct connection *conn)
 
   debug ("%s: can_write", b->name);
 
-  if (h->can_write == -1) {
-    /* Special case for outermost backend when -r is in effect. */
-    if (readonly && b == backend)
-      return h->can_write = 0;
+  if (h->can_write == -1)
     h->can_write = b->can_write (b, conn);
-  }
   return h->can_write;
 }
 
diff --git a/server/connections.c b/server/connections.c
index 7609e9a7..0c1f2413 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -149,7 +149,7 @@ _handle_single_connection (int sockin, int sockout)
     goto done;
 
   lock_request (conn);
-  r = backend->open (backend, conn, readonly);
+  r = backend_open (backend, conn, readonly);
   unlock_request (conn);
   if (r == -1)
     goto done;
diff --git a/server/filters.c b/server/filters.c
index 0e10816f..a8d7dd7c 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -195,7 +195,7 @@ next_open (void *nxdata, int readonly)
 {
   struct b_conn *b_conn = nxdata;
 
-  return b_conn->b->open (b_conn->b, b_conn->conn, readonly);
+  return backend_open (b_conn->b, b_conn->conn, readonly);
 }
 
 static int
@@ -205,8 +205,6 @@ filter_open (struct backend *b, struct connection *conn, int readonly)
   struct b_conn nxdata = { .b = b->next, .conn = conn };
   void *handle;
 
-  debug ("%s: open readonly=%d", b->name, readonly);
-
   if (f->filter.open) {
     handle = f->filter.open (next_open, &nxdata, readonly);
     if (handle == NULL)
@@ -215,7 +213,7 @@ filter_open (struct backend *b, struct connection *conn, int readonly)
     return 0;
   }
   else
-    return b->next->open (b->next, conn, readonly);
+    return backend_open (b->next, conn, readonly);
 }
 
 static void
diff --git a/server/internal.h b/server/internal.h
index a55d6406..fb197b9e 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -325,6 +325,9 @@ extern void backend_load (struct backend *b, const char *name,
 extern void backend_unload (struct backend *b, void (*unload) (void))
   __attribute__((__nonnull__ (1)));
 
+extern int backend_open (struct backend *b, struct connection *conn,
+                         int readonly)
+  __attribute__((__nonnull__ (1, 2)));
 extern void backend_set_handle (struct backend *b, struct connection *conn,
                                 void *handle)
   __attribute__((__nonnull__ (1, 2 /* not 3 */)));
diff --git a/server/plugins.c b/server/plugins.c
index 1dec4008..1c4b5f50 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -241,8 +241,6 @@ plugin_open (struct backend *b, struct connection *conn, int readonly)
   assert (connection_get_handle (conn, 0) == NULL);
   assert (p->plugin.open != NULL);
 
-  debug ("%s: open readonly=%d", b->name, readonly);
-
   handle = p->plugin.open (readonly);
   if (!handle)
     return -1;
-- 
2.11.4.GIT