[Libguestfs] [nbdkit PATCH v2 1/2] blocksize: Process requests in linear order

[Eric Blake]

Mixing the blocksize filter with the streaming plugin is likely to
fail, because streaming does not handle read-modify-write. Still, the
idea that a plugin may expect data in linear order, rather than a
filter rewriting things into random-access order, is something worth
addressing - as such, the blocksize filter should not handle an
unaligned tail until after the aligned body of a large request.

Note that blocksize, cache, and cow all do block fragmentation and
bounce-buffer alignment, but only the blocksize filter was doing
random-access visiting of the tail before the body (conversely, the
blocksize filter is lazy and relies on serialized access to reuse a
single bounce buffer, while the other two malloc on demand and used
the bounce buffer more frequently).  It is likely that in the future,
we will migrate the best parts of the different block handling code
implementations into nbdkit proper, and allow plugins/filters the
ability to report their preferred alignment and block sizes, at which
point the blocksize filter would be a much thinner task of just
altering the numbers that would otherwise be reported by the
plugin. For now, getting the logic consistent between the three
filters will make that future refactoring easier.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 filters/blocksize/blocksize.c | 81 ++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 45 deletions(-)

diff --git a/filters/blocksize/blocksize.c b/filters/blocksize/blocksize.c
index 4f3e9a3..47bea96 100644
--- a/filters/blocksize/blocksize.c
+++ b/filters/blocksize/blocksize.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
@@ -197,19 +197,9 @@ blocksize_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
     count -= keep;
   }

-  /* Unaligned tail */
-  if (count & (minblock - 1)) {
-    keep = count & (minblock - 1);
-    count -= keep;
-    if (next_ops->pread (nxdata, bounce, minblock, offs + count, flags,
-                         err) == -1)
-      return -1;
-    memcpy (buf + count, bounce, keep);
-  }
-
   /* Aligned body */
-  while (count) {
-    keep = MIN (maxdata, count);
+  while (count >= minblock) {
+    keep = MIN (maxdata, ROUND_DOWN (count, minblock));
     if (next_ops->pread (nxdata, buf, keep, offs, flags, err) == -1)
       return -1;
     buf += keep;
@@ -217,6 +207,13 @@ blocksize_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
     count -= keep;
   }

+  /* Unaligned tail */
+  if (count) {
+    if (next_ops->pread (nxdata, bounce, minblock, offs, flags, err) == -1)
+      return -1;
+    memcpy (buf, bounce, count);
+  }
+
   return 0;
 }

@@ -251,21 +248,9 @@ blocksize_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,
     count -= keep;
   }

-  /* Unaligned tail */
-  if (count & (minblock - 1)) {
-    keep = count & (minblock - 1);
-    count -= keep;
-    if (next_ops->pread (nxdata, bounce, minblock, offs + count, 0, err) == -1)
-      return -1;
-    memcpy (bounce, buf + count, keep);
-    if (next_ops->pwrite (nxdata, bounce, minblock, offs + count, flags,
-                          err) == -1)
-      return -1;
-  }
-
   /* Aligned body */
-  while (count) {
-    keep = MIN (maxdata, count);
+  while (count >= minblock) {
+    keep = MIN (maxdata, ROUND_DOWN (count, minblock));
     if (next_ops->pwrite (nxdata, buf, keep, offs, flags, err) == -1)
       return -1;
     buf += keep;
@@ -273,6 +258,15 @@ blocksize_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,
     count -= keep;
   }

+  /* Unaligned tail */
+  if (count) {
+    if (next_ops->pread (nxdata, bounce, minblock, offs, 0, err) == -1)
+      return -1;
+    memcpy (bounce, buf, count);
+    if (next_ops->pwrite (nxdata, bounce, minblock, offs, flags, err) == -1)
+      return -1;
+  }
+
   if (need_flush)
     return next_ops->flush (nxdata, 0, err);
   return 0;
@@ -292,16 +286,15 @@ blocksize_trim (struct nbdkit_next_ops *next_ops, void *nxdata,
     need_flush = true;
   }

-  /* Unaligned head */
+  /* Ignore unaligned head */
   if (offs & (minblock - 1)) {
     keep = MIN (minblock - (offs & (minblock - 1)), count);
     offs += keep;
     count -= keep;
   }

-  /* Unaligned tail */
-  if (count & (minblock - 1))
-    count -= count & (minblock - 1);
+  /* Ignore unaligned tail */
+  count = ROUND_DOWN (count, minblock);

   /* Aligned body */
   while (count) {
@@ -346,27 +339,25 @@ blocksize_zero (struct nbdkit_next_ops *next_ops, void *nxdata,
     count -= keep;
   }

-  /* Unaligned tail */
-  if (count & (minblock - 1)) {
-    keep = count & (minblock - 1);
-    count -= keep;
-    if (next_ops->pread (nxdata, bounce, minblock, offs + count, 0, err) == -1)
-      return -1;
-    memset (bounce, 0, keep);
-    if (next_ops->pwrite (nxdata, bounce, minblock, offs + count,
-                          flags & ~NBDKIT_FLAG_MAY_TRIM, err) == -1)
-      return -1;
-  }
-
   /* Aligned body */
-  while (count) {
-    keep = MIN (maxlen, count);
+  while (count >= minblock) {
+    keep = MIN (maxlen, ROUND_DOWN (count, minblock));
     if (next_ops->zero (nxdata, keep, offs, flags, err) == -1)
       return -1;
     offs += keep;
     count -= keep;
   }

+  /* Unaligned tail */
+  if (count) {
+    if (next_ops->pread (nxdata, bounce, minblock, offs, 0, err) == -1)
+      return -1;
+    memset (bounce, 0, count);
+    if (next_ops->pwrite (nxdata, bounce, minblock, offs,
+                          flags & ~NBDKIT_FLAG_MAY_TRIM, err) == -1)
+      return -1;
+  }
+
   if (need_flush)
     return next_ops->flush (nxdata, 0, err);
   return 0;
-- 
2.20.1