Skip to contentSkip to conversation

iotests: cure s390x failures by switching to ccw

PATCH Superseded Cornelia Huck 6 participants 20 messages Sep 5, 2017 – Sep 12, 2017

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover iotests: cure s390x failures by switching to ccw

Recent changes in s390x made pci support dependant on the zpci cpu feature, which is not provided on all models (and not on by default). This means we cannot instatiate pci devices on a standard qemu invocation for s390x. Moreover, the zpci instructions are not even wired up for tcg yet, so actually doing anything with those pci devices is bound to fail on tcg.

Let's follow the existing example in 068 and switch to the (default) virtio-ccw transport on s390x. The changes for 051 and 067 are split out as they require adding an output file for s390x (the actual command lines are part of the output).

Cornelia Huck (3):
iotests: use -ccw on s390x for 040, 139, and 182
iotests: use -ccw on s390x for 051
iotests: use -ccw on s390x for 067

 tests/qemu-iotests/040                     |   6 +-
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++
 tests/qemu-iotests/067                     |  11 +-
 tests/qemu-iotests/067.s390-ccw-virtio.out | 458 +++++++++++++++++++++++++++++
 tests/qemu-iotests/139                     |  12 +-
 tests/qemu-iotests/182                     |  13 +-
 7 files changed, 936 insertions(+), 7 deletions(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out
 create mode 100644 tests/qemu-iotests/067.s390-ccw-virtio.out
Signature · 1 line
2.13.5
#1 lore raw

PATCH 1/3 iotests: use -ccw on s390x for 040, 139, and 182

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

---
 tests/qemu-iotests/040 |  6 +++++-
 tests/qemu-iotests/139 | 12 ++++++++++--
 tests/qemu-iotests/182 | 13 +++++++++++--
 3 files changed, 26 insertions(+), 5 deletions(-)
tests/qemu-iotests/040 +5 -1
index 95b7510571..c284d08796 100755
@@ -82,7 +82,11 @@ class TestSingleDrive(ImageCommitTestCase):
         qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
         qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
         self.vm = iotests.VM().add_drive(test_img, "node-name=top,backing.node-name=mid,backing.backing.node-name=base", interface="none")
-        self.vm.add_device("virtio-scsi-pci")
+        if iotests.qemu_default_machine == 's390-ccw-virtio':
+            self.vm.add_device("virtio-scsi-ccw")
+        else:
+            self.vm.add_device("virtio-scsi-pci")
+
         self.vm.add_device("scsi-hd,id=scsi0,drive=drive0")
         self.vm.launch()
 
tests/qemu-iotests/139 +10 -2
index 50cf40fbd5..f8f02808a9 100644
@@ -25,13 +25,21 @@ import time
 
 base_img = os.path.join(iotests.test_dir, 'base.img')
 new_img = os.path.join(iotests.test_dir, 'new.img')
+if iotests.qemu_default_machine == 's390-ccw-virtio':
+    default_virtio_blk = 'virtio-blk-ccw'
+else:
+    default_virtio_blk = 'virtio-blk-pci'
 
 class TestBlockdevDel(iotests.QMPTestCase):
 
     def setUp(self):
         iotests.qemu_img('create', '-f', iotests.imgfmt, base_img, '1M')
         self.vm = iotests.VM()
-        self.vm.add_device("virtio-scsi-pci,id=virtio-scsi")
+        if iotests.qemu_default_machine == 's390-ccw-virtio':
+            self.vm.add_device("virtio-scsi-ccw,id=virtio-scsi")
+        else:
+            self.vm.add_device("virtio-scsi-pci,id=virtio-scsi")
+
         self.vm.launch()
 
     def tearDown(self):
@@ -87,7 +95,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
         self.checkBlockDriverState(node, expect_error)
 
     # Add a device model
-    def addDeviceModel(self, device, backend, driver = 'virtio-blk-pci'):
+    def addDeviceModel(self, device, backend, driver = default_virtio_blk):
         result = self.vm.qmp('device_add', id = device,
                              driver = driver, drive = backend)
         self.assert_qmp(result, 'return', {})
tests/qemu-iotests/182 +11 -2
index 7ecbb22604..2e078ceed8 100755
@@ -45,17 +45,26 @@ _supported_os Linux
 
 size=32M
 
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      virtioblk=virtio-blk-ccw
+      ;;
+  *)
+      virtioblk=virtio-blk-pci
+      ;;
+esac
+
 _make_test_img $size
 
 echo "Starting QEMU"
 _launch_qemu -drive file=$TEST_IMG,if=none,id=drive0,file.locking=on \
