On 20.01.20 18:20, Vladimir Sementsov-Ogievskiy wrote: > 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..
But backup_top always unshares the write permission on the source. >> 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/ Thanks! Max >>> + >>> +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 >> > >
signature.asc
Description: OpenPGP digital signature
