On 01.11.20 17:15, Maxim Levitsky wrote: > The recent changes that brought RCU delayed device deletion, > broke few tests and this test breakage went unnoticed. > > Fix this test by rewriting it in python > (which allows to wait for DEVICE_DELETED events before continuing). > > Signed-off-by: Maxim Levitsky <mlevi...@redhat.com> > --- > tests/qemu-iotests/240 | 228 ++++++++++++++++--------------------- > tests/qemu-iotests/240.out | 76 ++++++++----- > 2 files changed, 143 insertions(+), 161 deletions(-) > > diff --git a/tests/qemu-iotests/240 b/tests/qemu-iotests/240 > index 8b4337b58d..bfc9b72f36 100755 > --- a/tests/qemu-iotests/240 > +++ b/tests/qemu-iotests/240
[...] > +class TestCase(iotests.QMPTestCase): > + test_driver = "null-co" > + > + def required_drivers(self): > + return [self.test_driver] > + > + @iotests.skip_if_unsupported(required_drivers) > + def setUp(self): > + self.vm = iotests.VM() > + self.vm.launch() It seems to me like all tests create a null-co block device. The only difference is that test1() creates an R/W node, whereas all others create an RO node. I don’t think that matters though, so maybe we can replace code duplication by creating the (RO) null-co node here. Furthermore, we could also create two I/O threads and two accompanying virtio-scsi devices here (tests that don’t use them shouldn’t care)... > + > + def tearDown(self): ...and clean all of those up here. > + self.vm.shutdown() > + > + def test1(self): > + iotests.log('==Unplug a SCSI disk and then plug it again==') > + self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, > node_name='hd0') > + self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0") > + self.vm.qmp_log('device_add', id='scsi0', > driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters = > [iotests.filter_qmp_virtio_scsi]) A bit weird that you change your coding style for the @filters parameter (i.e., putting spaces around '='), and pylint thinks so, too. > + self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', > drive='hd0') > + self.vm.qmp_log('device_del', id='scsi-hd0') > + self.vm.event_wait('DEVICE_DELETED') > + self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', > drive='hd0') > + self.vm.qmp_log('device_del', id='scsi-hd0') > + self.vm.event_wait('DEVICE_DELETED') > + self.vm.qmp_log('device_del', id='scsi0') I don’t think it makes a difference for this test, but perhaps it would be cleaner to wait for the DEVICE_DELETED event even for the virtio-scsi devices? > + self.vm.qmp_log('blockdev-del', node_name='hd0') > + > + def test2(self): > + iotests.log('==Attach two SCSI disks using the same block device and > the same iothread==') > + self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, > node_name='hd0', read_only=True) > + self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0") > + self.vm.qmp_log('device_add', id='scsi0', > driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters = > [iotests.filter_qmp_virtio_scsi]) > + > + self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', > drive='hd0') > + self.vm.qmp_log('device_add', id='scsi-hd1', driver='scsi-hd', > drive='hd0') > + self.vm.qmp_log('device_del', id='scsi-hd1') > + self.vm.event_wait('DEVICE_DELETED') > + self.vm.qmp_log('device_del', id='scsi-hd0') > + self.vm.event_wait('DEVICE_DELETED') The bash version deleted them the other way around, but I suppose it doesn’t matter. (It just made me wonder why you changed the order.) > + self.vm.qmp_log('device_del', id='scsi0') > + self.vm.qmp_log('blockdev-del', node_name='hd0') > + > + def test3(self): > + iotests.log('==Attach two SCSI disks using the same block device but > different iothreads==') > + > + self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, > node_name='hd0', read_only=True) > + > + self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0") > + self.vm.qmp_log('object-add', qom_type='iothread', id="iothread1") > + > + self.vm.qmp_log('device_add', id='scsi0', > driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters = > [iotests.filter_qmp_virtio_scsi]) > + self.vm.qmp_log('device_add', id='scsi1', > driver=iotests.get_virtio_scsi_device(), iothread='iothread1', filters = > [iotests.filter_qmp_virtio_scsi]) > + > + self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', > drive='hd0', bus="scsi0.0") > + self.vm.qmp_log('device_add', id='scsi-hd1', driver='scsi-hd', > drive='hd0', bus="scsi1.0") > + > + self.vm.qmp_log('device_del', id='scsi-hd0') > + self.vm.event_wait('DEVICE_DELETED') > + self.vm.qmp_log('device_add', id='scsi-hd1', driver='scsi-hd', > drive='hd0', bus="scsi1.0") > + > + self.vm.qmp_log('device_del', id='scsi-hd1') > + self.vm.event_wait('DEVICE_DELETED') > + > + self.vm.qmp_log('device_del', id='scsi1') > + self.vm.qmp_log('device_del', id='scsi0') > + > + self.vm.qmp_log('blockdev-del', node_name='hd0') > + > + def test4(self): > + iotests.log('==Attach a SCSI disks using the same block device as a > NBD server==') > + > + self.vm.qmp_log('blockdev-add', driver='null-co', read_zeroes=True, > node_name='hd0', read_only=True) > + > + self.vm.qmp_log('nbd-server-start', > + filters=[iotests.filter_qmp_testfiles], > + addr={'type':'unix', 'data':{'path':nbd_sock}}) > + > + self.vm.qmp_log('nbd-server-add', device='hd0') > + > + self.vm.qmp_log('object-add', qom_type='iothread', id="iothread0") > + self.vm.qmp_log('device_add', id='scsi0', > driver=iotests.get_virtio_scsi_device(), iothread='iothread0', filters = > [iotests.filter_qmp_virtio_scsi]) > + self.vm.qmp_log('device_add', id='scsi-hd0', driver='scsi-hd', > drive='hd0') > + > + > +if __name__ == '__main__': > + if 'null-co' not in iotests.supported_formats(): > + iotests.notrun('null-co driver support missing') I suppose it isn’t wrong to check this in two places (in the setUp() decorator, and then here again), but it doesn’t seem necessary either. Max > + iotests.activate_logging() > + iotests.main()