-    -device virtio-blk-pci,drive=drive0
+    -device $virtioblk,drive=drive0
 
 echo
 echo "Starting a second QEMU using the same image should fail"
 echo 'quit' | $QEMU -monitor stdio \
     -drive file=$TEST_IMG,if=none,id=drive0,file.locking=on \
-    -device virtio-blk-pci,drive=drive0 2>&1 | _filter_testdir 2>&1 |
+    -device $virtioblk,drive=drive0 2>&1 | _filter_testdir 2>&1 |
     _filter_qemu |
     sed -e '/falling back to POSIX file/d' \
         -e '/locks can be lost unexpectedly/d'
Signature · 1 line
2.13.5
Reviewed-by on v1 1/3
#2 lore raw

Re: [Qemu-devel] [PATCH 1/3] iotests: use -ccw on s390x for 040, 139, and 182

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 6 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

#3 lore raw

PATCH 2/3 iotests: use -ccw on s390x for 051

In reply to Cornelia Huck

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out
tests/qemu-iotests/051 +8 -1
index c8cfc764bc..f6ad0f4f0b 100755
@@ -103,7 +103,14 @@ echo
 echo === Device without drive ===
 echo
 
-run_qemu -device virtio-scsi-pci -device scsi-hd
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      run_qemu -device virtio-scsi-ccw -device scsi-hd
+      ;;
+  *)
+      run_qemu -device virtio-scsi-pci -device scsi-hd
+      ;;
+esac
 
 echo
 echo === Overriding backing file ===
tests/qemu-iotests/051.s390-ccw-virtio.out new file +434 -0

434 lines not on this page · raw message

Signature · 1 line
2.13.5
#4 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Quoted 24 lines · Cornelia Huck

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Provide an output file for s390x.

---
  tests/qemu-iotests/051                     |   9 +-
  tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
  2 files changed, 442 insertions(+), 1 deletion(-)

create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out

tests/qemu-iotests/051 +1 -1
index c8cfc764bc..f6ad0f4f0b 100755
@@ -103,7 +103,14 @@ echo
  echo === Device without drive ===
  echo
 
-run_qemu -device virtio-scsi-pci -device scsi-hd
+case "$QEMU_DEFAULT_MACHINE" in

tests/qemu-iotests/051 echo · line 116

Quoted 10 lines
tests/qemu-iotests/051 +7 -0
+  s390-ccw-virtio)
+      run_qemu -device virtio-scsi-ccw -device scsi-hd
+      ;;
+  *)
+      run_qemu -device virtio-scsi-pci -device scsi-hd
+      ;;
+esac
 
  echo
  echo === Overriding backing file ===

Regarding the new output file, I have a doubt that why there is no change on "check" script where the result check is located:
if diff -w "$reference" $tmp.out >/dev/null 2>&1
Thanks!

Quoted 5 lines
tests/qemu-iotests/051.s390-ccw-virtio.out new file +0 -0
new file mode 100644
index 0000000000..7555f0b73e

[...]

Signature · 2 lines
Regards
QingFeng Hao
author Reviewed-by Max Reitz Reviewed-by Michael Mueller Reviewed-by Sascha Silbe
#5 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Quoted 33 lines · On Wed, 6 Sep 2017 15:19:01 +0800 QingFeng Hao <[email protected]> wrote:
Quoted 32 lines · Cornelia Huck

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Provide an output file for s390x.

---
  tests/qemu-iotests/051                     |   9 +-
  tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
  2 files changed, 442 insertions(+), 1 deletion(-)

create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out

tests/qemu-iotests/051 +8 -1
index c8cfc764bc..f6ad0f4f0b 100755
@@ -103,7 +103,14 @@ echo
  echo === Device without drive ===
  echo
 
