[for-2.7,05/15] raw-posix: Implement .bdrv_lockf

Commit Message

Fam Zheng April 13, 2016, 9:09 a.m. UTC

Because virtlockd in libvirt already uses the fcntl lock on the image file, we
have to workaround this by locking a digest-mapped temporary file.

Signed-off-by: Fam Zheng <[e-mail]>
---
 block/raw-posix.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

Comments

Daniel P. Berrangé April 13, 2016, 9:21 a.m. UTC | #1

On Wed, Apr 13, 2016 at 05:09:54PM +0800, Fam Zheng wrote:
> Because virtlockd in libvirt already uses the fcntl lock on the image file, we
> have to workaround this by locking a digest-mapped temporary file.
> 
> Signed-off-by: Fam Zheng <[e-mail]>
> ---
>  block/raw-posix.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 906d5c9..277f20d 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -35,6 +35,7 @@
>  #include "raw-aio.h"
>  #include "qapi/util.h"
>  #include "qapi/qmp/qstring.h"
> +#include "glib.h"
>  
>  #if defined(__APPLE__) && (__MACH__)
>  #include <paths.h>
> @@ -149,6 +150,9 @@ typedef struct BDRVRawState {
>      bool discard_zeroes:1;
>      bool has_fallocate;
>      bool needs_alignment;
> +    bool image_locked;
> +    int lock_file_fd;
> +    char *lock_file_name;
>  } BDRVRawState;
>  
>  typedef struct BDRVRawReopenState {
> @@ -397,6 +401,87 @@ static void raw_attach_aio_context(BlockDriverState *bs,
>  #endif
>  }
>  
> +static int raw_do_lockf(int fd, BdrvLockfCmd cmd)
> +{
> +    int ret;
> +    struct flock fl = (struct flock) {
> +        .l_start = 0,
> +        .l_whence  = SEEK_SET,
> +        .l_len = 0,
> +    };

If you change this to

   .l_start = 1,
   .l_len = 1,

then you would be telling a selective lock at byte 1 which would
not interfere with anything virtlockd currently does, and also
leave the other bytes unlocked for future use by QEMU or libvirt
as needed.


Regards,
Daniel

Fam Zheng April 14, 2016, 2:24 a.m. UTC | #2

On Wed, 04/13 10:21, Daniel P. Berrange wrote:
> On Wed, Apr 13, 2016 at 05:09:54PM +0800, Fam Zheng wrote:
> > Because virtlockd in libvirt already uses the fcntl lock on the image file, we
> > have to workaround this by locking a digest-mapped temporary file.
> > 
> > Signed-off-by: Fam Zheng <[e-mail]>
> > ---
> >  block/raw-posix.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 97 insertions(+)
> > 
> > diff --git a/block/raw-posix.c b/block/raw-posix.c
> > index 906d5c9..277f20d 100644
> > --- a/block/raw-posix.c
> > +++ b/block/raw-posix.c
> > @@ -35,6 +35,7 @@
> >  #include "raw-aio.h"
> >  #include "qapi/util.h"
> >  #include "qapi/qmp/qstring.h"
> > +#include "glib.h"
> >  
> >  #if defined(__APPLE__) && (__MACH__)
> >  #include <paths.h>
> > @@ -149,6 +150,9 @@ typedef struct BDRVRawState {
> >      bool discard_zeroes:1;
> >      bool has_fallocate;
> >      bool needs_alignment;
> > +    bool image_locked;
> > +    int lock_file_fd;
> > +    char *lock_file_name;
> >  } BDRVRawState;
> >  
> >  typedef struct BDRVRawReopenState {
> > @@ -397,6 +401,87 @@ static void raw_attach_aio_context(BlockDriverState *bs,
> >  #endif
> >  }
> >  
> > +static int raw_do_lockf(int fd, BdrvLockfCmd cmd)
> > +{
> > +    int ret;
> > +    struct flock fl = (struct flock) {
> > +        .l_start = 0,
> > +        .l_whence  = SEEK_SET,
> > +        .l_len = 0,
> > +    };
> 
> If you change this to
> 
>    .l_start = 1,
>    .l_len = 1,
> 
> then you would be telling a selective lock at byte 1 which would
> not interfere with anything virtlockd currently does, and also
> leave the other bytes unlocked for future use by QEMU or libvirt
> as needed.

That's brillent, thanks a lot!

Fam

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 906d5c9..277f20d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -35,6 +35,7 @@ 
 #include "raw-aio.h"
 #include "qapi/util.h"
 #include "qapi/qmp/qstring.h"
+#include "glib.h"
 
 #if defined(__APPLE__) && (__MACH__)
 #include <paths.h>
@@ -149,6 +150,9 @@  typedef struct BDRVRawState {
     bool discard_zeroes:1;
     bool has_fallocate;
     bool needs_alignment;
+    bool image_locked;
+    int lock_file_fd;
+    char *lock_file_name;
 } BDRVRawState;
 
 typedef struct BDRVRawReopenState {
@@ -397,6 +401,87 @@  static void raw_attach_aio_context(BlockDriverState *bs,
 #endif
 }
 
+static int raw_do_lockf(int fd, BdrvLockfCmd cmd)
+{
+    int ret;
+    struct flock fl = (struct flock) {
+        .l_start = 0,
+        .l_whence  = SEEK_SET,
+        .l_len = 0,
+    };
+    switch (cmd) {
+    case BDRV_LOCKF_RWLOCK:
+        fl.l_type = F_WRLCK;
+        break;
+    case BDRV_LOCKF_ROLOCK:
+        fl.l_type = F_RDLCK;
+        break;
+    case BDRV_LOCKF_UNLOCK:
+        fl.l_type = F_UNLCK;
+        break;
+    default:
+        abort();
+    }
+    ret = fcntl(fd, F_SETLK, &fl);
+    return ret == -1 ? -errno : 0;
+}
+
+static char *raw_get_lock_file_name(BlockDriverState *bs)
+{
+    GChecksum *cksm;
+    const char *digest;
+    char *lock_file = NULL;
+    char *fullpath = g_malloc(PATH_MAX);
+
+    if (!realpath(bs->filename, fullpath)) {
+        pstrcpy(fullpath, PATH_MAX, bs->filename);
+    }
+    cksm = g_checksum_new(G_CHECKSUM_SHA1);
+    g_checksum_update(cksm, (const guchar *)fullpath, strlen(fullpath));
+    g_free(fullpath);
+    digest = g_checksum_get_string(cksm);
+    if (digest) {
+        lock_file = g_strdup_printf("/var/tmp/.qemu-%s.lock", digest);
+    }
+    g_checksum_free(cksm);
+    return lock_file;
+}
+
+static int raw_lockf(BlockDriverState *bs, BdrvLockfCmd cmd, Error **errp)
+{
+
+    BDRVRawState *s = bs->opaque;
+    int ret;
+
+    if ((cmd == BDRV_LOCKF_UNLOCK) == (!s->image_locked)) {
+        return 0;
+    }
+    if (s->lock_file_fd == -1) {
+        /* Create and open the lock file if necessary. */
+
+        assert(cmd != BDRV_LOCKF_UNLOCK);
+
+        s->lock_file_name = raw_get_lock_file_name(bs);
+
+        s->lock_file_fd = qemu_open(s->lock_file_name, O_RDWR | O_CREAT, 0644);
+        if (s->lock_file_fd == -1) {
+            error_setg_errno(errp, errno, "Failed to open file");
+            ret = -EIO;
+            goto out;
+        }
+    }
+
+    ret = raw_do_lockf(s->lock_file_fd, cmd);
+    if (ret) {
+        error_setg_errno(errp, errno,
+                         "Failed to lock file: %s", s->lock_file_name);
+    } else {
+        s->image_locked = cmd != BDRV_LOCKF_UNLOCK;
+    }
+out:
+    return ret;
+}
+
 #ifdef CONFIG_LINUX_AIO
 static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags)
 {
@@ -585,6 +670,7 @@  static int raw_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
 
     s->type = FTYPE_FILE;
+    s->lock_file_fd = -1;
     ret = raw_open_common(bs, options, flags, 0, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -1393,6 +1479,15 @@  static void raw_close(BlockDriverState *bs)
 
     raw_detach_aio_context(bs);
 
+    if (s->lock_file_fd != -1) {
+        qemu_close(s->lock_file_fd);
+        s->lock_file_fd = -1;
+    }
+    if (s->lock_file_name) {
+        unlink(s->lock_file_name);
+        g_free(s->lock_file_name);
+        s->lock_file_name = NULL;
+   }
 #ifdef CONFIG_LINUX_AIO
     if (s->use_aio) {
         laio_cleanup(s->aio_ctx);
@@ -1960,6 +2055,8 @@  BlockDriver bdrv_file = {
     .bdrv_detach_aio_context = raw_detach_aio_context,
     .bdrv_attach_aio_context = raw_attach_aio_context,
 
+    .bdrv_lockf = raw_lockf,
+
     .create_opts = &raw_create_opts,
 };