[Qemu-devel] [PATCH 0/2] blockdev: Overlays are not snapshots

[Qemu-devel] [PATCH 0/2] blockdev: Overlays are not snapshots

[Max Reitz]

QEMU’s always been confused over what a snapshot is: Is it the overlay?
Is it the backing image?

Confusion is rarely a good thing.  I can’t think of any objective reason
why the overlay would be a snapshot.  A snapshot is something that does
not change over time; the overlay does.

(I suppose historically the reason is that “Taking an overlay” makes no
sense, so the operations are called “Taking a snapshot”.  Somehow, this
meaning carried over to the new file that is created during that
operation; if “Creating a snapshot” creates a file, that file must be
the snapshot, right?  Well, no, it isn’t.)

Let’s fix this as best as we can.  Better Nate than lever.


Mae Reitz (2):
  qapi/block-core: Overlays are not snapshots
  blockdev: Overlays are not snapshots

 qapi/block-core.json | 20 ++++++++++----------
 blockdev.c           | 10 +++++-----
 2 files changed, 15 insertions(+), 15 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH 1/2] qapi/block-core: Overlays are not snapshots

[Max Reitz]

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 <mreitz@redhat.com>
---
 qapi/block-core.json | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

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.
-- 
2.21.0

[Qemu-devel] [PATCH 2/2] blockdev: Overlays are not snapshots

[Max Reitz]

There are error messages which refer to an overlay node as the snapshot.
That is wrong, those are two different things.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 blockdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6963270108..7de0b04fe7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1608,13 +1608,13 @@ static void external_snapshot_prepare(BlkActionState *common,
             s->has_snapshot_node_name ? s->snapshot_node_name : NULL;
 
         if (node_name && !snapshot_node_name) {
-            error_setg(errp, "New snapshot node name missing");
+            error_setg(errp, "New overlay node name missing");
             goto out;
         }
 
         if (snapshot_node_name &&
             bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
-            error_setg(errp, "New snapshot node name already in use");
+            error_setg(errp, "New overlay node name already in use");
             goto out;
         }
 
@@ -1656,7 +1656,7 @@ static void external_snapshot_prepare(BlkActionState *common,
     }
 
     if (bdrv_has_blk(state->new_bs)) {
-        error_setg(errp, "The snapshot is already in use");
+        error_setg(errp, "The overlay is already in use");
         goto out;
     }
 
@@ -1666,12 +1666,12 @@ static void external_snapshot_prepare(BlkActionState *common,
     }
 
     if (state->new_bs->backing != NULL) {
-        error_setg(errp, "The snapshot already has a backing image");
+        error_setg(errp, "The overlay already has a backing image");
         goto out;
     }
 
     if (!state->new_bs->drv->supports_backing) {
-        error_setg(errp, "The snapshot does not support backing images");
+        error_setg(errp, "The overlay does not support backing images");
         goto out;
     }
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH 1/2] qapi/block-core: Overlays are not snapshots

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On 6/3/19 11:25 AM, Max Reitz wrote:
> 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 <mreitz@redhat.com>
> ---
>  qapi/block-core.json | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 2/2] blockdev: Overlays are not snapshots

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 489 bytes --]

On 6/3/19 11:25 AM, Max Reitz wrote:
> There are error messages which refer to an overlay node as the snapshot.
> That is wrong, those are two different things.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  blockdev.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 2/2] blockdev: Overlays are not snapshots

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

On 03.06.19 18:25, Max Reitz wrote:
> There are error messages which refer to an overlay node as the snapshot.
> That is wrong, those are two different things.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  blockdev.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Oops.  I remember I wanted to run the iotests and see what fails, but
then I forgot to do so.  Of course something fails, so I’ll send a v2.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]