-run_qemu -device virtio-scsi-pci -device scsi-hd
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      run_qemu -device virtio-scsi-ccw -device scsi-hd
+      ;;
+  *)
+      run_qemu -device virtio-scsi-pci -device scsi-hd
+      ;;
+esac
 
Quoted 6 lines

tests/qemu-iotests/051 echo · line 116

Quoted 2 lines
tests/qemu-iotests/051 +0 -0
  echo
  echo === Overriding backing file ===  

Regarding the new output file, I have a doubt that why there is no change on "check"
script where the result check is located:
if diff -w "$reference" $tmp.out >/dev/null 2>&1

The right output reference should be picked as of

commit e166b4148208656635ea2fe39df8b1e875a34fb8

Author: Bo Tu <[email protected]>
Date:   Fri Jul 3 15:28:46 2015 +0800

qemu-iotests: qemu machine type support

This patch adds qemu machine type support to the io test suite.
Based on the qemu default machine type and alias of the default machine type the reference output file can now vary from the default to a machine specific output file if necessary. When using a machine specific reference file if the default machine has an alias then use the alias as the output file name otherwise use the default machine name as the output file name.

#6 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Quoted 36 lines · Cornelia Huck
Quoted 33 lines · On Wed, 6 Sep 2017 15:19:01 +0800 QingFeng Hao <[email protected]> wrote:
Quoted 32 lines · Cornelia Huck

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Provide an output file for s390x.

---
   tests/qemu-iotests/051                     |   9 +-
   tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
   2 files changed, 442 insertions(+), 1 deletion(-)

create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out

tests/qemu-iotests/051 +8 -1
index c8cfc764bc..f6ad0f4f0b 100755
@@ -103,7 +103,14 @@ echo
   echo === Device without drive ===
   echo
 
-run_qemu -device virtio-scsi-pci -device scsi-hd
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      run_qemu -device virtio-scsi-ccw -device scsi-hd
+      ;;
+  *)
+      run_qemu -device virtio-scsi-pci -device scsi-hd
+      ;;
+esac
 
Quoted 6 lines
Quoted 6 lines

tests/qemu-iotests/051 echo · line 116

Quoted 2 lines
tests/qemu-iotests/051 +0 -0
   echo
   echo === Overriding backing file ===

Regarding the new output file, I have a doubt that why there is no change on "check"
script where the result check is located:
if diff -w "$reference" $tmp.out >/dev/null 2>&1

Got it. It makes sense.

Quoted 21 lines

The right output reference should be picked as of

commit e166b4148208656635ea2fe39df8b1e875a34fb8

Author: Bo Tu <[email protected]>
Date:   Fri Jul 3 15:28:46 2015 +0800

qemu-iotests: qemu machine type support

This patch adds qemu machine type support to the io test suite.
Based on the qemu default machine type and alias of the default machine type the reference output file can now vary from the default to a machine specific output file if necessary. When using a machine specific reference file if the default machine has an alias then use the alias as the output file name otherwise use the default machine name as the output file name.

Signature · 2 lines
Regards
QingFeng Hao
#7 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

In reply to Cornelia Huck

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 5 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Quoted 8 lines

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out

It's already a pain to have two separate output files for 051, let's try to avoid adding a third one. Even more so since I think that the split between 051.out and 051.pc.out was already made for s390, so I'm not sure if anyone would actually still make use of the plain 051.out output if s390 got it's own one.

Quoted 7 lines
tests/qemu-iotests/051 +0 -0
index c8cfc764bc..f6ad0f4f0b 100755
@@ -103,7 +103,14 @@ echo
 echo === Device without drive ===
 echo

tests/qemu-iotests/051 echo · line 113

Quoted 10 lines
tests/qemu-iotests/051 +8 -1
 
-run_qemu -device virtio-scsi-pci -device scsi-hd
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      run_qemu -device virtio-scsi-ccw -device scsi-hd
+      ;;
+  *)
+      run_qemu -device virtio-scsi-pci -device scsi-hd
+      ;;
+esac

The only real difference between 051.out and 051.s390-ccw-virtio.out is in this one command line. So if we don't want to just skip this part of the test for non-pc like we already skip ther parts, we generally solve this kind of thing by just filtering out strings that differ between setups.

