[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] block: add a 'blockdev-snapshot' QMP com
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] block: add a 'blockdev-snapshot' QMP command |
Date: |
Mon, 07 Sep 2015 13:31:13 +0200 |
User-agent: |
Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu) |
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
[Qemu-devel] [PATCH v2 1/2] block: rename BlockdevSnapshot to BlockdevSnapshotSync, Alberto Garcia, 2015/09/04