Re: [Qemu-devel] [PATCH v2 3/3] block: Fix snapshot=on for protocol parsed from filename

[Max Reitz <mreitz@redhat.com>]

On 04.04.2014 16:56, Kevin Wolf wrote:
> Since commit 9fd3171a, BDRV_O_SNAPSHOT uses an option QDict to specify
> the originally requested image as the backing file of the newly created
> temporary snapshot. This means that the filename is stored in
> "file.filename", which is an option that is not parsed for protocol
> names. Therefore things like -drive file=nbd:localhost:10809 were
> broken because it looked for a local file with the literal name
> 'nbd:localhost:10809'.
>
> This patch changes the way BDRV_O_SNAPSHOT works once again. We now open
> the originally requested image as normal, and then do a similar
> operation as for live snapshots to put the temporary snapshot on top.
> This way, both driver specific options and parsed filenames work.
>
> As a nice side effect, this results in code movement to factor
> bdrv_append_temp_snapshot() out. This is a good preparation for moving
> its call to drive_init() and friends eventually.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> v2:
> - open the backing file for the temporary snapshot read-only from the
>    beginning so that you can get an r/w block device from a read-only
>    file using snapshot=on. Gets rid of the bdrv_reopen() whose return
>    value was ignored in v1, too.
>
>   block.c                    | 148 ++++++++++++++++++++++++---------------------
>   include/block/block.h      |   1 +
>   tests/qemu-iotests/051     |   8 +++
>   tests/qemu-iotests/051.out |  28 +++++++++
>   4 files changed, 115 insertions(+), 70 deletions(-)

Together with "block: Check bdrv_getlength() return value in 
bdrv_append_temp_snapshot()":

Reviewed-by: Max Reitz <mreitz@redhat.com>