[PATCH 0/5] qemu-iotests: test savevm/loadvm iothread (and make it work!)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
26 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/5] qemu-iotests: test savevm/loadvm iothread (and make it work!)

Stefan Hajnoczi-7
This series extends qemu-iotests 068 to also run with iothread enabled.  Doing
so was harder than expected because:

1. ioeventfd is disabled without -M accel=kvm even though it should work
2. loadvm still has an iothread bug

Instead of adding a ./check -iothread option I decided to always run the test.
Kevin recently recommended this approach; the advantage is that iothread *will*
always be tested iothread mode whereas people won't run ./check -iothread.

Stefan Hajnoczi (5):
  virtio-pci: use ioeventfd even when KVM is disabled
  migration: hold AioContext lock for loadvm qemu_fclose()
  qemu-iotests: 068: extract _qemu() function
  qemu-iotests: 068: use -drive/-device instead of -hda
  qemu-iotests: 068: test iothread mode

 hw/virtio/virtio-pci.c     |  2 +-
 migration/savevm.c         |  2 +-
 tests/qemu-iotests/068     | 37 +++++++++++++++++++++++++------------
 tests/qemu-iotests/068.out | 11 ++++++++++-
 4 files changed, 37 insertions(+), 15 deletions(-)

--
2.9.4


Reply | Threaded
Open this post in threaded view
|

[PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Stefan Hajnoczi-7
Old kvm.ko versions only supported a tiny number of ioeventfds so
virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.

Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-blk-pci,iothread=iothread0 work even
when KVM is disabled.

I have tested that virtio-blk-pci works under TCG both with and without
iothread.

Cc: Michael S. Tsirkin <[hidden email]>
Signed-off-by: Stefan Hajnoczi <[hidden email]>
---
 hw/virtio/virtio-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index f9b7244..9f55476 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1740,7 +1740,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
     bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
                      !pci_bus_is_root(pci_dev->bus);
 
-    if (!kvm_has_many_ioeventfds()) {
+    if (kvm_enabled() && !kvm_has_many_ioeventfds()) {
         proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
     }
 
--
2.9.4


Reply | Threaded
Open this post in threaded view
|

[PATCH 2/5] migration: hold AioContext lock for loadvm qemu_fclose()

Stefan Hajnoczi-7
In reply to this post by Stefan Hajnoczi-7
migration_incoming_state_destroy() uses qemu_fclose() on the vmstate
file.  Make sure to call it inside an AioContext acquire/release region.

This fixes an 'qemu: qemu_mutex_unlock: Operation not permitted' abort
in loadvm.

This patch closes the vmstate file before ending the drained region.
Previously we closed the vmstate file after ending the drained region.
The order does not matter.

Signed-off-by: Stefan Hajnoczi <[hidden email]>
---
 migration/savevm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index ff126a1..943a43c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2292,11 +2292,11 @@ int load_snapshot(const char *name, Error **errp)
 
     aio_context_acquire(aio_context);
     ret = qemu_loadvm_state(f);
+    migration_incoming_state_destroy();
     aio_context_release(aio_context);
 
     bdrv_drain_all_end();
 
-    migration_incoming_state_destroy();
     if (ret < 0) {
         error_setg(errp, "Error %d while loading VM state", ret);
         return ret;
--
2.9.4


Reply | Threaded
Open this post in threaded view
|

[PATCH 3/5] qemu-iotests: 068: extract _qemu() function

Stefan Hajnoczi-7
In reply to this post by Stefan Hajnoczi-7
Avoid duplicating the QEMU command-line.

Signed-off-by: Stefan Hajnoczi <[hidden email]>
---
 tests/qemu-iotests/068 | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 9c1687d..653e23c 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -59,14 +59,17 @@ case "$QEMU_DEFAULT_MACHINE" in
       ;;
 esac
 
-# Give qemu some time to boot before saving the VM state
-bash -c 'sleep 1; echo -e "savevm 0\nquit"' |\
+_qemu()
+{
     $QEMU $platform_parm -nographic -monitor stdio -serial none -hda "$TEST_IMG" |\
+          "$@" |\
     _filter_qemu | _filter_hmp
+}
+
+# Give qemu some time to boot before saving the VM state
+bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
 # Now try to continue from that VM state (this should just work)
-echo quit |\
-    $QEMU $platform_parm -nographic -monitor stdio -serial none -hda "$TEST_IMG" -loadvm 0 |\
-    _filter_qemu | _filter_hmp
+echo quit | _qemu -loadvm 0
 
 # success, all done
 echo "*** done"
--
2.9.4


Reply | Threaded
Open this post in threaded view
|

[PATCH 4/5] qemu-iotests: 068: use -drive/-device instead of -hda

Stefan Hajnoczi-7
In reply to this post by Stefan Hajnoczi-7
The legacy -hda option does not support -drive/-device parameters.  They
will be required by the next patch that extends this test case.

Signed-off-by: Stefan Hajnoczi <[hidden email]>
---
 tests/qemu-iotests/068 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 653e23c..7292643 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -53,15 +53,20 @@ _make_test_img $IMG_SIZE
 case "$QEMU_DEFAULT_MACHINE" in
   s390-ccw-virtio)
       platform_parm="-no-shutdown"
+      hba=virtio-scsi-ccw
       ;;
   *)
       platform_parm=""
+      hba=virtio-scsi-pci
       ;;
 esac
 
 _qemu()
 {
-    $QEMU $platform_parm -nographic -monitor stdio -serial none -hda "$TEST_IMG" |\
+    $QEMU $platform_parm -nographic -monitor stdio -serial none \
+          -drive if=none,id=drive0,file="$TEST_IMG",format="$IMGFMT" \
+          -device $hba,id=hba0 \
+          -device scsi-hd,drive=drive0 \
           "$@" |\
     _filter_qemu | _filter_hmp
 }
--
2.9.4


Reply | Threaded
Open this post in threaded view
|

[PATCH 5/5] qemu-iotests: 068: test iothread mode

Stefan Hajnoczi-7
In reply to this post by Stefan Hajnoczi-7
Perform the savevm/loadvm test with both iothread on and off.  This
covers the recently found savevm/loadvm hang when iothread is enabled.

Signed-off-by: Stefan Hajnoczi <[hidden email]>
---
 tests/qemu-iotests/068     | 23 ++++++++++++++---------
 tests/qemu-iotests/068.out | 11 ++++++++++-
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 7292643..3801b65 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -45,11 +45,6 @@ _supported_os Linux
 IMGOPTS="compat=1.1"
 IMG_SIZE=128K
 
-echo
-echo "=== Saving and reloading a VM state to/from a qcow2 image ==="
-echo
-_make_test_img $IMG_SIZE
-
 case "$QEMU_DEFAULT_MACHINE" in
   s390-ccw-virtio)
       platform_parm="-no-shutdown"
@@ -71,10 +66,20 @@ _qemu()
     _filter_qemu | _filter_hmp
 }
 
-# Give qemu some time to boot before saving the VM state
-bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
-# Now try to continue from that VM state (this should just work)
-echo quit | _qemu -loadvm 0
+for extra_args in \
+    "" \
+    "-object iothread,id=iothread0 -set device.hba0.iothread=iothread0"; do
+    echo
+    echo "=== Saving and reloading a VM state to/from a qcow2 image ($extra_args) ==="
+    echo
+
+    _make_test_img $IMG_SIZE
+
+    # Give qemu some time to boot before saving the VM state
+    bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu $extra_args
+    # Now try to continue from that VM state (this should just work)
+    echo quit | _qemu $extra_args -loadvm 0
+done
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/068.out b/tests/qemu-iotests/068.out
index 0fa5340..aa063cf 100644
--- a/tests/qemu-iotests/068.out
+++ b/tests/qemu-iotests/068.out
@@ -1,6 +1,15 @@
 QA output created by 068
 