For example:

    case "$QEMU_DEFAULT_MACHINE" in
      s390-ccw-virtio)
          virtio_scsi=virtio-scsi-ccw
          ;;
      *)
          virtio_scsi=virtio-scsi-pci
          ;;
    esac

    run_qemu -device $virtio_scsi -device scsi-hd |
        sed -e "s/$virtio_scsi/VIRTIO-SCSI/"

Kevin

#8 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Quoted 15 lines · On Fri, 8 Sep 2017 13:04:25 +0200 Kevin Wolf <[email protected]> wrote:

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 5 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Quoted 8 lines

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out  
Quoted 5 lines

It's already a pain to have two separate output files for 051, let's try to avoid adding a third one. Even more so since I think that the split between 051.out and 051.pc.out was already made for s390, so I'm not sure if anyone would actually still make use of the plain 051.out output if s390 got it's own one.

Are there no non-pc and non-s390 machines for which this is run?

Quoted 9 lines
Quoted 9 lines
tests/qemu-iotests/051 +0 -0
index c8cfc764bc..f6ad0f4f0b 100755
@@ -103,7 +103,14 @@ echo
 echo === Device without drive ===
 echo
 
Quoted 13 lines

tests/qemu-iotests/051 echo · line 113

Quoted 9 lines
tests/qemu-iotests/051 +8 -1
-run_qemu -device virtio-scsi-pci -device scsi-hd
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      run_qemu -device virtio-scsi-ccw -device scsi-hd
+      ;;
+  *)
+      run_qemu -device virtio-scsi-pci -device scsi-hd
+      ;;
+esac  

The only real difference between 051.out and 051.s390-ccw-virtio.out is in this one command line. So if we don't want to just skip this part of the test for non-pc like we already skip ther parts,

I don't think there's a reason to skip this: The only difference is that we (currently) don't have a by-default usable virtio-pci implementation on s390 - but any virtio transport should do.

Another approach would be to drop the -pci postfix, but I don't want to introduce more usage of aliases.

Quoted 6 lines

we generally solve
this kind of thing by just filtering out strings that differ between setups.

For example:

Quoted 11 lines
    case "$QEMU_DEFAULT_MACHINE" in
      s390-ccw-virtio)
          virtio_scsi=virtio-scsi-ccw
          ;;
      *)
          virtio_scsi=virtio-scsi-pci
          ;;
    esac

    run_qemu -device $virtio_scsi -device scsi-hd |
        sed -e "s/$virtio_scsi/VIRTIO-SCSI/"

Yes, I can try this.

#9 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Am 08.09.2017 um 13:24 hat Cornelia Huck geschrieben:

Quoted 18 lines
Quoted 15 lines · On Fri, 8 Sep 2017 13:04:25 +0200 Kevin Wolf <[email protected]> wrote:

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 13 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out  
Quoted 7 lines
Quoted 5 lines

It's already a pain to have two separate output files for 051, let's try to avoid adding a third one. Even more so since I think that the split between 051.out and 051.pc.out was already made for s390, so I'm not sure if anyone would actually still make use of the plain 051.out output if s390 got it's own one.

Are there no non-pc and non-s390 machines for which this is run?

Who knows? But I'm not aware of anyone who is interested in something else and has contributed to the test cases until now.

Quoted 22 lines
Quoted 8 lines
Quoted 8 lines
tests/qemu-iotests/051 +0 -0
index c8cfc764bc..f6ad0f4f0b 100755
@@ -103,7 +103,14 @@ echo
 echo === Device without drive ===
 echo
 
Quoted 13 lines

tests/qemu-iotests/051 echo · line 113

Quoted 9 lines
tests/qemu-iotests/051 +8 -1
-run_qemu -device virtio-scsi-pci -device scsi-hd
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      run_qemu -device virtio-scsi-ccw -device scsi-hd
+      ;;
+  *)
+      run_qemu -device virtio-scsi-pci -device scsi-hd
+      ;;
+esac  

The only real difference between 051.out and 051.s390-ccw-virtio.out is in this one command line. So if we don't want to just skip this part of the test for non-pc like we already skip ther parts,

Quoted 3 lines

