[Qemu-devel] [PATCH 0/2] Don't allow snapshots if the overlay has parent nodes

[Qemu-devel] [PATCH 0/2] Don't allow snapshots if the overlay has parent nodes

[Alberto Garcia]

There are several sanity checks for the 'blockdev-snapshot' command,
but none covers the use of a file BDS as the overlay node.

      { 'execute': 'blockdev-add', 'arguments':
        { 'options': { 'driver': 'qcow2',
                       'node-name': 'new0',
                       'file': { 'driver': 'file',
                                 'filename': 'new.qcow2',
                                 'node-name': 'file0' } } } }

      { 'execute': 'blockdev-snapshot', 'arguments':
        { 'node': 'virtio0',
          'overlay': 'file0' } }

This series fixes that and adds a new test case. This of course
depends on the 'blockdev-snapshot' series:

   https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00974.html

I anyway wonder if it wouldn't be a good idea to have regular op
blockers in all file BDSs?

Regards,

Berto

Alberto Garcia (2):
  block: Don't allow snapshots if the overlay has parent nodes
  block: test 'blockdev-snapshot' using a file BDS as the overlay

 blockdev.c                 |  5 +++++
 tests/qemu-iotests/085     | 12 +++++++++++-
 tests/qemu-iotests/085.out |  4 ++++
 3 files changed, 20 insertions(+), 1 deletion(-)

-- 
2.6.1

[Qemu-devel] [PATCH 1/2] block: Don't allow snapshots if the overlay has parent nodes

[Alberto Garcia]

This addresses scenarios where the overlay node of the
'blockdev-snapshot' parameter is a child of an existing node,
such as this one:

  { 'execute': 'blockdev-add', 'arguments':
    { 'options': { 'driver': 'qcow2',
                   'node-name': 'new0',
                   'file': { 'driver': 'file',
                             'filename': 'new.qcow2',
                             'node-name': 'file0' } } } }

  { 'execute': 'blockdev-snapshot', 'arguments':
    { 'node': 'virtio0',
      'overlay': 'file0' } }

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index a567a05..fcddb07 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1667,6 +1667,11 @@ static void external_snapshot_prepare(BlkTransactionState *common,
 
     if (state->new_bs->backing != NULL) {
         error_setg(errp, "The snapshot already has a backing image");
+        return;
+    }
+
+    if (!QLIST_EMPTY(&state->new_bs->parents)) {
+        error_setg(errp, "The snapshot is already being used");
     }
 }
 
-- 
2.6.1

[Qemu-devel] [PATCH 2/2] block: test 'blockdev-snapshot' using a file BDS as the overlay

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/qemu-iotests/085     | 12 +++++++++++-
 tests/qemu-iotests/085.out |  4 ++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
index 9484117..ccde2ae 100755
--- a/tests/qemu-iotests/085
+++ b/tests/qemu-iotests/085
@@ -103,7 +103,8 @@ function add_snapshot_image()
            { 'options':
              { 'driver': 'qcow2', 'node-name': 'snap_"${1}"', "${extra_params}"
                'file':
-               { 'driver': 'file', 'filename': '"${snapshot_file}"' } } } }"
+               { 'driver': 'file', 'filename': '"${snapshot_file}"',
+                 'node-name': 'file_"${1}"' } } } }"
     _send_qemu_cmd $h "${cmd}" "return"
 }
 
@@ -187,6 +188,15 @@ add_snapshot_image ${SNAPSHOTS}
 blockdev_snapshot ${SNAPSHOTS}
 
 echo
+echo === Invalid command - cannot create a snapshot using a file BDS ===
+echo
+
+_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot',
+                     'arguments': { 'node':'virtio0',
+                                    'overlay':'file_"${SNAPSHOTS}"' }
+                   }" "error"
+
+echo
 echo === Invalid command - snapshot node used as active layer ===
 echo
 
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 52292ea..65395d3 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -62,6 +62,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/
 {"return": {}}
 {"return": {}}
 
+=== Invalid command - cannot create a snapshot using a file BDS ===
+
+{"error": {"class": "GenericError", "desc": "The snapshot is already being used"}}
+
 === Invalid command - snapshot node used as active layer ===
 
 {"error": {"class": "GenericError", "desc": "The snapshot is already in use by virtio0"}}
-- 
2.6.1

Re: [Qemu-devel] [PATCH 0/2] Don't allow snapshots if the overlay has parent nodes

[Max Reitz]

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

On 29.10.2015 14:00, Alberto Garcia wrote:
> There are several sanity checks for the 'blockdev-snapshot' command,
> but none covers the use of a file BDS as the overlay node.
> 
>       { 'execute': 'blockdev-add', 'arguments':
>         { 'options': { 'driver': 'qcow2',
>                        'node-name': 'new0',
>                        'file': { 'driver': 'file',
>                                  'filename': 'new.qcow2',
>                                  'node-name': 'file0' } } } }
> 
>       { 'execute': 'blockdev-snapshot', 'arguments':
>         { 'node': 'virtio0',
>           'overlay': 'file0' } }

Well, I don't think the problem here is that file0 has a parent. For
instance, you might want to use a qcow2 child node of a quorum BDS as
the new overlay; or in the future, you may want to use a snapshot node
which has some filters on top of it.

To me, the problem is that file0 is driven by the driver "file" which
does not support backing files. I think that's what should be fixed.

Max

> This series fixes that and adds a new test case. This of course
> depends on the 'blockdev-snapshot' series:
> 
>    https://lists.gnu.org/archive/html/qemu-block/2015-10/msg00974.html
> 
> I anyway wonder if it wouldn't be a good idea to have regular op
> blockers in all file BDSs?
> 
> Regards,
> 
> Berto
> 
> Alberto Garcia (2):
>   block: Don't allow snapshots if the overlay has parent nodes
>   block: test 'blockdev-snapshot' using a file BDS as the overlay
> 
>  blockdev.c                 |  5 +++++
>  tests/qemu-iotests/085     | 12 +++++++++++-
>  tests/qemu-iotests/085.out |  4 ++++
>  3 files changed, 20 insertions(+), 1 deletion(-)
> 



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