[Libguestfs] [nbdkit PATCH] blocksize: Allow parallel requests

[Eric Blake]

The only time where we have to be careful is when the client sends
unaligned data; if we add a lock around our use of the bounce buffer,
then we can provide the same parallelism as the underlying plugin for
all other transactions.

When we first added .can_multi_conn, we waffled on what the blocksize
filter should do[1]; blindly slamming to 1 when all requests are
serial would have been correct at the time, but we opted to stay
conservative by instead slamming things to 0 with a promise to revisit
it when changing the thread model.  Now that we are allowing parallel
transactions, blindly slamming to 1 is no longer a valid option, but
slamming to 0 is overkill when we can instead rely on passthrough to
the plugin's .can_multi_conn.

[1] https://listman.redhat.com/archives/libguestfs/2019-January/msg00074.html
---
 filters/blocksize/blocksize.c | 42 +++++++++++++----------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
index c3a2d60d..81d0e76c 100644
--- a/filters/blocksize/blocksize.c
+++ b/filters/blocksize/blocksize.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2018-2020 Red Hat Inc.
+ * Copyright (C) 2018-2021 Red Hat Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -49,23 +49,20 @@

 #define BLOCKSIZE_MIN_LIMIT (64U * 1024)

-/* As long as we don't have parallel requests, we can reuse a common
- * buffer for alignment purposes; size it to the maximum we allow for
- * minblock. */
+/* In order to handle parallel requests safely, this lock must be held
+ * when using the bounce buffer.
+ */
+static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
+
+/* A single bounce buffer for alignment purposes, guarded by the lock.
+ * Size it to the maximum we allow for minblock.
+ */
 static char bounce[BLOCKSIZE_MIN_LIMIT];
+
 static unsigned int minblock;
 static unsigned int maxdata;
 static unsigned int maxlen;

-/* We are using a common bounce buffer (see above) so we must
- * serialize all requests.
- */
-static int
-blocksize_thread_model (void)
-{
-  return NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS;
-}
-
 static int
 blocksize_parse (const char *name, const char *s, unsigned int *v)
 {
@@ -157,17 +154,6 @@ blocksize_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
   return ROUND_DOWN (size, minblock);
 }

-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;
-}
-
 static int
 blocksize_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
                  void *handle, void *b, uint32_t count, uint64_t offs,
@@ -179,6 +165,7 @@ blocksize_pread (struct nbdkit_next_ops *next_ops, void *nxdata,

   /* Unaligned head */
   if (offs & (minblock - 1)) {
+    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
     drop = offs & (minblock - 1);
     keep = MIN (minblock - drop, count);
     if (next_ops->pread (nxdata, bounce, minblock, offs - drop, flags,
@@ -202,6 +189,7 @@ blocksize_pread (struct nbdkit_next_ops *next_ops, void *nxdata,

   /* Unaligned tail */
   if (count) {
+    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
     if (next_ops->pread (nxdata, bounce, minblock, offs, flags, err) == -1)
       return -1;
     memcpy (buf, bounce, count);
@@ -228,6 +216,7 @@ blocksize_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,

   /* Unaligned head */
   if (offs & (minblock - 1)) {
+    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
     drop = offs & (minblock - 1);
     keep = MIN (minblock - drop, count);
     if (next_ops->pread (nxdata, bounce, minblock, offs - drop, 0, err) == -1)
@@ -253,6 +242,7 @@ blocksize_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,

   /* Unaligned tail */
   if (count) {
+    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
     if (next_ops->pread (nxdata, bounce, minblock, offs, 0, err) == -1)
       return -1;
     memcpy (bounce, buf, count);
@@ -332,6 +322,7 @@ blocksize_zero (struct nbdkit_next_ops *next_ops, void *nxdata,

   /* Unaligned head */
   if (offs & (minblock - 1)) {
+    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
     drop = offs & (minblock - 1);
     keep = MIN (minblock - drop, count);
     if (next_ops->pread (nxdata, bounce, minblock, offs - drop, 0, err) == -1)
@@ -355,6 +346,7 @@ blocksize_zero (struct nbdkit_next_ops *next_ops, void *nxdata,

   /* Unaligned tail */
   if (count) {
+    ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
     if (next_ops->pread (nxdata, bounce, minblock, offs, 0, err) == -1)
       return -1;
     memset (bounce, 0, count);
@@ -437,12 +429,10 @@ blocksize_cache (struct nbdkit_next_ops *next_ops, void *nxdata,
 static struct nbdkit_filter filter = {
   .name              = "blocksize",
   .longname          = "nbdkit blocksize filter",
-  .thread_model      = blocksize_thread_model,
   .config            = blocksize_config,
   .config_complete   = blocksize_config_complete,
   .config_help       = blocksize_config_help,
   .get_size          = blocksize_get_size,
-  .can_multi_conn    = blocksize_can_multi_conn,
   .pread             = blocksize_pread,
   .pwrite            = blocksize_pwrite,
   .trim              = blocksize_trim,
-- 
2.30.1