I don't think there's a reason to skip this: The only difference is that we (currently) don't have a by-default usable virtio-pci implementation on s390 - but any virtio transport should do.

Well, any SCSI controller should do, really. Or in fact, any block device that doesn't have removable media.

I agree that there's no real reason to skip the test for s390. On the other hand, testing it on s390 doesn't really contribute anything to the test coverage as long as the suite is run for PC, too (because there is nothing machine dependent in the tested code path), so not running it would be tolerable.

Quoted 2 lines

Another approach would be to drop the -pci postfix, but I don't want to introduce more usage of aliases.

Maybe that would indeed be the easiest way. As long as we don't intend to remove the alias from qemu, there's no reason not to use it in tests.

Quoted 15 lines
Quoted 15 lines

we generally solve
this kind of thing by just filtering out strings that differ between setups.

For example:

    case "$QEMU_DEFAULT_MACHINE" in
      s390-ccw-virtio)
          virtio_scsi=virtio-scsi-ccw
          ;;
      *)
          virtio_scsi=virtio-scsi-pci
          ;;
    esac
Quoted 4 lines
Quoted 2 lines

run_qemu -device $virtio_scsi -device scsi-hd |
sed -e "s/$virtio_scsi/VIRTIO-SCSI/"

Yes, I can try this.

Kevin

#10 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Quoted 25 lines · Kevin Wolf

Am 08.09.2017 um 13:24 hat Cornelia Huck geschrieben:

Quoted 24 lines
Quoted 20 lines · On Fri, 8 Sep 2017 13:04:25 +0200 Kevin Wolf <[email protected]> wrote:

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 5 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Quoted 8 lines

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out  

It's already a pain to have two separate output files for 051, let's try to avoid adding a third one. Even more so since I think that the split between 051.out and 051.pc.out was already made for s390, so I'm not sure if anyone would actually still make use of the plain 051.out output if s390 got it's own one.

Quoted 4 lines
Quoted 1 line

Are there no non-pc and non-s390 machines for which this is run?

Who knows? But I'm not aware of anyone who is interested in something else and has contributed to the test cases until now.

FWIW, as far as I know, Lukáš is running this test also on ppc64 in our weekly regression run. So it would be good to keep that working, please :-)

Quoted 5 lines
Quoted 2 lines

Another approach would be to drop the -pci postfix, but I don't want to introduce more usage of aliases.

Maybe that would indeed be the easiest way. As long as we don't intend to remove the alias from qemu, there's no reason not to use it in tests.

Maybe we should even use it in a couple of places on purpose - so we get some test coverage for them?

Thomas

#11 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Dne 8.9.2017 v 17:55 Thomas Huth napsal(a):

Quoted 26 lines · Kevin Wolf
Quoted 25 lines

Am 08.09.2017 um 13:24 hat Cornelia Huck geschrieben:

Quoted 24 lines
Quoted 20 lines · On Fri, 8 Sep 2017 13:04:25 +0200 Kevin Wolf <[email protected]> wrote:

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 5 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Quoted 8 lines

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out  

It's already a pain to have two separate output files for 051, let's try to avoid adding a third one. Even more so since I think that the split between 051.out and 051.pc.out was already made for s390, so I'm not sure if anyone would actually still make use of the plain 051.out output if s390 got it's own one.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Are there no non-pc and non-s390 machines for which this is run?

Who knows? But I'm not aware of anyone who is interested in something else and has contributed to the test cases until now.

FWIW, as far as I know, Lukáš is running this test also on ppc64 in our weekly regression run. So it would be good to keep that working, please :-)

Yes, works well on ppc64(le) and would be nice to keep it that way...

Lukáš

Quoted 11 lines
Quoted 5 lines
Quoted 2 lines

Another approach would be to drop the -pci postfix, but I don't want to introduce more usage of aliases.

Maybe that would indeed be the easiest way. As long as we don't intend to remove the alias from qemu, there's no reason not to use it in tests.

Maybe we should even use it in a couple of places on purpose - so we get some test coverage for them?

Thomas

#12 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

In reply to Thomas Huth
Quoted 34 lines · On Fri, 8 Sep 2017 17:55:29 +0200 Thomas Huth <[email protected]> wrote:
Quoted 25 lines · Kevin Wolf