-=== Saving and reloading a VM state to/from a qcow2 image ===
+=== Saving and reloading a VM state to/from a qcow2 image () ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm 0
+(qemu) quit
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) quit
+
+=== Saving and reloading a VM state to/from a qcow2 image (-object iothread,id=iothread0 -set device.hba0.iothread=iothread0) ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
 QEMU X.Y.Z monitor - type 'help' for more information
--
2.9.4


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/5] qemu-iotests: test savevm/loadvm iothread (and make it work!)

Stefan Hajnoczi
In reply to this post by Stefan Hajnoczi-7
On Thu, Jun 15, 2017 at 5:38 PM, Stefan Hajnoczi <[hidden email]> wrote:
> This series extends qemu-iotests 068 to also run with iothread enabled.

I forgot to mention: this series is based on Kevin's 'block' branch:

  git://repo.or.cz/qemu/kevin.git block

This is necessary in order to get the "[PATCH v3 0/4] block: fix
'savevm' hang with -object iothread" patches that fix savevm.

Stefan

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Michael S. Tsirkin-4
In reply to this post by Stefan Hajnoczi-7
On Thu, Jun 15, 2017 at 05:38:09PM +0100, Stefan Hajnoczi wrote:

> Old kvm.ko versions only supported a tiny number of ioeventfds so
> virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
>
> Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
>
> This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> when KVM is disabled.
>
> I have tested that virtio-blk-pci works under TCG both with and without
> iothread.
>
> Cc: Michael S. Tsirkin <[hidden email]>
> Signed-off-by: Stefan Hajnoczi <[hidden email]>

Don't we need to check we are on a host that supports eventfd?

