[Libguestfs] [nbdkit PATCH 4/4] filters: Check for mutex failures

Commit 975dab14 argued that for simple lock/unlock sequences, it was easier to avoid the cleanup.h macros. But since that time, we added additional sanity checking to the macros, at which point the boilerplate of inlining that sanity checking is outweighed compared to just using the macros in more places. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/cache/cache.c | 23 +++++++++++------------ filters/cow/cow.c | 19 ++++++++----------- filters/error/error.c | 7 ++++--- filters/log/log.c | 3 +-- filters/rate/rate.c | 10 +++++----- filters/readahead/readahead.c | 3 +-- filters/stats/stats.c | 18 ++++++------------ filters/error/Makefile.am | 5 ++++- 8 files changed, 40 insertions(+), 48 deletions(-) diff --git a/filters/cache/cache.c b/filters/cache/cache.c index 6a9966e..b3fef42 100644 --- a/filters/cache/cache.c +++ b/filters/cache/cache.c @@ -209,9 +209,8 @@ cache_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, nbdkit_debug ("cache: underlying file size: %" PRIi64, size); - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_set_size (size); - pthread_mutex_unlock (&lock); if (r == -1) return -1; @@ -266,9 +265,10 @@ cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - pthread_mutex_lock (&lock); - r = blk_read (next_ops, nxdata, blknum, block, err); - pthread_mutex_unlock (&lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + r = blk_read (next_ops, nxdata, blknum, block, err); + } if (r == -1) return -1; @@ -316,13 +316,12 @@ cache_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, /* Do a read-modify-write operation on the current block. * Hold the lock over the whole operation. */ - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_read (next_ops, nxdata, blknum, block, err); if (r != -1) { memcpy (&block[blkoffs], buf, n); r = blk_write (next_ops, nxdata, blknum, block, flags, err); } - pthread_mutex_unlock (&lock); if (r == -1) return -1; @@ -371,13 +370,12 @@ cache_zero (struct nbdkit_next_ops *next_ops, void *nxdata, /* Do a read-modify-write operation on the current block. * Hold the lock over the whole operation. */ - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_read (next_ops, nxdata, blknum, block, err); if (r != -1) { memset (&block[blkoffs], 0, n); r = blk_write (next_ops, nxdata, blknum, block, flags, err); } - pthread_mutex_unlock (&lock); if (r == -1) return -1; @@ -429,9 +427,10 @@ cache_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, * to be sure. Also we still need to issue the flush to the * underlying storage. */ - pthread_mutex_lock (&lock); - for_each_dirty_block (flush_dirty_block, &data); - pthread_mutex_unlock (&lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + for_each_dirty_block (flush_dirty_block, &data); + } /* Now issue a flush request to the underlying storage. */ if (next_ops->flush (nxdata, 0, diff --git a/filters/cow/cow.c b/filters/cow/cow.c index 04ec44f..35ad718 100644 --- a/filters/cow/cow.c +++ b/filters/cow/cow.c @@ -92,9 +92,8 @@ cow_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, nbdkit_debug ("cow: underlying file size: %" PRIi64, size); - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_set_size (size); - pthread_mutex_unlock (&lock); if (r == -1) return -1; @@ -174,9 +173,10 @@ cow_pread (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - pthread_mutex_lock (&lock); - r = blk_read (next_ops, nxdata, blknum, block, err); - pthread_mutex_unlock (&lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + r = blk_read (next_ops, nxdata, blknum, block, err); + } if (r == -1) return -1; @@ -218,13 +218,12 @@ cow_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, /* Do a read-modify-write operation on the current block. * Hold the lock over the whole operation. */ - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_read (next_ops, nxdata, blknum, block, err); if (r != -1) { memcpy (&block[blkoffs], buf, n); r = blk_write (blknum, block, err); } - pthread_mutex_unlock (&lock); if (r == -1) return -1; @@ -269,13 +268,12 @@ cow_zero (struct nbdkit_next_ops *next_ops, void *nxdata, * XXX There is the possibility of optimizing this: ONLY if we are * writing a whole, aligned block, then use FALLOC_FL_ZERO_RANGE. */ - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_read (next_ops, nxdata, blknum, block, err); if (r != -1) { memset (&block[blkoffs], 0, n); r = blk_write (blknum, block, err); } - pthread_mutex_unlock (&lock); if (r == -1) return -1; @@ -294,11 +292,10 @@ cow_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, { int r; - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); r = blk_flush (); if (r == -1) *err = errno; - pthread_mutex_unlock (&lock); return r; } diff --git a/filters/error/error.c b/filters/error/error.c index 0f84f12..22ebd1c 100644 --- a/filters/error/error.c +++ b/filters/error/error.c @@ -283,9 +283,10 @@ random_error (const struct error_settings *error_settings, * representable in a 64 bit integer, and because we don't need all * this precision anyway, let's work in 32 bits. */ - pthread_mutex_lock (&lock); - rand = xrandom (&random_state) & UINT32_MAX; - pthread_mutex_unlock (&lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + rand = xrandom (&random_state) & UINT32_MAX; + } if (rand >= error_settings->rate * UINT32_MAX) return false; diff --git a/filters/log/log.c b/filters/log/log.c index 63afcf3..76af98b 100644 --- a/filters/log/log.c +++ b/filters/log/log.c @@ -217,9 +217,8 @@ log_open (nbdkit_next_open *next, void *nxdata, int readonly) return NULL; } - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); h->connection = ++connections; - pthread_mutex_unlock (&lock); h->id = 0; return h; } diff --git a/filters/rate/rate.c b/filters/rate/rate.c index 978cdc3..cf03541 100644 --- a/filters/rate/rate.c +++ b/filters/rate/rate.c @@ -222,9 +222,8 @@ maybe_adjust (const char *file, struct bucket *bucket, pthread_mutex_t *lock) if (new_rate == -1) return; - pthread_mutex_lock (lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (lock); old_rate = bucket_adjust_rate (bucket, new_rate); - pthread_mutex_unlock (lock); if (old_rate != new_rate) nbdkit_debug ("rate adjusted from %" PRIu64 " to %" PRIi64, @@ -245,9 +244,10 @@ maybe_sleep (struct bucket *bucket, pthread_mutex_t *lock, uint32_t count) while (bits > 0) { /* Run the token bucket algorithm. */ - pthread_mutex_lock (lock); - bits = bucket_run (bucket, bits, &ts); - pthread_mutex_unlock (lock); + { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (lock); + bits = bucket_run (bucket, bits, &ts); + } if (bits > 0) nanosleep (&ts, NULL); diff --git a/filters/readahead/readahead.c b/filters/readahead/readahead.c index f46b6b0..dc27bae 100644 --- a/filters/readahead/readahead.c +++ b/filters/readahead/readahead.c @@ -103,9 +103,8 @@ readahead_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); size = r; - pthread_mutex_unlock (&lock); return r; } diff --git a/filters/stats/stats.c b/filters/stats/stats.c index 96de154..7cbf129 100644 --- a/filters/stats/stats.c +++ b/filters/stats/stats.c @@ -100,9 +100,8 @@ stats_unload (void) gettimeofday (&now, NULL); usecs = tvdiff_usec (&start_t, &now); if (fp && usecs > 0) { - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); print_stats (usecs); - pthread_mutex_unlock (&lock); } if (fp) @@ -163,10 +162,9 @@ stats_pread (struct nbdkit_next_ops *next_ops, void *nxdata, r = next_ops->pread (nxdata, buf, count, offset, flags, err); if (r == 0) { - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); pread_ops++; pread_bytes += count; - pthread_mutex_unlock (&lock); } return r; } @@ -182,10 +180,9 @@ stats_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, r = next_ops->pwrite (nxdata, buf, count, offset, flags, err); if (r == 0) { - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); pwrite_ops++; pwrite_bytes += count; - pthread_mutex_unlock (&lock); } return r; } @@ -201,10 +198,9 @@ stats_trim (struct nbdkit_next_ops *next_ops, void *nxdata, r = next_ops->trim (nxdata, count, offset, flags, err); if (r == 0) { - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); trim_ops++; trim_bytes += count; - pthread_mutex_unlock (&lock); } return r; } @@ -220,10 +216,9 @@ stats_zero (struct nbdkit_next_ops *next_ops, void *nxdata, r = next_ops->zero (nxdata, count, offset, flags, err); if (r == 0) { - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); zero_ops++; zero_bytes += count; - pthread_mutex_unlock (&lock); } return r; } @@ -239,14 +234,13 @@ stats_extents (struct nbdkit_next_ops *next_ops, void *nxdata, r = next_ops->extents (nxdata, count, offset, flags, extents, err); if (r == 0) { - pthread_mutex_lock (&lock); + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); extents_ops++; /* XXX There's a case for trying to determine how long the extents * will be that are returned to the client, given the flags and * the complex rules in the protocol. */ extents_bytes += count; - pthread_mutex_unlock (&lock); } return r; } diff --git a/filters/error/Makefile.am b/filters/error/Makefile.am index 9f116fe..1fb85ec 100644 --- a/filters/error/Makefile.am +++ b/filters/error/Makefile.am @@ -41,12 +41,15 @@ nbdkit_error_filter_la_SOURCES = \ nbdkit_error_filter_la_CPPFLAGS = \ -I$(top_srcdir)/include \ - -I$(top_srcdir)/common/include + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/utils nbdkit_error_filter_la_CFLAGS = \ $(WARNINGS_CFLAGS) nbdkit_error_filter_la_LDFLAGS = \ -module -avoid-version -shared \ -Wl,--version-script=$(top_srcdir)/filters/filters.syms +nbdkit_error_filter_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la if HAVE_POD -- 2.20.1