On Wed, Sep 14, 2016 at 06:52:15PM +0300, Alberto Garcia wrote:
> If an image is opened with snapshot=on, its flags are modified by
> bdrv_backing_options() and then bs->open_flags is updated accordingly.
> This last step is unnecessary if we calculate the new flags before
> setting bs->open_flags.
>
> Soon we'll introduce the "read-only" option, and then we'll need to be
> able to modify its value in the QDict when snapshot=on. This is more
> cumbersome if bs->options is already set. This patch simplifies that.
>
> The code that sets BDRV_O_ALLOW_RDWR is also moved for the same
> reason.
>
Before, we would not set BDRV_O_ALLOW_RDWR for protocols, but this will
change that. Is that side-affect intentional?
> Signed-off-by: Alberto Garcia <be...@igalia.com>
> ---
> block.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/block.c b/block.c
> index 1c75a6f..7cae841 100644
> --- a/block.c
> +++ b/block.c
> @@ -1627,6 +1627,17 @@ static BlockDriverState *bdrv_open_inherit(const char
> *filename,
> goto fail;
> }
>
> + if (flags & BDRV_O_RDWR) {
> + flags |= BDRV_O_ALLOW_RDWR;
> + }
> +
> + if (flags & BDRV_O_SNAPSHOT) {
> + snapshot_options = qdict_new();
> + bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
> + flags, options);
> + bdrv_backing_options(&flags, options, flags, options);
> + }
> +
> bs->open_flags = flags;
> bs->options = options;
> options = qdict_clone_shallow(options);
> @@ -1651,18 +1662,6 @@ static BlockDriverState *bdrv_open_inherit(const char
> *filename,
>
> /* Open image file without format layer */
> if ((flags & BDRV_O_PROTOCOL) == 0) {
> - if (flags & BDRV_O_RDWR) {
> - flags |= BDRV_O_ALLOW_RDWR;
> - }
> - if (flags & BDRV_O_SNAPSHOT) {
> - snapshot_options = qdict_new();
> - bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
> - flags, options);
> - bdrv_backing_options(&flags, options, flags, options);
> - }
> -
> - bs->open_flags = flags;
> -
> file = bdrv_open_child(filename, options, "file", bs,
> &child_file, true, &local_err);
> if (local_err) {