> ---
>  hw/virtio/virtio-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index f9b7244..9f55476 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1740,7 +1740,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
>      bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
>                       !pci_bus_is_root(pci_dev->bus);
>  
> -    if (!kvm_has_many_ioeventfds()) {
> +    if (kvm_enabled() && !kvm_has_many_ioeventfds()) {
>          proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
>      }
>  
> --
> 2.9.4

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Stefan Hajnoczi
On Fri, Jun 16, 2017 at 06:26:01AM +0300, Michael S. Tsirkin wrote:

> On Thu, Jun 15, 2017 at 05:38:09PM +0100, Stefan Hajnoczi wrote:
> > Old kvm.ko versions only supported a tiny number of ioeventfds so
> > virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
> >
> > Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> > always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> > ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> > qtest or TCG mode.
> >
> > This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> > when KVM is disabled.
> >
> > I have tested that virtio-blk-pci works under TCG both with and without
> > iothread.
> >
> > Cc: Michael S. Tsirkin <[hidden email]>
> > Signed-off-by: Stefan Hajnoczi <[hidden email]>
>
> Don't we need to check we are on a host that supports eventfd?
That is not necessary because the ioeventfd memory API is based on
EventNotifier instead of raw eventfds.

EventNotifier falls back to pipes on POSIX platforms and uses native
Event objects on Windows.

signature.asc (465 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Michael S. Tsirkin-4
In reply to this post by Stefan Hajnoczi-7
On Thu, Jun 15, 2017 at 05:38:09PM +0100, Stefan Hajnoczi wrote:

> Old kvm.ko versions only supported a tiny number of ioeventfds so
> virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
>
> Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
>
> This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> when KVM is disabled.
>
> I have tested that virtio-blk-pci works under TCG both with and without
> iothread.
>
> Cc: Michael S. Tsirkin <[hidden email]>
> Signed-off-by: Stefan Hajnoczi <[hidden email]>


Reviewed-by: Michael S. Tsirkin <[hidden email]>

Do you want to merge this with the rest of the patchset,
or should I?

> ---
>  hw/virtio/virtio-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index f9b7244..9f55476 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1740,7 +1740,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
>      bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
>                       !pci_bus_is_root(pci_dev->bus);
>  
> -    if (!kvm_has_many_ioeventfds()) {
> +    if (kvm_enabled() && !kvm_has_many_ioeventfds()) {
>          proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
>      }
>  
> --
> 2.9.4

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Stefan Hajnoczi-7
On Fri, Jun 16, 2017 at 05:12:18PM +0300, Michael S. Tsirkin wrote:

> On Thu, Jun 15, 2017 at 05:38:09PM +0100, Stefan Hajnoczi wrote:
> > Old kvm.ko versions only supported a tiny number of ioeventfds so
> > virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
> >
> > Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> > always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> > ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> > qtest or TCG mode.
> >
> > This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> > when KVM is disabled.
> >
> > I have tested that virtio-blk-pci works under TCG both with and without
> > iothread.
> >
> > Cc: Michael S. Tsirkin <[hidden email]>
> > Signed-off-by: Stefan Hajnoczi <[hidden email]>
>
>
> Reviewed-by: Michael S. Tsirkin <[hidden email]>
>
> Do you want to merge this with the rest of the patchset,
> or should I?
It can go through Kevin's tree as part of this patch series.

Thanks,
Stefan

signature.asc (465 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/5] qemu-iotests: test savevm/loadvm iothread (and make it work!)

Pavel Butsykin-2
In reply to this post by Stefan Hajnoczi-7
On 15.06.2017 19:38, Stefan Hajnoczi wrote:
> This series extends qemu-iotests 068 to also run with iothread enabled.  Doing
> so was harder than expected because:
>
> 1. ioeventfd is disabled without -M accel=kvm even though it should work
> 2. loadvm still has an iothread bug
>
> Instead of adding a ./check -iothread option I decided to always run the test.
> Kevin recently recommended this approach; the advantage is that iothread *will*
> always be tested iothread mode whereas people won't run ./check -iothread.

Why not just add -iothread option in check-block.sh? We can do an
additional run with -iothread, as it is done for different block
formats.

Because all the test cases already exist, we can just reuse them.

> Stefan Hajnoczi (5):
>    virtio-pci: use ioeventfd even when KVM is disabled
>    migration: hold AioContext lock for loadvm qemu_fclose()
>    qemu-iotests: 068: extract _qemu() function
>    qemu-iotests: 068: use -drive/-device instead of -hda
>    qemu-iotests: 068: test iothread mode
>
>   hw/virtio/virtio-pci.c     |  2 +-
>   migration/savevm.c         |  2 +-
>   tests/qemu-iotests/068     | 37 +++++++++++++++++++++++++------------
>   tests/qemu-iotests/068.out | 11 ++++++++++-
>   4 files changed, 37 insertions(+), 15 deletions(-)
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/5] qemu-iotests: 068: extract _qemu() function

Kevin Wolf-5
In reply to this post by Stefan Hajnoczi-7
Am 15.06.2017 um 18:38 hat Stefan Hajnoczi geschrieben:

> Avoid duplicating the QEMU command-line.
>
> Signed-off-by: Stefan Hajnoczi <[hidden email]>
> ---
>  tests/qemu-iotests/068 | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
> index 9c1687d..653e23c 100755
> --- a/tests/qemu-iotests/068
> +++ b/tests/qemu-iotests/068
> @@ -59,14 +59,17 @@ case "$QEMU_DEFAULT_MACHINE" in
>        ;;
>  esac
>  
> -# Give qemu some time to boot before saving the VM state
> -bash -c 'sleep 1; echo -e "savevm 0\nquit"' |\
> +_qemu()
> +{
>      $QEMU $platform_parm -nographic -monitor stdio -serial none -hda "$TEST_IMG" |\

This line shouldn't end with a pipe now. The bug goes away with the next
patch anyway, so I can just remove that one character while applying the
series.

> +          "$@" |\
>      _filter_qemu | _filter_hmp
> +}

Kevin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/5] qemu-iotests: test savevm/loadvm iothread (and make it work!)

Kevin Wolf-5
In reply to this post by Stefan Hajnoczi-7
Am 15.06.2017 um 18:38 hat Stefan Hajnoczi geschrieben:
> This series extends qemu-iotests 068 to also run with iothread enabled.  Doing
> so was harder than expected because:
>
> 1. ioeventfd is disabled without -M accel=kvm even though it should work
> 2. loadvm still has an iothread bug
>
> Instead of adding a ./check -iothread option I decided to always run the test.
> Kevin recently recommended this approach; the advantage is that iothread *will*
> always be tested iothread mode whereas people won't run ./check -iothread.

Thanks, fixed the intermediate state after patch 3 (without changing the
final state after applying all patches) and applied to the block branch.

Kevin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Fam Zheng-2
In reply to this post by Stefan Hajnoczi-7
On Thu, 06/15 17:38, Stefan Hajnoczi wrote:

> Old kvm.ko versions only supported a tiny number of ioeventfds so
> virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
>
> Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
>
> This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> when KVM is disabled.
>
> I have tested that virtio-blk-pci works under TCG both with and without
> iothread.
>
> Cc: Michael S. Tsirkin <[hidden email]>
> Signed-off-by: Stefan Hajnoczi <[hidden email]>

This one was dropped out from Kevin's pull request but the iotest case update on
068 which depends on it is merged. Now the test fails for me:

068 2s ... - output mismatch (see 068.out.bad)
--- /stor/work/qemu/tests/qemu-iotests/068.out 2017-06-27 16:22:55.003815188 +0800
+++ 068.out.bad 2017-06-27 16:41:37.903626275 +0800
@@ -12,9 +12,8 @@
 === Saving and reloading a VM state to/from a qcow2 image (-object iothread,id=iothread0 -set device.hba0.iothread=iothread0) ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
+qemu-system-x86_64: -device virtio-scsi-pci,id=hba0: ioeventfd is required for iothread
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) savevm 0
-(qemu) quit
+(qemu) qemu-system-x86_64: -device virtio-scsi-pci,id=hba0: ioeventfd is required for iothread
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) quit
-*** done
+(qemu) *** done
Failures: 068
Failed 1 of 1 tests

