Re: [PATCH v2 2/2] iotests: add test for backup-top failure on permission activation

[Max Reitz]

On 21.01.20 15:28, Vladimir Sementsov-Ogievskiy wrote:
> This test checks that bug is really fixed by previous commit.
> 
> Cc: address@hidden # v4.2.0
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  tests/qemu-iotests/283     | 92 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/283.out |  8 ++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 101 insertions(+)
>  create mode 100644 tests/qemu-iotests/283
>  create mode 100644 tests/qemu-iotests/283.out
> 
> diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283
> new file mode 100644
> index 0000000000..293e557bd9
> --- /dev/null
> +++ b/tests/qemu-iotests/283
> @@ -0,0 +1,92 @@

[...]

> +""" Test description
> +
> +When performing a backup, all writes on the source subtree must go through 
> the
> +backup-top filter so it can copy all data to the target before it is changed.
> +backup-top filter is appended above source node, to achieve this thing, so 
> all
> +parents of source node are handled. A configuration with side parents of 
> source
> +sub-tree with write permission is unsupported (we'd have append several
> +backup-top filter like nodes to handle such parents). The test create an
> +example of such configuration and checks that a backup is then not allowed
> +(blockdev-backup command should fail).
> +
> +The configuration:
> +
> +    ┌────────┐  target  ┌─────────────┐
> +    │ target │ ◀─────── │ backup_top  │
> +    └────────┘          └─────────────┘
> +                            │
> +                            │ backing
> +                            ▼
> +                        ┌─────────────┐
> +                        │   source    │
> +                        └─────────────┘
> +                            │
> +                            │ file
> +                            ▼
> +                        ┌─────────────┐  write perm   ┌───────┐
> +                        │    base     │ ◀──────────── │ other │
> +                        └─────────────┘               └───────┘
> +
> +On activation (see .active field of backup-top state in block/backup-top.c),
> +backup-top is going to unshare write permission on its source child. Write
> +unsharing will be propagated to the "source->base" link and will conflict 
> with
> +other node write permission. So permission update will fail and backup job 
> will
> +not be started.
> +
> +Note, that the only thing which prevents backup of running on such
> +configuration is default permission propagation scheme. It may be altered by
> +different block drivers, so backup will run in invalid configuration. But
> +something is better than nothing. Also, before the previous commit (commit
> +preceding this test creation), starting backup on such configuration led to
> +crash, so current "something" is a lot better, and this test actual goal is
> +to check that crash is fixed :)

Thanks a lot for bearing with me!

I was wondering whether this is the first smiley in our code, but it
isn’t.  (Not unfortunately, I think. :-))  It’s also not the first
smiley in the iotests, but the second one!  (As far as I can tell.)

Max

signature.asc
Description: OpenPGP digital signature