On Fri 04 Sep 2015 04:42:17 PM CEST, Eric Blake <ebl...@redhat.com> 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
