[Libguestfs] [nbdkit PATCH 2/4] truncate: Factor out reading real_size under mutex

[Eric Blake]

Add a helper function get_real_size() to make it easier to add sanity
checking that mutex calls don't fail.

Signed-off-by: Eric Blake <eblake at redhat.com>

---

I'm a bit unsure why truncate.c needs a lock anyway. I guess it's
because we are storing 'size' and 'real_size' as globals rather than
per-connection values in the handle, and we're worrying about parallel
connections where we don't want one thread reading inconsistent values
while another thread is in the middle of truncate_get_size()? At any
rate, as long as we don't have dynamic resize in the NBD protocol,
size can't change within the confines of a single connection. And even
if we DO assume that the underlying plugin reports different sizes for
different connections, our use of a global does not play well (if
client A sees size 1k, then client B sees size 2k, that changes client
A's use of 'real_size' to be something different than client A was
expecting).

So, I think we have to move 'size' and 'real_size' into the
per-connection handle, at which point, can't we just set them once
during truncate_prepare by moving the existing guts of
truncate_get_size there, and then letting truncate_get_size just
return h->size while all other truncate_* functions just access
h->real_size without a lock?

I guess I should look to see what other global filter state is better
tracked per-filter rather than globally?
---
 filters/truncate/truncate.c | 41 ++++++++++++-------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/filters/truncate/truncate.c b/filters/truncate/truncate.c
index 5f3370d..38745da 100644
--- a/filters/truncate/truncate.c
+++ b/filters/truncate/truncate.c
@@ -63,6 +63,13 @@ static uint64_t size;
 /* This lock protects the real_size and size fields. */
 static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

+static uint64_t
+get_real_size (void)
+{
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
+  return real_size;
+}
+
 static int
 parse_round_param (const char *key, const char *value, unsigned *ret)
 {
@@ -147,7 +154,7 @@ truncate_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
   if (r == -1)
     return -1;

-  pthread_mutex_lock (&lock);
+  ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);

   real_size = size = r;

@@ -163,8 +170,6 @@ truncate_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
     size = ROUND_DOWN (size, round_down);
   ret = size;

-  pthread_mutex_unlock (&lock);
-
   return ret;
 }

@@ -176,11 +181,7 @@ truncate_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
 {
   int r;
   uint32_t n;
-  uint64_t real_size_copy;
-
-  pthread_mutex_lock (&lock);
-  real_size_copy = real_size;
-  pthread_mutex_unlock (&lock);
+  uint64_t real_size_copy = get_real_size ();

   if (offset < real_size_copy) {
     if (offset + count <= real_size_copy)
@@ -209,11 +210,7 @@ truncate_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,
 {
   int r;
   uint32_t n;
-  uint64_t real_size_copy;
-
-  pthread_mutex_lock (&lock);
-  real_size_copy = real_size;
-  pthread_mutex_unlock (&lock);
+  uint64_t real_size_copy = get_real_size ();

   if (offset < real_size_copy) {
     if (offset + count <= real_size_copy)
@@ -246,11 +243,7 @@ truncate_trim (struct nbdkit_next_ops *next_ops, void *nxdata,
                uint32_t flags, int *err)
 {
   uint32_t n;
-  uint64_t real_size_copy;
-
-  pthread_mutex_lock (&lock);
-  real_size_copy = real_size;
-  pthread_mutex_unlock (&lock);
+  uint64_t real_size_copy = get_real_size ();

   if (offset < real_size_copy) {
     if (offset + count <= real_size_copy)
@@ -269,11 +262,7 @@ truncate_zero (struct nbdkit_next_ops *next_ops, void *nxdata,
                uint32_t flags, int *err)
 {
   uint32_t n;
-  uint64_t real_size_copy;
-
-  pthread_mutex_lock (&lock);
-  real_size_copy = real_size;
-  pthread_mutex_unlock (&lock);
+  uint64_t real_size_copy = get_real_size ();

   if (offset < real_size_copy) {
     if (offset + count <= real_size_copy)
@@ -292,14 +281,10 @@ truncate_extents (struct nbdkit_next_ops *next_ops, void *nxdata,
                   uint32_t flags, struct nbdkit_extents *extents, int *err)
 {
   uint32_t n;
-  uint64_t real_size_copy;
+  uint64_t real_size_copy = get_real_size ();
   CLEANUP_EXTENTS_FREE struct nbdkit_extents *extents2 = NULL;
   size_t i;

-  pthread_mutex_lock (&lock);
-  real_size_copy = real_size;
-  pthread_mutex_unlock (&lock);
-
   /* If the entire request is beyond the end of the underlying plugin
    * then this is the easy case: return a hole up to the end of the
    * file.
-- 
2.20.1