Re: [Qemu-devel] [PATCH v6 03/22] blockdev: Add and parse "lock-mode" option for image locking

[Kevin Wolf <kwolf@redhat.com>]

Am 03.06.2016 um 10:48 hat Fam Zheng geschrieben:
> Respect the locking mode from CLI or QMP, and set the open flags
> accordingly.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  blockdev.c      | 23 +++++++++++++++++++++++
>  qemu-options.hx |  1 +
>  2 files changed, 24 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 717785e..5acb286 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -356,6 +356,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>      const char *discard;
>      Error *local_error = NULL;
>      const char *aio;
> +    const char *lock_mode;
>  
>      if (bdrv_flags) {
>          if (!qemu_opt_get_bool(opts, "read-only", false)) {
> @@ -382,6 +383,18 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>                 return;
>              }
>          }
> +
> +        lock_mode = qemu_opt_get(opts, "lock-mode") ? : "off";
> +        if (!strcmp(lock_mode, "exclusive")) {
> +            /* Default */
> +        } else if (!strcmp(lock_mode, "shared")) {
> +            *bdrv_flags |= BDRV_O_SHARED_LOCK;
> +        } else if (!strcmp(lock_mode, "off")) {
> +            *bdrv_flags |= BDRV_O_NO_LOCK;
> +        } else {
> +           error_setg(errp, "invalid lock mode");
> +           return;
> +        }
>      }
>  
>      /* disk I/O throttling */
> @@ -4296,6 +4309,11 @@ QemuOptsList qemu_common_drive_opts = {
>              .type = QEMU_OPT_BOOL,
>              .help = "whether to account for failed I/O operations "
>                      "in the statistics",
> +        },{
> +            .name = "lock-mode",
> +            .type = QEMU_OPT_STRING,
> +            .help = "how to lock the image (exclusive, shared, off. "
> +                    "default: exclusive)",
>          },
>          { /* end of list */ }
>      },
> @@ -4325,6 +4343,11 @@ static QemuOptsList qemu_root_bds_opts = {
>              .name = "detect-zeroes",
>              .type = QEMU_OPT_STRING,
>              .help = "try to optimize zero writes (off, on, unmap)",
> +        },{
> +            .name = "lock-mode",
> +            .type = QEMU_OPT_STRING,
> +            .help = "how to lock the image (exclusive, shared, off. "
> +                    "default: exclusive)",
>          },
>          { /* end of list */ }
>      },

This is the wrong level to implement the feature. You would only be able
to configure the locking on the top level image this way (because what
we're doing here is BB, not BDS configuration). If you want to allow
configuration per node, you need to put the options into
bdrv_runtime_opts and interpret them in bdrv_open_common().

Otherwise we would have to specify in the blockdev-add documentation
that this works only on the top level, but I don't see a reason for
such a restriction.

Kevin