Am 08.09.2017 um 13:24 hat Cornelia Huck geschrieben:

Quoted 24 lines
Quoted 20 lines · On Fri, 8 Sep 2017 13:04:25 +0200 Kevin Wolf <[email protected]> wrote:

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 5 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Quoted 8 lines

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out    

It's already a pain to have two separate output files for 051, let's try to avoid adding a third one. Even more so since I think that the split between 051.out and 051.pc.out was already made for s390, so I'm not sure if anyone would actually still make use of the plain 051.out output if s390 got it's own one.

Quoted 4 lines
Quoted 1 line

Are there no non-pc and non-s390 machines for which this is run?

Who knows? But I'm not aware of anyone who is interested in something else and has contributed to the test cases until now.

FWIW, as far as I know, Lukáš is running this test also on ppc64 in our weekly regression run. So it would be good to keep that working, please :-)

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

Another approach would be to drop the -pci postfix, but I don't want to introduce more usage of aliases.

Maybe that would indeed be the easiest way. As long as we don't intend to remove the alias from qemu, there's no reason not to use it in tests.

Maybe we should even use it in a couple of places on purpose - so we get some test coverage for them?

FWIW, using the the sed post-run filtering works well for 051, but it is driving me bonkers on 067 (or maybe I should just call it a day...)

We could use the sed approach for 051 and the alias approach on 067 - that way we would also test aliases :)

Any preferences by the iotest maintainers?

[I'd be happy if iotests worked again on s390x...]

#13 lore raw

Re: [Qemu-devel] [PATCH 2/3] iotests: use -ccw on s390x for 051

Am 12.09.2017 um 18:05 hat Cornelia Huck geschrieben:

Quoted 46 lines
Quoted 34 lines · On Fri, 8 Sep 2017 17:55:29 +0200 Thomas Huth <[email protected]> wrote:
Quoted 25 lines · Kevin Wolf

Am 08.09.2017 um 13:24 hat Cornelia Huck geschrieben:

Quoted 24 lines
Quoted 20 lines · On Fri, 8 Sep 2017 13:04:25 +0200 Kevin Wolf <[email protected]> wrote:

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 5 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Quoted 8 lines

Provide an output file for s390x.

---
 tests/qemu-iotests/051                     |   9 +-
 tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out    

It's already a pain to have two separate output files for 051, let's try to avoid adding a third one. Even more so since I think that the split between 051.out and 051.pc.out was already made for s390, so I'm not sure if anyone would actually still make use of the plain 051.out output if s390 got it's own one.

Quoted 4 lines
Quoted 1 line

Are there no non-pc and non-s390 machines for which this is run?

Who knows? But I'm not aware of anyone who is interested in something else and has contributed to the test cases until now.

FWIW, as far as I know, Lukáš is running this test also on ppc64 in our weekly regression run. So it would be good to keep that working, please :-)

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

Another approach would be to drop the -pci postfix, but I don't want to introduce more usage of aliases.

Maybe that would indeed be the easiest way. As long as we don't intend to remove the alias from qemu, there's no reason not to use it in tests.

Maybe we should even use it in a couple of places on purpose - so we get some test coverage for them?

Quoted 7 lines

FWIW, using the the sed post-run filtering works well for 051, but it is driving me bonkers on 067 (or maybe I should just call it a day...)

We could use the sed approach for 051 and the alias approach on 067 - that way we would also test aliases :)

Any preferences by the iotest maintainers?

