[Qemu-devel] [PATCH v3 0/6] tests: Add device unplug tests

[Qemu-devel] [PATCH v3 0/6] tests: Add device unplug tests

[David Hildenbrand]

This is a set of tests to test basic device unplugging functionality for
- some PCI implementations
- CCW devices on s390x
- spapr memory and cpu core devices

I plaed with ACPI CPU unplug but getting that to run with qtest is more
involved. (remove devices on reset, trick cpu-hotplug-legacy property,
somehow create cpu hotplug state objects ...). Well we no have at least
one unplug test for DIMMs and one for CPUs.

v2 -> v3:
- Use a smaller DIMM for spapr memory unplug request test
- Combine both spapr test registrations in a single if statement
- Use "-" instead of "_" in test names
- Add a huge load of r-b/a-b :) hpe I didn't forget some.

v1 -> v2:
- Tests for CCW, spapr memory and spapr cpu core unplug added
- Fix a deadlock when unplugging dummy CPUs
- Style fixes / simplifications (Thomas)
- Don't use the global_qtest variable (Thomas)
- Fix double-free (Greg)

David Hildenbrand (6):
  cpus: Properly release the iothread lock when killing a dummy VCPU
  spapr: support memory unplug for qtest
  tests/device-plug: Add a simple PCI unplug request test
  tests/device-plug: Add CCW unplug test for s390x
  tests/device-plug: Add CPU core unplug request test for spapr
  tests/device-plug: Add memory unplug request test for spapr

 cpus.c                   |   1 +
 hw/ppc/spapr_ovec.c      |   6 ++
 tests/Makefile.include   |   4 +
 tests/device-plug-test.c | 162 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 173 insertions(+)
 create mode 100644 tests/device-plug-test.c

-- 
2.17.2

[Qemu-devel] [PATCH v3 1/6] cpus: Properly release the iothread lock when killing a dummy VCPU

[David Hildenbrand]

This enables CPU unplug under qtest.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 cpus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpus.c b/cpus.c
index 154daf57dc..e83f72b48b 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1333,6 +1333,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
         qemu_wait_io_event(cpu);
     } while (!cpu->unplug);
 
+    qemu_mutex_unlock_iothread();
     rcu_unregister_thread();
     return NULL;
 #endif
-- 
2.17.2

[Qemu-devel] [PATCH v3 2/6] spapr: support memory unplug for qtest

[David Hildenbrand]

Fake availability of OV5_HP_EVT, so we can test memory unplug in qtest.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/ppc/spapr_ovec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
index 318bf33de4..12510b236a 100644
--- a/hw/ppc/spapr_ovec.c
+++ b/hw/ppc/spapr_ovec.c
@@ -16,6 +16,7 @@
 #include "qemu/bitmap.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
+#include "sysemu/qtest.h"
 #include "trace.h"
 #include <libfdt.h>
 
@@ -131,6 +132,11 @@ bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr)
     g_assert(ov);
     g_assert(bitnr < OV_MAXBITS);
 
+    /* support memory unplug for qtest */
+    if (qtest_enabled() && bitnr == OV5_HP_EVT) {
+        return true;
+    }
+
     return test_bit(bitnr, ov->bitmap) ? true : false;
 }
 
-- 
2.17.2

[Qemu-devel] [PATCH v3 3/6] tests/device-plug: Add a simple PCI unplug request test

[David Hildenbrand]

The issue with testing asynchronous unplug requests it that they usually
require a running guest to handle the request. However, to test if
unplug of PCI devices works, we can apply a nice little trick on some
architectures:

On system reset, x86 ACPI, s390x and spapr will perform the unplug,
resulting in the device of interest to get deleted and a DEVICE_DELETED
event getting sent.

On s390x, we still get a warning
    qemu-system-s390x: -device virtio-mouse-pci,id=dev0:
    warning: Plugging a PCI/zPCI device without the 'zpci' CPU feature
    enabled; the guest will not be able to see/use this device

