[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] iotests: rewrite iotest 240 in python
From: |
Max Reitz |
Subject: |
Re: [PATCH v2 2/2] iotests: rewrite iotest 240 in python |
Date: |
Tue, 3 Nov 2020 13:53:06 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 |
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 <mlevitsk@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()