Re: [Qemu-devel] [PATCH 06/19] block: Make bdrv_default_refresh_format_filename public

[Kevin Wolf]

Am 26.04.2016 um 23:32 hat Max Reitz geschrieben:
> In order to allow block drivers to use that function, it needs to be
> public. In order to be useful, it needs to take a parameter which allows
> the caller to specify whether the runtime options allowed by the block
> driver are actually significant for the guest-visible BDS content.
> 
> Signed-off-by: Max Reitz <address@hidden>

Is this actually good enough? I expect that many drivers will have some
options that are significant and other options that aren't. We already
have some (Quorum: children are significant, rewrite-corrupted isn't),
but as we convert more things to proper options, we'll get more of them
(raw-posix: filename is significant, aio=native isn't).

We might actually need to pass a list of significant fields instead that
append_open_options() can use.

Kevin

>  block.c                   | 9 ++++++---
>  include/block/block_int.h | 3 +++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 511a0b1..b3ff08f 100644
> --- a/block.c
> +++ b/block.c
> @@ -3872,7 +3872,10 @@ static bool append_open_options(QDict *d, 
> BlockDriverState *bs)
>      return found_any;
>  }
>  
> -static void bdrv_default_refresh_format_filename(BlockDriverState *bs)
> +/* @options_significant must be true if any of the driver-specific runtime
> + * options may change the guest-visible content of the BDS */
> +void bdrv_default_refresh_format_filename(BlockDriverState *bs,
> +                                          bool options_significant)
>  {
>      BlockDriver *drv = bs->drv;
>      QDict *opts;
> @@ -3885,7 +3888,7 @@ static void 
> bdrv_default_refresh_format_filename(BlockDriverState *bs)
>      }
>  
>      opts = qdict_new();
> -    has_open_options = append_open_options(opts, bs);
> +    has_open_options = append_open_options(opts, bs) && options_significant;
>
>      has_open_options |= bs->backing_overridden;
>  
>      /* If no specific options have been given for this BDS, the filename of
> @@ -3997,7 +4000,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
>          QDECREF(opts);
>      } else if (bs->file) {
>          /* Try to reconstruct valid information from the underlying file */
> -        bdrv_default_refresh_format_filename(bs);
> +        bdrv_default_refresh_format_filename(bs, true);
>      } else if (!bs->full_open_options && qdict_size(bs->options)) {
>          bdrv_default_refresh_protocol_filename(bs);
>      }
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index d73e9ce..eb3665a 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -532,6 +532,9 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int 
> buf_size,
>  void bdrv_set_io_limits(BlockDriverState *bs,
>                          ThrottleConfig *cfg);
>  
> +void bdrv_default_refresh_format_filename(BlockDriverState *bs,
> +                                          bool options_significant);
> +
>  
>  /**
>   * bdrv_add_before_write_notifier:
> -- 
> 2.8.0
>