This will be fixed soon, when we enable the zpci CPU feature always
(Conny already has a patch for this queued).

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 tests/Makefile.include   |  4 ++
 tests/device-plug-test.c | 93 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)
 create mode 100644 tests/device-plug-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index b39e989f72..f242d65ea2 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -192,6 +192,7 @@ check-qtest-i386-$(CONFIG_ISA_IPMI_KCS) += tests/ipmi-kcs-test$(EXESUF)
 # check-qtest-i386-$(CONFIG_ISA_IPMI_BT) += tests/ipmi-bt-test$(EXESUF)
 check-qtest-i386-y += tests/i440fx-test$(EXESUF)
 check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
+check-qtest-i386-y += tests/device-plug-test$(EXESUF)
 check-qtest-i386-y += tests/drive_del-test$(EXESUF)
 check-qtest-i386-$(CONFIG_WDT_IB700) += tests/wdt_ib700-test$(EXESUF)
 check-qtest-i386-y += tests/tco-test$(EXESUF)
@@ -256,6 +257,7 @@ check-qtest-ppc-$(CONFIG_M48T59) += tests/m48t59-test$(EXESUF)
 
 check-qtest-ppc64-y += $(check-qtest-ppc-y)
 check-qtest-ppc64-$(CONFIG_PSERIES) += tests/spapr-phb-test$(EXESUF)
+check-qtest-ppc64-$(CONFIG_PSERIES) += tests/device-plug-test$(EXESUF)
 check-qtest-ppc64-$(CONFIG_POWERNV) += tests/pnv-xscom-test$(EXESUF)
 check-qtest-ppc64-y += tests/migration-test$(EXESUF)
 check-qtest-ppc64-$(CONFIG_PSERIES) += tests/rtas-test$(EXESUF)
@@ -310,6 +312,7 @@ check-qtest-s390x-$(CONFIG_SLIRP) += tests/test-netfilter$(EXESUF)
 check-qtest-s390x-$(CONFIG_POSIX) += tests/test-filter-mirror$(EXESUF)
 check-qtest-s390x-$(CONFIG_POSIX) += tests/test-filter-redirector$(EXESUF)
 check-qtest-s390x-y += tests/drive_del-test$(EXESUF)
+check-qtest-s390x-y += tests/device-plug-test$(EXESUF)
 check-qtest-s390x-y += tests/virtio-ccw-test$(EXESUF)
 check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
 check-qtest-s390x-y += tests/migration-test$(EXESUF)
@@ -750,6 +753,7 @@ tests/ipoctal232-test$(EXESUF): tests/ipoctal232-test.o
 tests/qom-test$(EXESUF): tests/qom-test.o
 tests/test-hmp$(EXESUF): tests/test-hmp.o
 tests/machine-none-test$(EXESUF): tests/machine-none-test.o
+tests/device-plug-test$(EXESUF): tests/device-plug-test.o
 tests/drive_del-test$(EXESUF): tests/drive_del-test.o $(libqos-virtio-obj-y)
 tests/nvme-test$(EXESUF): tests/nvme-test.o $(libqos-pc-obj-y)
 tests/pvpanic-test$(EXESUF): tests/pvpanic-test.o
diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
new file mode 100644
index 0000000000..cd9ada539d
--- /dev/null
+++ b/tests/device-plug-test.c
@@ -0,0 +1,93 @@
+/*
+ * QEMU device plug/unplug handling
+ *
+ * Copyright (C) 2019 Red Hat Inc.
+ *
+ * Authors:
+ *  David Hildenbrand <david@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
+
+static void device_del_request(QTestState *qtest, const char *id)
+{
+    QDict *resp;
+
+    resp = qtest_qmp(qtest,
+                     "{'execute': 'device_del', 'arguments': { 'id': %s } }",
+                     id);
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+}
+
+static void system_reset(QTestState *qtest)
+{
+    QDict *resp;
+
+    resp = qtest_qmp(qtest, "{'execute': 'system_reset'}");
+    g_assert(qdict_haskey(resp, "return"));
+    qobject_unref(resp);
+}
+
+static void wait_device_deleted_event(QTestState *qtest, const char *id)
+{
+    QDict *resp, *data;
+    QString *qstr;
+
+    /*
+     * Other devices might get removed along with the removed device. Skip
+     * these. The device of interest will be the last one.
+     */
+    for (;;) {
+        resp = qtest_qmp_eventwait_ref(qtest, "DEVICE_DELETED");
+        data = qdict_get_qdict(resp, "data");
+        if (!data || !qdict_get(data, "device")) {
+            qobject_unref(resp);
+            continue;
+        }
+        qstr = qobject_to(QString, qdict_get(data, "device"));
+        g_assert(qstr);
+        if (!strcmp(qstring_get_str(qstr), id)) {
+            qobject_unref(resp);
+            break;
+        }
+        qobject_unref(resp);
+    }
+}
+
+static void test_pci_unplug_request(void)
+{
+    QTestState *qtest = qtest_initf("-device virtio-mouse-pci,id=dev0");
+
+    /*
+     * Request device removal. As the guest is not running, the request won't
+     * be processed. However during system reset, the removal will be
+     * handled, removing the device.
+     */
+    device_del_request(qtest, "dev0");
+    system_reset(qtest);
+    wait_device_deleted_event(qtest, "dev0");
+
+    qtest_quit(qtest);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    /*
+     * We need a system that will process unplug requests during system resets
+     * and does not do PCI surprise removal. This holds for x86 ACPI,
+     * s390x and spapr.
+     */
+    qtest_add_func("/device-plug/pci-unplug-request",
+                   test_pci_unplug_request);
+
+    return g_test_run();
+}
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 3/6] tests/device-plug: Add a simple PCI unplug request test

[Cornelia Huck]

On Mon, 18 Feb 2019 10:21:59 +0100
David Hildenbrand <david@redhat.com> wrote:

> The issue with testing asynchronous unplug requests it that they usually
> require a running guest to handle the request. However, to test if
> unplug of PCI devices works, we can apply a nice little trick on some
> architectures:
> 
> On system reset, x86 ACPI, s390x and spapr will perform the unplug,
> resulting in the device of interest to get deleted and a DEVICE_DELETED
> event getting sent.
> 
> On s390x, we still get a warning
>     qemu-system-s390x: -device virtio-mouse-pci,id=dev0:
>     warning: Plugging a PCI/zPCI device without the 'zpci' CPU feature
>     enabled; the guest will not be able to see/use this device
> 
> This will be fixed soon, when we enable the zpci CPU feature always
> (Conny already has a patch for this queued).

Let's see which series makes it into mainline first ;)

> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  tests/Makefile.include   |  4 ++
>  tests/device-plug-test.c | 93 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+)
>  create mode 100644 tests/device-plug-test.c

Looks sane at a glance.

Acked-by: Cornelia Huck <cohuck@redhat.com>

[Qemu-devel] [PATCH v3 4/6] tests/device-plug: Add CCW unplug test for s390x

[David Hildenbrand]

As CCW unplugs are surprise removals without asking the guest first,
we can test this without any guest interaction.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 tests/device-plug-test.c | 41 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
index cd9ada539d..d1a6c94af2 100644
--- a/tests/device-plug-test.c
+++ b/tests/device-plug-test.c
@@ -15,17 +15,26 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
 
-static void device_del_request(QTestState *qtest, const char *id)
+static void device_del_start(QTestState *qtest, const char *id)
 {
-    QDict *resp;
+    qtest_qmp_send(qtest,
+                   "{'execute': 'device_del', 'arguments': { 'id': %s } }", id);
+}
+
+static void device_del_finish(QTestState *qtest)
+{
+    QDict *resp = qtest_qmp_receive(qtest);
 
-    resp = qtest_qmp(qtest,
-                     "{'execute': 'device_del', 'arguments': { 'id': %s } }",
-                     id);
     g_assert(qdict_haskey(resp, "return"));
     qobject_unref(resp);
 }
 
