All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Feng <fengli@smartx.com>
To: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>,
	qemu-block@nongnu.org (open list:raw),
	qemu-devel@nongnu.org (open list:All patches CC here)
Cc: lifeng1519@gmail.com, Li Feng <fengli@smartx.com>, kyle@smartx.com
Subject: [PATCH] file-posix: detect the lock using the real file
Date: Tue,  8 Dec 2020 20:59:37 +0800	[thread overview]
Message-ID: <1607432377-87084-1-git-send-email-fengli@smartx.com> (raw)

This patch addresses this issue:
When accessing a volume on an NFS filesystem without supporting the file lock,
tools, like qemu-img, will complain "Failed to lock byte 100".

In the original code, the qemu_has_ofd_lock will test the lock on the
"/dev/null" pseudo-file. Actually, the file.locking is per-drive property,
which depends on the underlay filesystem.

In this patch, make the 'qemu_has_ofd_lock' with a filename be more generic
and reasonable.

Signed-off-by: Li Feng <fengli@smartx.com>
---
 block/file-posix.c         | 32 +++++++++++++++-------------
 include/qemu/osdep.h       |  2 +-
 tests/test-image-locking.c |  2 +-
 util/osdep.c               | 43 ++++++++++++++++++++++++--------------
 4 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 806764f7e3..03be1b188c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -595,7 +595,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     switch (locking) {
     case ON_OFF_AUTO_ON:
         s->use_lock = true;
-        if (!qemu_has_ofd_lock()) {
+        if (!qemu_has_ofd_lock(filename)) {
             warn_report("File lock requested but OFD locking syscall is "
                         "unavailable, falling back to POSIX file locks");
             error_printf("Due to the implementation, locks can be lost "
@@ -606,7 +606,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
         s->use_lock = false;
         break;
     case ON_OFF_AUTO_AUTO:
-        s->use_lock = qemu_has_ofd_lock();
+        s->use_lock = qemu_has_ofd_lock(filename);
         break;
     default:
         abort();
@@ -2388,6 +2388,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
     int fd;
     uint64_t perm, shared;
     int result = 0;
+    bool use_lock;
 
     /* Validate options and set default values */
     assert(options->driver == BLOCKDEV_DRIVER_FILE);
@@ -2428,19 +2429,22 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
     perm = BLK_PERM_WRITE | BLK_PERM_RESIZE;
     shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
 
-    /* Step one: Take locks */
-    result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);
-    if (result < 0) {
-        goto out_close;
-    }
+    use_lock = qemu_has_ofd_lock(file_opts->filename);
+    if (use_lock) {
+        /* Step one: Take locks */
+        result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);
+        if (result < 0) {
+            goto out_close;
+        }
 
-    /* Step two: Check that nobody else has taken conflicting locks */
-    result = raw_check_lock_bytes(fd, perm, shared, errp);
-    if (result < 0) {
-        error_append_hint(errp,
-                          "Is another process using the image [%s]?\n",
-                          file_opts->filename);
-        goto out_unlock;
+        /* Step two: Check that nobody else has taken conflicting locks */
+        result = raw_check_lock_bytes(fd, perm, shared, errp);
+        if (result < 0) {
+            error_append_hint(errp,
+                              "Is another process using the image [%s]?\n",
+                              file_opts->filename);
+            goto out_unlock;
+        }
     }
 
     /* Clear the file by truncating it to 0 */
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index f9ec8c84e9..349adad465 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -512,7 +512,7 @@ int qemu_dup(int fd);
 int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
 int qemu_unlock_fd(int fd, int64_t start, int64_t len);
 int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
-bool qemu_has_ofd_lock(void);
+bool qemu_has_ofd_lock(const char *filename);
 #endif
 
 #if defined(__HAIKU__) && defined(__i386__)
diff --git a/tests/test-image-locking.c b/tests/test-image-locking.c
index ba057bd66c..3e80246081 100644
--- a/tests/test-image-locking.c
+++ b/tests/test-image-locking.c
@@ -149,7 +149,7 @@ int main(int argc, char **argv)
 
     g_test_init(&argc, &argv, NULL);
 
-    if (qemu_has_ofd_lock()) {
+    if (qemu_has_ofd_lock(NULL)) {
         g_test_add_func("/image-locking/basic", test_image_locking_basic);
         g_test_add_func("/image-locking/set-perm-abort", test_set_perm_abort);
     }
diff --git a/util/osdep.c b/util/osdep.c
index 66d01b9160..e7e502edd1 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -42,6 +42,7 @@ extern int madvise(char *, size_t, int);
 static bool fips_enabled = false;
 
 static const char *hw_version = QEMU_HW_VERSION;
+static const char *null_device = "/dev/null";
 
 int socket_set_cork(int fd, int v)
 {
@@ -187,11 +188,10 @@ static int qemu_parse_fdset(const char *param)
     return qemu_parse_fd(param);
 }
 
-static void qemu_probe_lock_ops(void)
+static void qemu_probe_lock_ops_fd(int fd)
 {
     if (fcntl_op_setlk == -1) {
 #ifdef F_OFD_SETLK
-        int fd;
         int ret;
         struct flock fl = {
             .l_whence = SEEK_SET,
@@ -200,17 +200,7 @@ static void qemu_probe_lock_ops(void)
             .l_type   = F_WRLCK,
         };
 
-        fd = open("/dev/null", O_RDWR);
-        if (fd < 0) {
-            fprintf(stderr,
-                    "Failed to open /dev/null for OFD lock probing: %s\n",
-                    strerror(errno));
-            fcntl_op_setlk = F_SETLK;
-            fcntl_op_getlk = F_GETLK;
-            return;
-        }
         ret = fcntl(fd, F_OFD_GETLK, &fl);
-        close(fd);
         if (!ret) {
             fcntl_op_setlk = F_OFD_SETLK;
             fcntl_op_getlk = F_OFD_GETLK;
@@ -225,9 +215,30 @@ static void qemu_probe_lock_ops(void)
     }
 }
 
-bool qemu_has_ofd_lock(void)
+static void qemu_probe_lock_ops(const char *filename)
+{
+    int fd;
+    if (filename) {
+        fd = open(filename, O_RDWR);
+    } else {
+        fd = open(null_device, O_RDONLY);
+    }
+    if (fd < 0) {
+        fprintf(stderr,
+                "Failed to open %s for OFD lock probing: %s\n",
+                filename ? filename : null_device,
+                strerror(errno));
+        fcntl_op_setlk = F_SETLK;
+        fcntl_op_getlk = F_GETLK;
+        return;
+    }
+    qemu_probe_lock_ops_fd(fd);
+    close(fd);
+}
+
+bool qemu_has_ofd_lock(const char *filename)
 {
-    qemu_probe_lock_ops();
+    qemu_probe_lock_ops(filename);
 #ifdef F_OFD_SETLK
     return fcntl_op_setlk == F_OFD_SETLK;
 #else
@@ -244,7 +255,7 @@ static int qemu_lock_fcntl(int fd, int64_t start, int64_t len, int fl_type)
         .l_len    = len,
         .l_type   = fl_type,
     };
-    qemu_probe_lock_ops();
+    qemu_probe_lock_ops_fd(fd);
     do {
         ret = fcntl(fd, fcntl_op_setlk, &fl);
     } while (ret == -1 && errno == EINTR);
@@ -270,7 +281,7 @@ int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive)
         .l_len    = len,
         .l_type   = exclusive ? F_WRLCK : F_RDLCK,
     };
-    qemu_probe_lock_ops();
+    qemu_probe_lock_ops_fd(fd);
     ret = fcntl(fd, fcntl_op_getlk, &fl);
     if (ret == -1) {
         return -errno;
-- 
2.24.3



             reply	other threads:[~2020-12-08 13:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-08 12:59 Li Feng [this message]
2020-12-08 13:45 ` [PATCH] file-posix: detect the lock using the real file Daniel P. Berrangé
2020-12-09  3:49   ` Li Feng
2020-12-08 14:38 ` Kevin Wolf
2020-12-09  4:09   ` Li Feng
2020-12-09  9:33   ` Daniel P. Berrangé
2020-12-09 17:43     ` Kevin Wolf
2020-12-10 14:56       ` Li Feng
2020-12-10 15:29         ` Daniel P. Berrangé
2020-12-10 15:53           ` Feng Li
2020-12-10 15:56             ` Daniel P. Berrangé
2020-12-09  4:54 ` [PATCH v2] " Li Feng
2020-12-10 15:59   ` Daniel P. Berrangé
2020-12-10 16:12     ` Feng Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1607432377-87084-1-git-send-email-fengli@smartx.com \
    --to=fengli@smartx.com \
    --cc=kwolf@redhat.com \
    --cc=kyle@smartx.com \
    --cc=lifeng1519@gmail.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.