[v2,1/2] qapi/block-core: Overlays are not snapshots

Commit Message

Max Reitz June 3, 2019, 8:22 p.m. UTC

A snapshot is something that reflects the state of something at a
certain point in time.  It does not change.

The file our snapshot commands create (or the node they install) is not
a snapshot, as it does change over time.  It is an overlay.  We cannot
do anything about the parameter names, but we can at least adjust the
descriptions to reflect that fact.

Signed-off-by: Max Reitz <[e-mail]>
Reviewed-by: Eric Blake <[e-mail]>
---
 qapi/block-core.json | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Markus Armbruster June 5, 2019, 5:43 a.m. UTC | #1

Max Reitz <[e-mail]> writes:

> A snapshot is something that reflects the state of something at a
> certain point in time.  It does not change.
>
> The file our snapshot commands create (or the node they install) is not
> a snapshot, as it does change over time.  It is an overlay.  We cannot
> do anything about the parameter names,

We certainly could: add new parameter, default to the old one, error out
when both are given, deprecate the old one.  Way more trouble than it's
worth.

If the QAPI machinery made such renames as simple as

    'overlay-file': {'type': 'str', 'alias': 'snapshot-file'}

we could consider it.  Of course, whether enhancing the machinery that
way would be worthwhile depends on complexity and on use.  I think we
got bigger fish to fry.

>                                        but we can at least adjust the
> descriptions to reflect that fact.
>
> Signed-off-by: Max Reitz <[e-mail]>
> Reviewed-by: Eric Blake <[e-mail]>

Since you already got competent review, I merely glanced at the patches,
and didn't check for completeness.  No objections.

Patch

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1defcde048..df52a90736 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1279,17 +1279,17 @@ 
 #
 # Either @device or @node-name must be set but not both.
 #
-# @device: the name of the device to generate the snapshot from.
+# @device: the name of the device to take a snapshot of.
 #
 # @node-name: graph node name to generate the snapshot from (Since 2.0)
 #
-# @snapshot-file: the target of the new image. If the file exists, or
-# if it is a device, the snapshot will be created in the existing
-# file/device. Otherwise, a new file will be created.
+# @snapshot-file: the target of the new overlay image. If the file
+# exists, or if it is a device, the overlay will be created in the
+# existing file/device. Otherwise, a new file will be created.
 #
 # @snapshot-node-name: the graph node name of the new image (Since 2.0)
 #
-# @format: the format of the snapshot image, default is 'qcow2'.
+# @format: the format of the overlay image, default is 'qcow2'.
 #
 # @mode: whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
@@ -1302,10 +1302,10 @@ 
 ##
 # @BlockdevSnapshot:
 #
-# @node: device or node name that will have a snapshot created.
+# @node: device or node name that will have a snapshot taken.
 #
 # @overlay: reference to the existing block device that will become
-#           the overlay of @node, as part of creating the snapshot.
+#           the overlay of @node, as part of taking the snapshot.
 #           It must not have a current backing file (this can be
 #           achieved by passing "backing": null to blockdev-add).
 #
@@ -1443,7 +1443,7 @@ 
 ##
 # @blockdev-snapshot-sync:
 #
-# Generates a synchronous snapshot of a block device.
+# Takes a synchronous snapshot of a block device.
 #
 # For the arguments, see the documentation of BlockdevSnapshotSync.
 #
@@ -1469,9 +1469,9 @@ 
 ##
 # @blockdev-snapshot:
 #
-# Generates a snapshot of a block device.
+# Takes a snapshot of a block device.
 #
-# Create a snapshot, by installing 'node' as the backing image of
+# Take a snapshot, by installing 'node' as the backing image of
 # 'overlay'. Additionally, if 'node' is associated with a block
 # device, the block device changes to using 'overlay' as its new active
 # image.