+static void device_del_request(QTestState *qtest, const char *id)
+{
+    device_del_start(qtest, id);
+    device_del_finish(qtest);
+}
+
 static void system_reset(QTestState *qtest)
 {
     QDict *resp;
@@ -77,8 +86,25 @@ static void test_pci_unplug_request(void)
     qtest_quit(qtest);
 }
 
+static void test_ccw_unplug(void)
+{
+    QTestState *qtest = qtest_initf("-device virtio-balloon-ccw,id=dev0");
+
+    /*
+     * The DEVICE_DELETED events will be sent before the command
+     * completes.
+     */
+    device_del_start(qtest, "dev0");
+    wait_device_deleted_event(qtest, "dev0");
+    device_del_finish(qtest);
+
+    qtest_quit(qtest);
+}
+
 int main(int argc, char **argv)
 {
+    const char *arch = qtest_get_arch();
+
     g_test_init(&argc, &argv, NULL);
 
     /*
@@ -89,5 +115,10 @@ int main(int argc, char **argv)
     qtest_add_func("/device-plug/pci-unplug-request",
                    test_pci_unplug_request);
 
+    if (!strcmp(arch, "s390x")) {
+        qtest_add_func("/device-plug/ccw-unplug",
+                       test_ccw_unplug);
+    }
+
     return g_test_run();
 }
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 4/6] tests/device-plug: Add CCW unplug test for s390x

[Cornelia Huck]

On Mon, 18 Feb 2019 10:22:00 +0100
David Hildenbrand <david@redhat.com> wrote:

> As CCW unplugs are surprise removals without asking the guest first,
> we can test this without any guest interaction.
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  tests/device-plug-test.c | 41 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 36 insertions(+), 5 deletions(-)

No in-depth review, but looks reasonable.

Acked-by: Cornelia Huck <cohuck@redhat.com>

[Qemu-devel] [PATCH v3 5/6] tests/device-plug: Add CPU core unplug request test for spapr

[David Hildenbrand]

We can easily test this, just like PCI. On s390x, cpu unplug is not
supported. On x86 ACPI, cpu unplug requires guest interaction to work, so
it can't be tested that easily. We might add tests for ACPI later.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 tests/device-plug-test.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
index d1a6c94af2..0262ad6be6 100644
--- a/tests/device-plug-test.c
+++ b/tests/device-plug-test.c
@@ -101,6 +101,21 @@ static void test_ccw_unplug(void)
     qtest_quit(qtest);
 }
 
+static void test_spapr_cpu_unplug_request(void)
+{
+    QTestState *qtest;
+
+    qtest = qtest_initf("-cpu power9_v2.0 -smp 1,maxcpus=2 "
+                        "-device power9_v2.0-spapr-cpu-core,core-id=1,id=dev0");
+
+    /* similar to test_pci_unplug_request */
+    device_del_request(qtest, "dev0");
+    system_reset(qtest);
+    wait_device_deleted_event(qtest, "dev0");
+
+    qtest_quit(qtest);
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -120,5 +135,10 @@ int main(int argc, char **argv)
                        test_ccw_unplug);
     }
 
+    if (!strcmp(arch, "ppc64")) {
+        qtest_add_func("/device-plug/spapr-cpu-unplug-request",
+                       test_spapr_cpu_unplug_request);
+    }
+
     return g_test_run();
 }
-- 
2.17.2

[Qemu-devel] [PATCH v3 6/6] tests/device-plug: Add memory unplug request test for spapr

[David Hildenbrand]

We can easily test this, just like PCI. On x86 ACPI, we need guest
interaction to make it work, so it is not that easy to test. We might
add tests for that later on.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 tests/device-plug-test.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
index 0262ad6be6..87593d9ecf 100644
--- a/tests/device-plug-test.c
+++ b/tests/device-plug-test.c
@@ -116,6 +116,22 @@ static void test_spapr_cpu_unplug_request(void)
     qtest_quit(qtest);
 }
 
