Re: [Qemu-devel] [PATCH v2 2/2] block: add a 'blockdev-snapshot' QMP command

[Alberto Garcia]

On Fri 04 Sep 2015 04:42:17 PM CEST, Eric Blake <address@hidden> wrote:
>> +    if (snapshot_ref) {
>> +        if (!bdrv_lookup_bs(snapshot_ref, snapshot_ref, &local_err)) {
>>              error_propagate(errp, local_err);
>>              return;
>>          }
>>      }
>
> Shouldn't you also check that snapshot_ref does not currently have a
> backing BDS (as it is the act of creating the snapshot that sets the
> current device as the backing of the snapshot_ref BDS before altering
> the BB to point to snapshot_ref as its new BDS)?

Wait, I actually think that it should have a backing BDS. This new
command is roughly equivalent to the 'existing' mode of the
'blockdev-snapshot-sync' command, so the new image must be created
externally in both cases having the current image as its backing file.

The difference is that 'blockdev-snapshot-sync' will open the new image
and not its backing chain (using the BDRV_O_NO_BACKING flag), but in
'blockdev-snapshot' the image must be opened previously with
'blockdev-add', which will open the whole chain.

So I think that we should expect a backing image and we have to unref it
during the 'commit' part of the transaction (before bdrv_append()).

Berto