On Fri 04 Sep 2015 04:42:17 PM CEST, Eric Blake <ebl...@redhat.com> wrote:
>> @@ -1183,6 +1183,18 @@ void qmp_blockdev_snapshot_sync(bool has_device,
>> const char *device,
>> &snapshot, errp);
>> }
>>
>> +void qmp_blockdev_snapshot(const char *device, const char *snapshot,
>
> Is 'node' a better name than 'device' here?
I don't have a strong preference (I was just following Kevin's
suggestion), but 'device' seems to be the most common name for this
parameter. This one can take a node name as well, but it will only
accept an active layer anyway... I can change the name if you prefer.
>> + 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)?
I think you're right, thanks!
>> +SQMP
>> +blockdev-snapshot
>> +-----------------
>> +Since 2.5
>> +
>> +Create a snapshot of a block device. 'device' and 'snapshot' both
>> +refer to existing block devices. The former is the one to generate
>> +the snapshot from, and the latter is the target of the snapshot.
>
> Is there any better terminology? Maybe:
>
> The act of creating a snapshot installs 'device' as the backing image
> of 'snapshot'; additionally, if 'device' is associated with a block
> device, the block device changes to using 'snapshot' as its new active
> image.
Sounds good.
> Hmm - I wonder if that means we should have an optional boolean
> parameter that allows us to avoid the automatic pivot. After all,
> with 'blockdev-snapshot-sync', you can specify 'device' and omit
> 'node-name' to update the device's active layer, or you can omit
> 'device' and specify 'node-name' to create another qcow2 file but NOT
> install it as the active layer, regardless of which 'node-name' that
> serves as the starting point. So when 'node-name' is the BDS node that
> 'device' is currently visiting, you have control over whether 'device'
> auto-updates to the new BDS.
What's the use case for that?
Berto