Sure, that works for me. I'm happy with anything that works and doesn't split the reference output in two versions. (Not splitting is also in your own interest; if anything bitrots, it's the non-pc version.)

Kevin

#14 lore raw

PATCH 3/3 iotests: use -ccw on s390x for 067

In reply to Cornelia Huck

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Provide an output file for s390x.

---
 tests/qemu-iotests/067                     |  11 +-
 tests/qemu-iotests/067.s390-ccw-virtio.out | 458 +++++++++++++++++++++++++++++
 2 files changed, 468 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/067.s390-ccw-virtio.out
tests/qemu-iotests/067 +10 -1
index 5d4ca4bc61..c5db04fb50 100755
@@ -37,6 +37,15 @@ _supported_os Linux
 # Because anything other than 16 would change the output of query-block
 _unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
 
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+      hba=virtio-scsi-ccw
+      ;;
+  *)
+      hba=virtio-scsi-pci
+      ;;
+esac
+
 function do_run_qemu()
 {
     echo Testing: "$@"
@@ -141,7 +150,7 @@ echo
 echo === Empty drive with -device and device_del ===
 echo
 
-run_qemu -device virtio-scsi-pci -device scsi-cd,id=cd0 <<EOF
+run_qemu -device $hba -device scsi-cd,id=cd0 <<EOF
 { "execute": "qmp_capabilities" }
 { "execute": "query-block" }
 { "execute": "device_del", "arguments": { "id": "cd0" } }
tests/qemu-iotests/067.s390-ccw-virtio.out new file +458 -0

458 lines not on this page · raw message

Signature · 1 line
2.13.5
#15 lore raw

Re: [Qemu-devel] [PATCH 3/3] iotests: use -ccw on s390x for 067

Am 05.09.2017 um 17:16 hat Cornelia Huck geschrieben:

Quoted 8 lines

The default cpu model on s390x does not provide zPCI, which is not yet wired up on tcg. Moreover, virtio-ccw is the standard on s390x, so use the -ccw instead of the -pci versions of virtio devices on s390x.

Provide an output file for s390x.

The same argument as for patch 2 applies here, except that 067 has a single output file so far, so keeping it this way is even more important. You can again just filter the output.

Kevin

#16 lore raw
In reply to Cornelia Huck
Quoted 11 lines · Cornelia Huck

Recent changes in s390x made pci support dependant on the zpci cpu feature, which is not provided on all models (and not on by default). This means we cannot instatiate pci devices on a standard qemu invocation for s390x. Moreover, the zpci instructions are not even wired up for tcg yet, so actually doing anything with those pci devices is bound to fail on tcg.

Let's follow the existing example in 068 and switch to the (default) virtio-ccw transport on s390x. The changes for 051 and 067 are split out as they require adding an output file for s390x (the actual command lines are part of the output).

We also found this error and YiMin suggested to change the code in ccw_init as below:

if (pci_available) {
    DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
    ...
}
We tested it and it can make the 5 cases passed.
How do you think this? :-)
Thanks!

Quoted 17 lines

Cornelia Huck (3):

   iotests: use -ccw on s390x for 040, 139, and 182
   iotests: use -ccw on s390x for 051
   iotests: use -ccw on s390x for 067
  tests/qemu-iotests/040                     |   6 +-
  tests/qemu-iotests/051                     |   9 +-
  tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++
  tests/qemu-iotests/067                     |  11 +-
  tests/qemu-iotests/067.s390-ccw-virtio.out | 458 +++++++++++++++++++++++++++++
  tests/qemu-iotests/139                     |  12 +-
  tests/qemu-iotests/182                     |  13 +-
  7 files changed, 936 insertions(+), 7 deletions(-)

create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out create mode 100644 tests/qemu-iotests/067.s390-ccw-virtio.out

Signature · 2 lines
Regards
QingFeng Hao
#17 lore raw
Quoted 15 lines · On Wed, 6 Sep 2017 14:57:48 +0800 QingFeng Hao <[email protected]> wrote:
Quoted 11 lines · Cornelia Huck

Recent changes in s390x made pci support dependant on the zpci cpu feature, which is not provided on all models (and not on by default). This means we cannot instatiate pci devices on a standard qemu invocation for s390x. Moreover, the zpci instructions are not even wired up for tcg yet, so actually doing anything with those pci devices is bound to fail on tcg.

Let's follow the existing example in 068 and switch to the (default) virtio-ccw transport on s390x. The changes for 051 and 067 are split out as they require adding an output file for s390x (the actual command lines are part of the output).

We also found this error and YiMin suggested to change the code in ccw_init as below:

Quoted 6 lines

if (pci_available) {
    DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
    ...
}
We tested it and it can make the 5 cases passed.
How do you think this? :-)

I can take a look at this.

But I think we want to prefer ccw for s390x in the tests in any case, for the reasons stated above.

