20.01.2020 20:04, Max Reitz wrote: > On 16.01.20 16:54, Vladimir Sementsov-Ogievskiy wrote: >> This test checks that bug is really fixed by previous commit. >> >> Cc: qemu-sta...@nongnu.org # v4.2.0 >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> tests/qemu-iotests/283 | 75 ++++++++++++++++++++++++++++++++++++++ >> tests/qemu-iotests/283.out | 8 ++++ >> tests/qemu-iotests/group | 1 + >> 3 files changed, 84 insertions(+) >> create mode 100644 tests/qemu-iotests/283 >> create mode 100644 tests/qemu-iotests/283.out > > The test looks good to me, I just have a comment nit and a note on the > fact that this should probably be queued only after Thomas’s “Enable > more iotests during "make check-block"” series. > >> diff --git a/tests/qemu-iotests/283 b/tests/qemu-iotests/283 >> new file mode 100644 >> index 0000000000..f0f216d109 >> --- /dev/null >> +++ b/tests/qemu-iotests/283 >> @@ -0,0 +1,75 @@ >> +#!/usr/bin/env python >> +# >> +# Test for backup-top filter permission activation failure >> +# >> +# Copyright (c) 2019 Virtuozzo International GmbH. >> +# >> +# This program is free software; you can redistribute it and/or modify >> +# it under the terms of the GNU General Public License as published by >> +# the Free Software Foundation; either version 2 of the License, or >> +# (at your option) any later version. >> +# >> +# This program is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program. If not, see <http://www.gnu.org/licenses/>. >> +# >> + >> +import iotests >> + >> +# The test is unrelated to formats, restrict it to qcow2 to avoid extra runs >> +iotests.verify_image_format(supported_fmts=['qcow2']) >> + >> +size = 1024 * 1024 >> + >> +""" >> +On activation, backup-top is going to unshare write permission on its >> +source child. It will be impossible for the following configuration: > > “The following configuration will become impossible”?
Hmm, no, the configuration is possible. But "it", i.e. "unshare write permission", is impossible with such configuration.. > > I think there should be some note that this is exactly what we want to > test, i.e. what happens when this impossible configuration is attempted > by starting a backup. (And maybe why this isn’t allowed; namely because > we couldn’t do CBW for such write accesses.) > >> + >> + ┌────────┐ target ┌─────────────┐ >> + │ target │ ◀─────── │ backup_top │ >> + └────────┘ └─────────────┘ >> + │ >> + │ backing >> + ▼ >> + ┌─────────────┐ >> + │ source │ >> + └─────────────┘ >> + │ >> + │ file >> + ▼ >> + ┌─────────────┐ write perm ┌───────┐ >> + │ base │ ◀──────────── │ other │ >> + └─────────────┘ └───────┘ > > Cool Unicode art. :-) I found the great tool: https://dot-to-ascii.ggerganov.com/ > >> + >> +Write unsharing will be propagated to the "source->base"link and will >> +conflict with other node write permission. >> + >> +(Note, that we can't just consider source to be direct child of other, >> +as in this case this link will be broken, when backup_top is appended) >> +""" >> + >> +vm = iotests.VM() >> +vm.launch() >> + >> +vm.qmp_log('blockdev-add', **{'node-name': 'target', 'driver': 'null-co'}) >> + >> +vm.qmp_log('blockdev-add', **{ >> + 'node-name': 'source', >> + 'driver': 'blkdebug', >> + 'image': {'node-name': 'base', 'driver': 'null-co', 'size': size} >> +}) >> + >> +vm.qmp_log('blockdev-add', **{ >> + 'node-name': 'other', >> + 'driver': 'blkdebug', >> + 'image': 'base', >> + 'take-child-perms': ['write'] >> +}) >> + >> +vm.qmp_log('blockdev-backup', sync='full', device='source', target='target') >> + >> +vm.shutdown() > > [...] > >> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group >> index cb2b789e44..d827e8c821 100644 >> --- a/tests/qemu-iotests/group >> +++ b/tests/qemu-iotests/group >> @@ -288,3 +288,4 @@ >> 277 rw quick >> 279 rw backing quick >> 280 rw migration quick >> +283 auto quick > > Hm. This would be the first Python test in auto. Missed that. It's OK to define it just "quick" and update later. > Thomas’s series has > at least one patch that seems useful to come before we do this, namely > “Skip Python-based tests if QEMU does not support virtio-blk”. So I > suppose his series should come before this, then. > > Max > -- Best regards, Vladimir