+static void test_spapr_memory_unplug_request(void)
+{
+    QTestState *qtest;
+
+    qtest = qtest_initf("-m 256M,slots=1,maxmem=768M "
+                        "-object memory-backend-ram,id=mem0,size=512M "
+                        "-device pc-dimm,id=dev0,memdev=mem0");
+
+    /* similar to test_pci_unplug_request */
+    device_del_request(qtest, "dev0");
+    system_reset(qtest);
+    wait_device_deleted_event(qtest, "dev0");
+
+    qtest_quit(qtest);
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -138,6 +154,8 @@ int main(int argc, char **argv)
     if (!strcmp(arch, "ppc64")) {
         qtest_add_func("/device-plug/spapr-cpu-unplug-request",
                        test_spapr_cpu_unplug_request);
+        qtest_add_func("/device-plug/spapr-memory-unplug-request",
+                       test_spapr_memory_unplug_request);
     }
 
     return g_test_run();
-- 
2.17.2

Re: [Qemu-devel] [PATCH v3 0/6] tests: Add device unplug tests

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]

On Mon, Feb 18, 2019 at 10:21:56AM +0100, David Hildenbrand wrote:
> This is a set of tests to test basic device unplugging functionality for
> - some PCI implementations
> - CCW devices on s390x
> - spapr memory and cpu core devices
> 
> I plaed with ACPI CPU unplug but getting that to run with qtest is more
> involved. (remove devices on reset, trick cpu-hotplug-legacy property,
> somehow create cpu hotplug state objects ...). Well we no have at least
> one unplug test for DIMMs and one for CPUs.

I believe we were talking about merging this via the ppc tree, so I've
merged it into ppc-for-4.0 (with Connie's acks).

If I was mistaken about that, let me know and I'll pull them out
before I send my next pull request.

> 
> v2 -> v3:
> - Use a smaller DIMM for spapr memory unplug request test
> - Combine both spapr test registrations in a single if statement
> - Use "-" instead of "_" in test names
> - Add a huge load of r-b/a-b :) hpe I didn't forget some.
> 
> v1 -> v2:
> - Tests for CCW, spapr memory and spapr cpu core unplug added
> - Fix a deadlock when unplugging dummy CPUs
> - Style fixes / simplifications (Thomas)
> - Don't use the global_qtest variable (Thomas)
> - Fix double-free (Greg)
> 
> David Hildenbrand (6):
>   cpus: Properly release the iothread lock when killing a dummy VCPU
>   spapr: support memory unplug for qtest
>   tests/device-plug: Add a simple PCI unplug request test
>   tests/device-plug: Add CCW unplug test for s390x
>   tests/device-plug: Add CPU core unplug request test for spapr
>   tests/device-plug: Add memory unplug request test for spapr
> 
>  cpus.c                   |   1 +
>  hw/ppc/spapr_ovec.c      |   6 ++
>  tests/Makefile.include   |   4 +
>  tests/device-plug-test.c | 162 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 173 insertions(+)
>  create mode 100644 tests/device-plug-test.c
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v3 0/6] tests: Add device unplug tests

[David Hildenbrand]

On 19.02.19 03:58, David Gibson wrote:
> On Mon, Feb 18, 2019 at 10:21:56AM +0100, David Hildenbrand wrote:
>> This is a set of tests to test basic device unplugging functionality for
>> - some PCI implementations
>> - CCW devices on s390x
>> - spapr memory and cpu core devices
>>
>> I plaed with ACPI CPU unplug but getting that to run with qtest is more
>> involved. (remove devices on reset, trick cpu-hotplug-legacy property,
>> somehow create cpu hotplug state objects ...). Well we no have at least
>> one unplug test for DIMMs and one for CPUs.
> 
> I believe we were talking about merging this via the ppc tree, so I've
> merged it into ppc-for-4.0 (with Connie's acks).
> 
> If I was mistaken about that, let me know and I'll pull them out
> before I send my next pull request.

That's also how I understood it :)

Thanks David!

-- 

Thanks,

David / dhildenb