Re: [Qemu-devel] [PULL 08/23] file-posix: Forbid trying to change unsupported options during reopen

[Peter Maydell]

On 1 October 2018 at 18:18, Kevin Wolf <address@hidden> wrote:
> From: Alberto Garcia <address@hidden>
>
> The file-posix code is used for the "file", "host_device" and
> "host_cdrom" drivers, and it allows reopening images. However the only
> option that is actually processed is "x-check-cache-dropped", and
> changes in all other options (e.g. "filename") are silently ignored:
>
>    (qemu) qemu-io virtio0 "reopen -o file.filename=no-such-file"
>
> While we could allow changing some of the other options, let's keep
> things as they are for now but return an error if the user tries to
> change any of them.
>
> Signed-off-by: Alberto Garcia <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block/file-posix.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index bc5e54560a..2da3a76355 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -849,8 +849,13 @@ static int raw_reopen_prepare(BDRVReopenState *state,
>          goto out;
>      }
>
> -    rs->check_cache_dropped = qemu_opt_get_bool(opts, 
> "x-check-cache-dropped",
> -                                                false);
> +    rs->check_cache_dropped =
> +        qemu_opt_get_bool_del(opts, "x-check-cache-dropped", false);
> +
> +    /* This driver's reopen function doesn't currently allow changing
> +     * other options, so let's put them back in the original QDict and
> +     * bdrv_reopen_prepare() will detect changes and complain. */
> +    qemu_opts_to_qdict(opts, state->options);

Hi. Coverity is suspicious about this call to qemu_opts_to_qdict()
because it returns a value which this callsite is ignoring but
almost all others don't ignore (CID 1395991). Is it correct?

(It also doesn't like the call in block.c: CID 1395989.)

thanks
-- PMM