On 2018-03-06 21:48, Stefan Hajnoczi wrote: > This test case adds an NBD server export and then invokes > blockdev-snapshot-sync, which changes the BlockDriverState node that the > NBD server's BlockBackend points to. This is an interesting scenario to > test and exercises the code path fixed by the previous commit. > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > --- > tests/qemu-iotests/208 | 55 > ++++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/208.out | 9 ++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 65 insertions(+) > create mode 100755 tests/qemu-iotests/208 > create mode 100644 tests/qemu-iotests/208.out > > diff --git a/tests/qemu-iotests/208 b/tests/qemu-iotests/208 > new file mode 100755 > index 0000000000..4e82b96c82 > --- /dev/null > +++ b/tests/qemu-iotests/208 > @@ -0,0 +1,55 @@ > +#!/usr/bin/env python > +# > +# Copyright (C) 2018 Red Hat, Inc. > +# > +# 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/>. > +# > +# Creator/Owner: Stefan Hajnoczi <stefa...@redhat.com> > +# > +# Check that the runtime NBD server does not crash when stopped after > +# blockdev-snapshot-sync. > + > +import iotests > + > +with iotests.FilePath('disk.img') as disk_img_path, \ > + iotests.FilePath('disk-snapshot.img') as disk_snapshot_img_path, \ > + iotests.FilePath('nbd.sock') as nbd_sock_path, \ > + iotests.VM() as vm: > + > + img_size = '10M' > + iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk_img_path, > img_size) > + > + iotests.log('Launching VM...') > + (vm.add_drive(disk_img_path, 'node-name=drive0-node', interface='none') > + .launch()) > + > + iotests.log('Starting NBD server...') > + iotests.log(vm.qmp('nbd-server-start', addr={ > + "type": "unix", > + "data": { > + "path": nbd_sock_path, > + } > + })) > + > + iotests.log('Adding NBD export...') > + iotests.log(vm.qmp('nbd-server-add', device='drive0-node', > writable=True)) > + > + iotests.log('Creating external snapshot...') > + iotests.log(vm.qmp('blockdev-snapshot-sync', > + node_name='drive0-node', > + snapshot_node_name='drive0-snapshot-node', > + snapshot_file=disk_snapshot_img_path)) > + > + iotests.log('Stopping NBD server...') > + iotests.log(vm.qmp('nbd-server-stop'))
Hm. Tests what it's supposed to test. However, I'm not sure whether that's what we want... If I give a node name to nbd-server-add, I'd probably expect the NBD server to stay at that node. We have BdrvChild.stay_at_node for that, so we could implement it. The question is whether we want to. However, both wanting the snapshot to be below and above the NBD server makes sense... Perhaps it's best to leave it as-is, even if it is surprising to some, because this way we at least don't have to switch to RO mode (which we probably would if the NBD server is suddenly below the snapshot). I'm sure adding atomic graph change operations to QMP will fix all of that. I'm sure. Anyway, since your test doesn't even check whether the NBD server is above or below the snapshot (clever! :-))... Reviewed-by: Max Reitz <mre...@redhat.com>
signature.asc
Description: OpenPGP digital signature
