[PATCH v2 1/2] util: Rework virFileFlock() to be unambiguous
Martin Kletzander
mkletzan at redhat.com
Fri Jul 10 20:05:36 UTC 2020
On Fri, Jul 10, 2020 at 09:45:25PM +0200, Martin Kletzander wrote:
>The boolean parameters for lock/unlock and read/write (shared/exclusive) caused
>a lot of confusion when reading the callers. The new approach is explicit and
>unambiguous.
>
>Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>Reviewed-by: Andrea Bolognani <abologna at redhat.com>
>---
> src/util/virfile.c | 27 ++++++++++++++++++---------
> src/util/virfile.h | 9 ++++++++-
> src/util/virresctrl.c | 4 ++--
> 3 files changed, 28 insertions(+), 12 deletions(-)
>
>diff --git a/src/util/virfile.c b/src/util/virfile.c
>index 213acdbcaa2b..554011eecb25 100644
>--- a/src/util/virfile.c
>+++ b/src/util/virfile.c
>@@ -465,23 +465,33 @@ int virFileUnlock(int fd, off_t start, off_t len)
> /**
> * virFileFlock:
> * @fd: file descriptor to call flock on
>- * @lock: true for lock, false for unlock
>- * @shared: true if shared, false for exclusive, ignored if `@lock == false`
>+ * @op: operation to perform
> *
> * This is just a simple wrapper around flock(2) that errors out on unsupported
> * platforms.
> *
> * The lock will be released when @fd is closed or this function is called with
>- * `@lock == false`.
>+ * `@op == VIR_FILE_FLOCK_UNLOCK`.
> *
> * Returns 0 on success, -1 otherwise (with errno set)
> */
>-int virFileFlock(int fd, bool lock, bool shared)
>+int virFileFlock(int fd, virFileFlockOperation op)
> {
>- if (lock)
>- return flock(fd, shared ? LOCK_SH : LOCK_EX);
>+ int flock_op = -1;
>
>- return flock(fd, LOCK_UN);
>+ switch (op) {
>+ case VIR_FILE_FLOCK_SHARED:
>+ flock_op = LOCK_SH;
>+ break;
>+ case VIR_FILE_FLOCK_EXCLUSIVE:
>+ flock_op = LOCK_EX;
>+ break;
>+ case VIR_FILE_FLOCK_UNLOCK:
>+ flock_op = LOCK_UN;
>+ break;
Yes, I know I forgot to add the default branch and it complicates matters even
more because it would set an error in a function that just sets errno... I'll
send a v3.
>+ }
>+
>+ return flock(fd, flock_op);
> }
>
> #else /* WIN32 */
>@@ -505,8 +515,7 @@ int virFileUnlock(int fd G_GNUC_UNUSED,
>
>
> int virFileFlock(int fd G_GNUC_UNUSED,
>- bool lock G_GNUC_UNUSED,
>- bool shared G_GNUC_UNUSED)
>+ virFileFlockOperation op G_GNUC_UNUSED)
> {
> errno = ENOSYS;
> return -1;
>diff --git a/src/util/virfile.h b/src/util/virfile.h
>index 7a92364a5c9f..04428727fd3b 100644
>--- a/src/util/virfile.h
>+++ b/src/util/virfile.h
>@@ -118,7 +118,14 @@ int virFileLock(int fd, bool shared, off_t start, off_t len, bool waitForLock)
> int virFileUnlock(int fd, off_t start, off_t len)
> G_GNUC_NO_INLINE;
>
>-int virFileFlock(int fd, bool lock, bool shared);
>+
>+typedef enum {
>+ VIR_FILE_FLOCK_EXCLUSIVE,
>+ VIR_FILE_FLOCK_SHARED,
>+ VIR_FILE_FLOCK_UNLOCK,
>+} virFileFlockOperation;
>+
>+int virFileFlock(int fd, virFileFlockOperation op);
>
> typedef int (*virFileRewriteFunc)(int fd, const void *opaque);
> int virFileRewrite(const char *path,
>diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
>index 3563fc560db5..b685b8bb6f7b 100644
>--- a/src/util/virresctrl.c
>+++ b/src/util/virresctrl.c
>@@ -463,7 +463,7 @@ virResctrlLockWrite(void)
> return -1;
> }
>
>- if (virFileFlock(fd, true, true) < 0) {
>+ if (virFileFlock(fd, VIR_FILE_FLOCK_SHARED) < 0) {
> virReportSystemError(errno, "%s", _("Cannot lock resctrl"));
> VIR_FORCE_CLOSE(fd);
> return -1;
>@@ -485,7 +485,7 @@ virResctrlUnlock(int fd)
> virReportSystemError(errno, "%s", _("Cannot close resctrl"));
>
> /* Trying to save the already broken */
>- if (virFileFlock(fd, false, false) < 0)
>+ if (virFileFlock(fd, VIR_FILE_FLOCK_UNLOCK) < 0)
> virReportSystemError(errno, "%s", _("Cannot unlock resctrl"));
>
> return -1;
>--
>2.27.0
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200710/4ac60386/attachment-0001.sig>
More information about the libvir-list
mailing list