Fam

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Kevin Wolf-5
Am 27.06.2017 um 10:43 hat Fam Zheng geschrieben:

> On Thu, 06/15 17:38, Stefan Hajnoczi wrote:
> > Old kvm.ko versions only supported a tiny number of ioeventfds so
> > virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
> >
> > Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> > always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> > ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> > qtest or TCG mode.
> >
> > This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> > when KVM is disabled.
> >
> > I have tested that virtio-blk-pci works under TCG both with and without
> > iothread.
> >
> > Cc: Michael S. Tsirkin <[hidden email]>
> > Signed-off-by: Stefan Hajnoczi <[hidden email]>
>
> This one was dropped out from Kevin's pull request but the iotest case
> update on 068 which depends on it is merged. Now the test fails for
> me:

Whoops, sorry about that. Anyway, I think if we can, the way to fix it
is to find out why this patch is failing qtest, and merge a fixed v2,
rather than reverting the test cases.

Stefan, can you reproduce the failure?

Kevin

> 068 2s ... - output mismatch (see 068.out.bad)
> --- /stor/work/qemu/tests/qemu-iotests/068.out 2017-06-27 16:22:55.003815188 +0800
> +++ 068.out.bad 2017-06-27 16:41:37.903626275 +0800
> @@ -12,9 +12,8 @@
>  === Saving and reloading a VM state to/from a qcow2 image (-object iothread,id=iothread0 -set device.hba0.iothread=iothread0) ===
>  
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
> +qemu-system-x86_64: -device virtio-scsi-pci,id=hba0: ioeventfd is required for iothread
>  QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) savevm 0
> -(qemu) quit
> +(qemu) qemu-system-x86_64: -device virtio-scsi-pci,id=hba0: ioeventfd is required for iothread
>  QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) quit
> -*** done
> +(qemu) *** done
> Failures: 068
> Failed 1 of 1 tests
>
> Fam

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/5] qemu-iotests: 068: extract _qemu() function

