[Libguestfs] [PATCH nbdkit] plugins: file: More standard cache mode names

Nir Soffer nirsof at gmail.com
Fri Aug 7 22:24:02 UTC 2020


The new cache=none mode is misleading since it does not avoid usage of
the page cache. When using shared storage, we may get stale data from
the page cache. When writing, we flush after every write which is
inefficient and unneeded.

Rename the cache modes to:
- writeback - write complete when the system call returned, and the data
  was copied to the page cache.
- writethrough - write completes when the data reach storage. read marks
  pages as not needed to minimize use of the page cache.

These terms are more aligned with other software like qemu or LIO and
common caching concepts.

[1] https://www.linuxjournal.com/article/7105
[2] https://www.qemu.org/docs/master/system/invocation.html
    (see cache=cache)

Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
 plugins/file/file.c                 | 22 +++++++++++-----------
 plugins/file/nbdkit-file-plugin.pod | 19 +++++++++++++------
 tests/test-gzip.c                   |  2 +-
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/plugins/file/file.c b/plugins/file/file.c
index f6f91955..deac9b7f 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -76,7 +76,7 @@ static int fadvise_mode =
   ;
 
 /* cache mode */
-static enum { cache_default, cache_none } cache_mode = cache_default;
+static enum { cache_writeback, cache_writethrough } cache_mode = cache_writeback;
 
 /* Any callbacks using lseek must be protected by this lock. */
 static pthread_mutex_t lseek_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -140,10 +140,10 @@ file_config (const char *key, const char *value)
     }
   }
   else if (strcmp (key, "cache") == 0) {
-    if (strcmp (value, "default") == 0)
-      cache_mode = cache_default;
-    else if (strcmp (value, "none") == 0)
-      cache_mode = cache_none;
+    if (strcmp (value, "writeback") == 0)
+      cache_mode = cache_writeback;
+    else if (strcmp (value, "writethrough") == 0)
+      cache_mode = cache_writethrough;
     else {
       nbdkit_error ("unknown cache mode: %s", value);
       return -1;
@@ -423,7 +423,7 @@ file_pread (void *handle, void *buf, uint32_t count, uint64_t offset,
 
 #ifdef HAVE_POSIX_FADVISE
   /* On Linux this will evict the pages we just read from the page cache. */
-  if (cache_mode == cache_none)
+  if (cache_mode == cache_writethrough)
     posix_fadvise (h->fd, orig_offset, orig_count, POSIX_FADV_DONTNEED);
 #endif
 
@@ -441,11 +441,11 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
   uint32_t orig_count = count;
   uint64_t orig_offset = offset;
 
-  /* If cache=none we want to force pages we have just written to the
-   * file to be flushed to disk so we can immediately evict them from
-   * the page cache.
+  /* If cache=writethrough we want to force pages we have just written
+   * to the file to be flushed to disk so we can immediately evict them
+   * from the page cache.
    */
-  if (cache_mode == cache_none) flags |= NBDKIT_FLAG_FUA;
+  if (cache_mode == cache_writethrough) flags |= NBDKIT_FLAG_FUA;
 #endif
 
   while (count > 0) {
@@ -464,7 +464,7 @@ file_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset,
 
 #ifdef HAVE_POSIX_FADVISE
   /* On Linux this will evict the pages we just wrote from the page cache. */
-  if (cache_mode == cache_none)
+  if (cache_mode == cache_writethrough)
     posix_fadvise (h->fd, orig_offset, orig_count, POSIX_FADV_DONTNEED);
 #endif
 
diff --git a/plugins/file/nbdkit-file-plugin.pod b/plugins/file/nbdkit-file-plugin.pod
index e2ae0b1b..6a5bd088 100644
--- a/plugins/file/nbdkit-file-plugin.pod
+++ b/plugins/file/nbdkit-file-plugin.pod
@@ -5,7 +5,7 @@ nbdkit-file-plugin - nbdkit file plugin
 =head1 SYNOPSIS
 
  nbdkit file [file=]FILENAME
-             [cache=default|none] [fadvise=normal|random|sequential]
+             [cache=writeback|writethrough] [fadvise=normal|random|sequential]
 
 =head1 DESCRIPTION
 
@@ -18,12 +18,19 @@ It serves the named C<FILENAME> over NBD.  Local block devices
 
 =over 4
 
-=item B<cache=default>
+=item B<cache=writeback>
 
-=item B<cache=none>
+=item B<cache=writethrough>
 
-Using C<cache=none> tries to prevent the kernel from keeping parts of
-the file that have already been read or written in the page cache.
+Using C<cache=writeback>, write will be considered as completed as soon
+as the the data is store in the kernel page cache, before it reached the
+actual stoarge.
+
+Using C<cache=writethrough>, write will be considered as completed only
+when the data reach actual storage, minimizing use of kernel page cache.
+reading will try to minimize use of the page cache.
+
+The default is C<writeback>.
 
 =item B<fadvise=normal>
 
@@ -73,7 +80,7 @@ the file sequentially one time (eg for making a single copy or backup)
 then this will stop other processes from being evicted from the page
 cache:
 
- nbdkit file disk.img fadvise=sequential cache=none
+ nbdkit file disk.img fadvise=sequential cache=writethrough
 
 =head2 Files on tmpfs
 
diff --git a/tests/test-gzip.c b/tests/test-gzip.c
index 8f81c5b7..b1685098 100644
--- a/tests/test-gzip.c
+++ b/tests/test-gzip.c
@@ -62,7 +62,7 @@ main (int argc, char *argv[])
 
   /* Test the new filter. */
   if (test_start_nbdkit ("file", "--filter=gzip", disk,
-                         "fadvise=sequential", "cache=none",
+                         "fadvise=sequential", "cache=writethrough",
                          NULL) == -1)
     exit (EXIT_FAILURE);
   do_test ();
-- 
2.25.4




More information about the Libguestfs mailing list