Re: [PATCH] file-win32: Fix "locking" option

[Philippe Mathieu-Daudé]

On 9/7/20 11:27 AM, Kevin Wolf wrote:
> The intended behaviour was that locking=off/auto work and have no
> effect (to remain compatible with file-posix), whereas locking=on would
> return an error. Unfortunately, the code forgot to remove "locking" from
> the options QDict, so any attempt to use the option would fail.
> 
> Replace the option parsing code for "locking" with something that is
> part of the raw_runtime_opts QemuOptsList (so it is properly removed
> from the QDict) and looks more like file-posix.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  block/file-win32.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/block/file-win32.c b/block/file-win32.c
> index ab69bd811a..e2900c3a51 100644
> --- a/block/file-win32.c
> +++ b/block/file-win32.c
> @@ -299,6 +299,11 @@ static QemuOptsList raw_runtime_opts = {
>              .type = QEMU_OPT_STRING,
>              .help = "host AIO implementation (threads, native)",
>          },
> +        {
> +            .name = "locking",
> +            .type = QEMU_OPT_STRING,
> +            .help = "file locking mode (on/off/auto, default: auto)",
> +        },
>          { /* end of list */ }
>      },
>  };
> @@ -333,6 +338,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
> int flags,
>      Error *local_err = NULL;
>      const char *filename;
>      bool use_aio;
> +    OnOffAuto locking;
>      int ret;
>  
>      s->type = FTYPE_FILE;
> @@ -343,10 +349,24 @@ static int raw_open(BlockDriverState *bs, QDict 
> *options, int flags,
>          goto fail;
>      }
>  
> -    if (qdict_get_try_bool(options, "locking", false)) {
> +    locking = qapi_enum_parse(&OnOffAuto_lookup,
> +                              qemu_opt_get(opts, "locking"),
> +                              ON_OFF_AUTO_AUTO, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +    switch (locking) {
> +    case ON_OFF_AUTO_ON:
>          error_setg(errp, "locking=on is not supported on Windows");
>          ret = -EINVAL;
>          goto fail;
> +    case ON_OFF_AUTO_OFF:
> +    case ON_OFF_AUTO_AUTO:
> +        break;
> +    default:
> +        g_assert_not_reached();
>      }
>  
>      filename = qemu_opt_get(opts, "filename");
>