Eric Blake
In reply to this post by Stefan Hajnoczi-7
On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
> Avoid duplicating the QEMU command-line.
>
> Signed-off-by: Stefan Hajnoczi <[hidden email]>
> ---
>  tests/qemu-iotests/068 | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>

> +# Give qemu some time to boot before saving the VM state
> +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu

Are we sure that 'bash' on PATH is the same as the /bin/bash running the
script?

Also, while bash has more deterministic behavior for 'echo -e' (it is
non-portable if you are porting a script to other shells), it is still
possible to set bash to a mode where it does not work (see xhopt
xpg_echo).  I'd much prefer you use 'printf' instead of 'set -e'.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/5] qemu-iotests: 068: extract _qemu() function

Eric Blake
On 06/27/2017 06:40 AM, Eric Blake wrote:

> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
>> Avoid duplicating the QEMU command-line.
>>
>> Signed-off-by: Stefan Hajnoczi <[hidden email]>
>> ---
>>  tests/qemu-iotests/068 | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>
>> +# Give qemu some time to boot before saving the VM state
>> +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
>
> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
> script?
>
> Also, while bash has more deterministic behavior for 'echo -e' (it is
> non-portable if you are porting a script to other shells), it is still
> possible to set bash to a mode where it does not work (see xhopt
shopt (I can't type first thing in the morning)

> xpg_echo).  I'd much prefer you use 'printf' instead of 'set -e'.
>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/5] qemu-iotests: 068: extract _qemu() function

Stefan Hajnoczi-7
In reply to this post by Eric Blake
On Tue, Jun 27, 2017 at 06:40:04AM -0500, Eric Blake wrote:

> On 06/15/2017 11:38 AM, Stefan Hajnoczi wrote:
> > Avoid duplicating the QEMU command-line.
> >
> > Signed-off-by: Stefan Hajnoczi <[hidden email]>
> > ---
> >  tests/qemu-iotests/068 | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
>
> > +# Give qemu some time to boot before saving the VM state
> > +bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu
>
> Are we sure that 'bash' on PATH is the same as the /bin/bash running the
> script?
>
> Also, while bash has more deterministic behavior for 'echo -e' (it is
> non-portable if you are porting a script to other shells), it is still
> possible to set bash to a mode where it does not work (see xhopt
> xpg_echo).  I'd much prefer you use 'printf' instead of 'set -e'.
Do you want to send a separate patch?

The purpose of this patch was just to move this code.

Stefan

signature.asc (465 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/5] virtio-pci: use ioeventfd even when KVM is disabled

Stefan Hajnoczi-7
In reply to this post by Kevin Wolf-5
On Tue, Jun 27, 2017 at 01:07:35PM +0200, Kevin Wolf wrote:

> Am 27.06.2017 um 10:43 hat Fam Zheng geschrieben:
> > On Thu, 06/15 17:38, Stefan Hajnoczi wrote:
> > > Old kvm.ko versions only supported a tiny number of ioeventfds so
> > > virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.
> > >
> > > Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
> > > always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> > > ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> > > qtest or TCG mode.
> > >
> > > This patch makes -device virtio-blk-pci,iothread=iothread0 work even
> > > when KVM is disabled.
> > >
> > > I have tested that virtio-blk-pci works under TCG both with and without
> > > iothread.
> > >
> > > Cc: Michael S. Tsirkin <[hidden email]>
> > > Signed-off-by: Stefan Hajnoczi <[hidden email]>
> >
> > This one was dropped out from Kevin's pull request but the iotest case
> > update on 068 which depends on it is merged. Now the test fails for
> > me:
>
> Whoops, sorry about that. Anyway, I think if we can, the way to fix it
> is to find out why this patch is failing qtest, and merge a fixed v2,
> rather than reverting the test cases.
>
> Stefan, can you reproduce the failure?
I will merge this patch via my tree and send a pull request today.

The purpose of this patch is to make -device virtio-blk-pci,iothread=ID
work under TCG/qtest.  This is necessary because qemu-iotests isn't
allowed to depend on KVM.

signature.asc (465 bytes) Download Attachment
12