Re: [PATCH 4/4] iotests: rewrite iotest 240 in python

[Paolo Bonzini <pbonzini@redhat.com>]

On 29/10/20 12:16, Christian Borntraeger wrote:
> On 19.10.20 18:37, 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).
> 
> While this is now fine for x86, this seems to not cover the s390 specific ccw bus:

You can add a filter to qmp_log calls that do device_add, for example

  filters=((lambda x: x.replace("virtio-scsi-ccw", "virtio-scsi-pci")),)

Paolo

> --- /home/cborntra/REPOS/qemu/tests/qemu-iotests/240.out	2020-10-29 12:14:42.409233949 +0100
> +++ /home/cborntra/REPOS/qemu/build/240.out.bad	2020-10-29 12:15:29.309233949 +0100
> @@ -3,7 +3,7 @@
>  {"return": {}}
>  {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
>  {"return": {}}
> -{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-ccw", "id": "scsi0", "iothread": "iothread0"}}
>  {"return": {}}
>  {"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>  {"return": {}}
> @@ -22,7 +22,7 @@
>  {"return": {}}
>  {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
>  {"return": {}}
> -{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-ccw", "id": "scsi0", "iothread": "iothread0"}}
>  {"return": {}}
>  {"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>  {"return": {}}
> @@ -43,9 +43,9 @@
>  {"return": {}}
>  {"execute": "object-add", "arguments": {"id": "iothread1", "qom-type": "iothread"}}
>  {"return": {}}
> -{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-ccw", "id": "scsi0", "iothread": "iothread0"}}
>  {"return": {}}
> -{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi1", "iothread": "iothread1"}}
> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-ccw", "id": "scsi1", "iothread": "iothread1"}}
>  {"return": {}}
>  {"execute": "device_add", "arguments": {"bus": "scsi0.0", "drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>  {"return": {}}
> @@ -72,7 +72,7 @@
>  {"return": {}}
>  {"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
>  {"return": {}}
> -{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-ccw", "id": "scsi0", "iothread": "iothread0"}}
>  {"return": {}}
>  {"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>  {"return": {}}
> Failures: 240
> Failed 1 of 1 iotests
> 
>>
>> 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..a739de6769 100755
>> --- a/tests/qemu-iotests/240
>> +++ b/tests/qemu-iotests/240
>> @@ -1,5 +1,5 @@
>> -#!/usr/bin/env bash
>> -#
>> +#!/usr/bin/env python3
>> +
>>  # Test hot plugging and unplugging with iothreads
>>  #
>>  # Copyright (C) 2019 Igalia, S.L.
>> @@ -17,133 +17,99 @@
>>  #
>>  # 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=berto@igalia.com
>> -
>> -seq=`basename $0`
>> -echo "QA output created by $seq"
>> -
>> -status=1	# failure is the default!
>> -
>> -_cleanup()
>> -{
>> -    rm -f "$SOCK_DIR/nbd"
>> -}
>> -trap "_cleanup; exit \$status" 0 1 2 3 15
>> -
>> -# get standard environment, filters and checks
>> -. ./common.rc
>> -. ./common.filter
>> -
>> -_supported_fmt generic
>> -_supported_proto generic
>> -
>> -do_run_qemu()
>> -{
>> -    echo Testing: "$@"
>> -    $QEMU -nographic -qmp stdio -serial none "$@"
>> -    echo
>> -}
>> -
>> -# Remove QMP events from (pretty-printed) output. Doesn't handle
>> -# nested dicts correctly, but we don't get any of those in this test.
>> -_filter_qmp_events()
>> -{
>> -    tr '\n' '\t' | sed -e \
>> -	's/{\s*"timestamp":\s*{[^}]*},\s*"event":[^,}]*\(,\s*"data":\s*{[^}]*}\)\?\s*}\s*//g' \
>> -	| tr '\t' '\n'
>> -}
>> -
>> -run_qemu()
>> -{
>> -    do_run_qemu "$@" 2>&1 | _filter_qmp | _filter_qmp_events
>> -}
>> -
>> -case "$QEMU_DEFAULT_MACHINE" in
>> -  s390-ccw-virtio)
>> -      virtio_scsi=virtio-scsi-ccw
>> -      ;;
>> -  *)
>> -      virtio_scsi=virtio-scsi-pci
>> -      ;;
>> -esac
>> -
>> -echo
>> -echo === Unplug a SCSI disk and then plug it again ===
>> -echo
>> -
>> -run_qemu <<EOF
>> -{ "execute": "qmp_capabilities" }
>> -{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0"}}
>> -{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi0"}}
>> -{ "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
>> -{ "execute": "quit"}
>> -EOF
>> -
>> -echo
>> -echo === Attach two SCSI disks using the same block device and the same iothread ===
>> -echo
>> -
>> -run_qemu <<EOF
>> -{ "execute": "qmp_capabilities" }
>> -{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0", "read-only": true}}
>> -{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd1", "driver": "scsi-hd", "drive": "hd0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi-hd1"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi0"}}
>> -{ "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
>> -{ "execute": "quit"}
>> -EOF
>> -
>> -echo
>> -echo === Attach two SCSI disks using the same block device but different iothreads ===
>> -echo
>> -
>> -run_qemu <<EOF
>> -{ "execute": "qmp_capabilities" }
>> -{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0", "read-only": true}}
>> -{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
>> -{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread1"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi1", "driver": "${virtio_scsi}", "iothread": "iothread1"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi0.0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd1", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi1.0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd1", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi1.0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi-hd1"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi0"}}
>> -{ "execute": "device_del", "arguments": {"id": "scsi1"}}
>> -{ "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
>> -{ "execute": "quit"}
>> -EOF
>> -
>> -echo
>> -echo === Attach a SCSI disks using the same block device as a NBD server ===
>> -echo
>> -
>> -run_qemu <<EOF
>> -{ "execute": "qmp_capabilities" }
>> -{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "read-zeroes": true, "node-name": "hd0", "read-only": true}}
>> -{ "execute": "nbd-server-start", "arguments": {"addr":{"type":"unix","data":{"path":"$SOCK_DIR/nbd"}}}}
>> -{ "execute": "nbd-server-add", "arguments": {"device":"hd0"}}
>> -{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": "iothread0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi0", "driver": "${virtio_scsi}", "iothread": "iothread0"}}
>> -{ "execute": "device_add", "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0", "bus": "scsi0.0"}}
>> -{ "execute": "quit"}
>> -EOF
>> -
>> -# success, all done
>> -echo "*** done"
>> -rm -f $seq.full
>> -status=0
>> +import iotests
>> +import os
>> +
>> +nbd_sock = iotests.file_path('nbd.sock', base_dir=iotests.sock_dir)
>> +
>> +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()
>> +
>> +    def tearDown(self):
>> +        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')
>> +        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')
>> +        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')
>> +
>> +        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')
>> +        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')
>> +        self.vm.qmp_log('device_add', id='scsi1', driver=iotests.get_virtio_scsi_device(), iothread='iothread1')
>> +
>> +        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')
>> +        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')
>> +    iotests.activate_logging()
>> +    iotests.main()
>> diff --git a/tests/qemu-iotests/240.out b/tests/qemu-iotests/240.out
>> index d00df50297..24847be6b3 100644
>> --- a/tests/qemu-iotests/240.out
>> +++ b/tests/qemu-iotests/240.out
>> @@ -1,67 +1,83 @@
>> -QA output created by 240
>> -
>> -=== Unplug a SCSI disk and then plug it again ===
>> -
>> -Testing:
>> -QMP_VERSION
>> -{"return": {}}
>> +==Unplug a SCSI disk and then plug it again==
>> +{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-zeroes": true}}
>>  {"return": {}}
>> +{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi0"}}
>>  {"return": {}}
>> +{"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
>>  {"return": {}}
>> +==Attach two SCSI disks using the same block device and the same iothread==
>> +{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
>>  {"return": {}}
>> -
>> -=== Attach two SCSI disks using the same block device and the same iothread ===
>> -
>> -Testing:
>> -QMP_VERSION
>> +{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd1"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi-hd1"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi0"}}
>>  {"return": {}}
>> +{"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
>>  {"return": {}}
>> +==Attach two SCSI disks using the same block device but different iothreads==
>> +{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
>>  {"return": {}}
>> +{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
>>  {"return": {}}
>> +{"execute": "object-add", "arguments": {"id": "iothread1", "qom-type": "iothread"}}
>>  {"return": {}}
>> -
>> -=== Attach two SCSI disks using the same block device but different iothreads ===
>> -
>> -Testing:
>> -QMP_VERSION
>> -{"return": {}}
>> -{"return": {}}
>> -{"return": {}}
>> -{"return": {}}
>> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi1", "iothread": "iothread1"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"bus": "scsi0.0", "drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"bus": "scsi1.0", "drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd1"}}
>>  {"error": {"class": "GenericError", "desc": "Cannot change iothread of active block backend"}}
>> +{"execute": "device_del", "arguments": {"id": "scsi-hd0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"bus": "scsi1.0", "drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd1"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi-hd1"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi1"}}
>>  {"return": {}}
>> +{"execute": "device_del", "arguments": {"id": "scsi0"}}
>>  {"return": {}}
>> +{"execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
>>  {"return": {}}
>> +==Attach a SCSI disks using the same block device as a NBD server==
>> +{"execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": "hd0", "read-only": true, "read-zeroes": true}}
>>  {"return": {}}
>> -
>> -=== Attach a SCSI disks using the same block device as a NBD server ===
>> -
>> -Testing:
>> -QMP_VERSION
>> -{"return": {}}
>> -{"return": {}}
>> +{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}}
>>  {"return": {}}
>> +{"execute": "nbd-server-add", "arguments": {"device": "hd0"}}
>>  {"return": {}}
>> +{"execute": "object-add", "arguments": {"id": "iothread0", "qom-type": "iothread"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"driver": "virtio-scsi-pci", "id": "scsi0", "iothread": "iothread0"}}
>>  {"return": {}}
>> +{"execute": "device_add", "arguments": {"drive": "hd0", "driver": "scsi-hd", "id": "scsi-hd0"}}
>>  {"return": {}}
>> -{"return": {}}
>> -*** done
>> +....
>> +----------------------------------------------------------------------
>> +Ran 4 tests
>> +
>> +OK
>>
>