Re: [Qemu-devel] [PATCH v5 11/27] raw-posix: Implement .bdrv_lockf

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]

On 17.05.2016 09:35, Fam Zheng wrote:
> virtlockd in libvirt locks the first byte, we lock byte 1 to avoid
> the intervene.
> 
> Both file and host device protocols are covered.

Is there a reason you didn't cover host_cdrom other than it generally
being read-only and thus probably not really needing a lock?

> Suggested-by: "Daniel P. Berrange" <berrange@redhat.com>

I think the quotation marks are superfluous.

> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/raw-posix.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index bb8669f..acd3be2 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"

What for?

>  
>  #if defined(__APPLE__) && (__MACH__)
>  #include <paths.h>
> @@ -397,6 +398,23 @@ static void raw_attach_aio_context(BlockDriverState *bs,
>  #endif
>  }
>  
> +static int raw_lockf(BlockDriverState *bs, BdrvLockfCmd cmd)
> +{
> +
> +    BDRVRawState *s = bs->opaque;
> +
> +    switch (cmd) {
> +    case BDRV_LOCKF_EXCLUSIVE:
> +        return qemu_lock_fd(s->fd, 1, 1, true);
> +    case BDRV_LOCKF_SHARED:
> +        return qemu_lock_fd(s->fd, 1, 1, false);
> +    case BDRV_LOCKF_UNLOCK:
> +        return qemu_unlock_fd(s->fd, 1, 1);
> +    default:
> +        abort();
> +    }

I figure the comment from patch 8 about why byte 1 is locked should be
here somewhere.

Max

> +}
> +
>  #ifdef CONFIG_LINUX_AIO
>  static int raw_set_aio(LinuxAioState **aio_ctx, int *use_aio, int bdrv_flags)
>  {
> @@ -1942,6 +1960,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,
>  };
>  
> @@ -2396,6 +2416,8 @@ static BlockDriver bdrv_host_device = {
>  #ifdef __linux__
>      .bdrv_aio_ioctl     = hdev_aio_ioctl,
>  #endif
> +
> +    .bdrv_lockf = raw_lockf,
>  };
>  
>  #if defined(__linux__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]