Quoted 20 lines

Thanks!

Quoted 17 lines

Cornelia Huck (3):

   iotests: use -ccw on s390x for 040, 139, and 182
   iotests: use -ccw on s390x for 051
   iotests: use -ccw on s390x for 067
  tests/qemu-iotests/040                     |   6 +-
  tests/qemu-iotests/051                     |   9 +-
  tests/qemu-iotests/051.s390-ccw-virtio.out | 434 +++++++++++++++++++++++++++
  tests/qemu-iotests/067                     |  11 +-
  tests/qemu-iotests/067.s390-ccw-virtio.out | 458 +++++++++++++++++++++++++++++
  tests/qemu-iotests/139                     |  12 +-
  tests/qemu-iotests/182                     |  13 +-
  7 files changed, 936 insertions(+), 7 deletions(-)

create mode 100644 tests/qemu-iotests/051.s390-ccw-virtio.out create mode 100644 tests/qemu-iotests/067.s390-ccw-virtio.out

#18 lore raw
In reply to QingFeng Hao
Quoted 12 lines · On Wed, 6 Sep 2017 14:57:48 +0800 QingFeng Hao <[email protected]> wrote:
Quoted 11 lines · Cornelia Huck

Recent changes in s390x made pci support dependant on the zpci cpu feature, which is not provided on all models (and not on by default). This means we cannot instatiate pci devices on a standard qemu invocation for s390x. Moreover, the zpci instructions are not even wired up for tcg yet, so actually doing anything with those pci devices is bound to fail on tcg.

Let's follow the existing example in 068 and switch to the (default) virtio-ccw transport on s390x. The changes for 051 and 067 are split out as they require adding an output file for s390x (the actual command lines are part of the output).

Quoted 8 lines

We also found this error and YiMin suggested to change the code in ccw_init as below:

if (pci_available) {
    DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
    ...
}
We tested it and it can make the 5 cases passed.

OK, looked at this. This won't work: pci_available means "this qemu has pci support built in". _Working_ zpci, however, depends on the presence of the zpci feature bit: You'll have a host bridge and can define devices that have absolutely no chance of working, since all pci instruction will return errors. You will be in a similar situation under kvm as under tcg: you can specify virtio-pci devices on the command line, but they can't work.

This probably makes the 5 cases pass as they only rely on the ability to create the device, not to do anything with them.

So, I still think the right thing to do is to switch to ccw in the tests (and to wire up pci in tcg, but that's an orthogonal issue).

#19 lore raw
Quoted 23 lines · Cornelia Huck
Quoted 12 lines · On Wed, 6 Sep 2017 14:57:48 +0800 QingFeng Hao <[email protected]> wrote:
Quoted 11 lines · Cornelia Huck

Recent changes in s390x made pci support dependant on the zpci cpu feature, which is not provided on all models (and not on by default). This means we cannot instatiate pci devices on a standard qemu invocation for s390x. Moreover, the zpci instructions are not even wired up for tcg yet, so actually doing anything with those pci devices is bound to fail on tcg.

Let's follow the existing example in 068 and switch to the (default) virtio-ccw transport on s390x. The changes for 051 and 067 are split out as they require adding an output file for s390x (the actual command lines are part of the output).

Quoted 8 lines

We also found this error and YiMin suggested to change the code in ccw_init as below:

if (pci_available) {
    DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
    ...
}
We tested it and it can make the 5 cases passed.

Quoted 7 lines

OK, looked at this. This won't work: pci_available means "this qemu has pci support built in". _Working_ zpci, however, depends on the presence of the zpci feature bit: You'll have a host bridge and can define devices that have absolutely no chance of working, since all pci instruction will return errors. You will be in a similar situation under kvm as under tcg: you can specify virtio-pci devices on the command line, but they can't work.

Oh. Yes, that makes sense. Actually the first way we thought about was change the code not change the testcases. Thanks for your work.

Quoted 6 lines

This probably makes the 5 cases pass as they only rely on the ability to create the device, not to do anything with them.

So, I still think the right thing to do is to switch to ccw in the tests (and to wire up pci in tcg, but that's an orthogonal issue).

Agree.

Quoted 2 lines