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

[Qemu-devel] [PATCH v2 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.

@David, I dropped you t-b and r-b due to many changes but would be happy
to readd them ;)

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 | 165 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 176 insertions(+)
 create mode 100644 tests/device-plug-test.c

-- 
2.17.2

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

[David Hildenbrand]

This enables CPU unplug under qtest.

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 v2 2/6] spapr: support memory unplug for qtest

[David Hildenbrand]

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

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 v2 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).

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..066433ebf5
--- /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

[Qemu-devel] [PATCH v2 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.

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 066433ebf5..6f7255552a 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

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

[David Hildenbrand]

We can easily test this, just like PCI.

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 6f7255552a..ec6cb5de7b 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 v2 6/6] tests/device-plug: Add memory unplug request test for spapr

[David Hildenbrand]

We can easily test this, just like PCI.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 tests/device-plug-test.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
index ec6cb5de7b..4c581319c0 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 1G,slots=1,maxmem=2G "
+                        "-object memory-backend-ram,id=mem0,size=1G "
+                        "-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();
@@ -140,5 +156,10 @@ int main(int argc, char **argv)
                        test_spapr_cpu_unplug_request);
     }
 
+    if (!strcmp(arch, "ppc64")) {
+        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 v2 2/6] spapr: support memory unplug for qtest

[Greg Kurz]

On Fri, 15 Feb 2019 14:30:01 +0100
David Hildenbrand <david@redhat.com> wrote:

> Fake availability of OV5_HP_EVT, so we can test memory unplug in qtest.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  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;
>  }
>  

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

[Greg Kurz]

On Fri, 15 Feb 2019 14:30:00 +0100
David Hildenbrand <david@redhat.com> wrote:

> This enables CPU unplug under qtest.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  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

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

[Thomas Huth]

On 15/02/2019 14.30, David Hildenbrand wrote:
> This enables CPU unplug under qtest.
> 
> 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

That's definitely a bug that you fix here.

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Greg Kurz]

On Fri, 15 Feb 2019 14:30:02 +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).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  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..066433ebf5
> --- /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();
> +}

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

[Thomas Huth]

On 15/02/2019 14.30, David Hildenbrand 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).
> 
> 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

Reviewed-by: Thomas Huth <thuth@redhat.com>

BTW, have you spotted the qpci_unplug_acpi_device_test() in libqos
already? Might be interesting for these kinds of tests, too - we use it
in a couple of other tests already, though, so I'm not sure if we need
additional tests with that...

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

[Thomas Huth]

On 15/02/2019 14.30, David Hildenbrand wrote:
> As CCW unplugs are surprise removals without asking the guest first,
> we can test this without any guest interaction.
> 
> 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 066433ebf5..6f7255552a 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);
> +    }

Nit: Most of the other qtests rather use "-" instead of "_" in their
names. Apart from that:

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Thomas Huth]

On 15/02/2019 14.30, David Hildenbrand wrote:
> We can easily test this, just like PCI.

... maybe add a sentence why this is only done for spapr, and not for
s390x and x86 ?

> 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 6f7255552a..ec6cb5de7b 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);
> +}

My initial thought was: This should go into tests/cpu-plug-test.c
instead ... but since you need the functions that you defined here,
looks like this is the better place here...

>  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",

spapr-cpu-unplug-request ?

> +                       test_spapr_cpu_unplug_request);
> +    }
> +
>      return g_test_run();
>  }

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Thomas Huth]

On 15/02/2019 14.30, David Hildenbrand wrote:
> We can easily test this, just like PCI.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  tests/device-plug-test.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
> index ec6cb5de7b..4c581319c0 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 1G,slots=1,maxmem=2G "
> +                        "-object memory-backend-ram,id=mem0,size=1G "
> +                        "-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();
> @@ -140,5 +156,10 @@ int main(int argc, char **argv)
>                         test_spapr_cpu_unplug_request);
>      }
>  
> +    if (!strcmp(arch, "ppc64")) {
> +        qtest_add_func("/device-plug/spapr_memory_unplug_request",
> +                       test_spapr_memory_unplug_request);
> +    }
> +
>      return g_test_run();
>  }

I think I'd maybe use a smaller DIMM (256 MiB? 512 MiB), just in case
the test runs on an overloaded CI system with memory constraints ... but
anyway:

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Thomas Huth]

On 15/02/2019 14.30, David Hildenbrand wrote:
> We can easily test this, just like PCI.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  tests/device-plug-test.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
> index ec6cb5de7b..4c581319c0 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 1G,slots=1,maxmem=2G "
> +                        "-object memory-backend-ram,id=mem0,size=1G "
> +                        "-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();
> @@ -140,5 +156,10 @@ int main(int argc, char **argv)
>                         test_spapr_cpu_unplug_request);
>      }
>  
> +    if (!strcmp(arch, "ppc64")) {
> +        qtest_add_func("/device-plug/spapr_memory_unplug_request",
> +                       test_spapr_memory_unplug_request);
> +    }

By the way, it's maybe nicer to put all ppc-related tests into the same
if-statement?

 Thomas

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

[Greg Kurz]

On Fri, 15 Feb 2019 14:30:04 +0100
David Hildenbrand <david@redhat.com> wrote:

> We can easily test this, just like PCI.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  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 6f7255552a..ec6cb5de7b 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();
>  }

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

[Greg Kurz]

On Fri, 15 Feb 2019 14:30:05 +0100
David Hildenbrand <david@redhat.com> wrote:

> We can easily test this, just like PCI.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  tests/device-plug-test.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
> index ec6cb5de7b..4c581319c0 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 1G,slots=1,maxmem=2G "
> +                        "-object memory-backend-ram,id=mem0,size=1G "
> +                        "-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();
> @@ -140,5 +156,10 @@ int main(int argc, char **argv)
>                         test_spapr_cpu_unplug_request);
>      }
>  
> +    if (!strcmp(arch, "ppc64")) {
> +        qtest_add_func("/device-plug/spapr_memory_unplug_request",
> +                       test_spapr_memory_unplug_request);
> +    }
> +
>      return g_test_run();
>  }

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

[Collin Walling]

On 2/15/19 8:30 AM, David Hildenbrand 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).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Looks fine from what I can tell. Ran it on my own system and passed
with no issues.

Reviewed-by: Collin Walling <walling@linux.ibm.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..066433ebf5
> --- /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();
> +}
> 

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

[Michael S. Tsirkin]

On Fri, Feb 15, 2019 at 02:29:59PM +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.
> 
> @David, I dropped you t-b and r-b due to many changes but would be happy
> to readd them ;)


Series:
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


I think it's mostly spapr things so feel free to merge through
that tree.

> 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 | 165 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 176 insertions(+)
>  create mode 100644 tests/device-plug-test.c
> 
> -- 
> 2.17.2

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

[David Hildenbrand]

On 15.02.19 22:27, Michael S. Tsirkin wrote:
> On Fri, Feb 15, 2019 at 02:29:59PM +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.
>>
>> @David, I dropped you t-b and r-b due to many changes but would be happy
>> to readd them ;)
> 
> 
> Series:
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> 
> I think it's mostly spapr things so feel free to merge through
> that tree.

Thanks, I'll address the minor review comments next week and most
probably resend on Monday. I also agree that spapr would be a good fit.


-- 

Thanks,

David / dhildenb

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

[David Gibson]

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

On Fri, Feb 15, 2019 at 02:30:00PM +0100, David Hildenbrand wrote:
> This enables CPU unplug under qtest.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  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

-- 
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 v2 2/6] spapr: support memory unplug for qtest

[David Gibson]

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

On Fri, Feb 15, 2019 at 02:30:01PM +0100, David Hildenbrand wrote:
> Fake availability of OV5_HP_EVT, so we can test memory unplug in qtest.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Kinda gross, but I don't see a better way of handling this quickly,
so,

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  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;
>  }
>  

-- 
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 v2 3/6] tests/device-plug: Add a simple PCI unplug request test

[David Gibson]

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

On Fri, Feb 15, 2019 at 02:30:02PM +0100, David Hildenbrand 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).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  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..066433ebf5
> --- /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();
> +}

-- 
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 v2 6/6] tests/device-plug: Add memory unplug request test for spapr

[David Gibson]

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

On Fri, Feb 15, 2019 at 04:56:53PM +0100, Thomas Huth wrote:
> On 15/02/2019 14.30, David Hildenbrand wrote:
> > We can easily test this, just like PCI.
> > 
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> > ---
> >  tests/device-plug-test.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
> > index ec6cb5de7b..4c581319c0 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 1G,slots=1,maxmem=2G "
> > +                        "-object memory-backend-ram,id=mem0,size=1G "
> > +                        "-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();
> > @@ -140,5 +156,10 @@ int main(int argc, char **argv)
> >                         test_spapr_cpu_unplug_request);
> >      }
> >  
> > +    if (!strcmp(arch, "ppc64")) {
> > +        qtest_add_func("/device-plug/spapr_memory_unplug_request",
> > +                       test_spapr_memory_unplug_request);
> > +    }
> > +
> >      return g_test_run();
> >  }
> 
> I think I'd maybe use a smaller DIMM (256 MiB? 512 MiB), just in case
> the test runs on an overloaded CI system with memory constraints ... but
> anyway:

256MiB is the smallest we can do - that's the LMB size.  I'd suggest
512MiB so we're testing the logic that gathers the multiple LMBs for
the DIMM, since that's kind of hairy (it's basically the bridge
between the PAPR and qemu hotplug models which are quite different for
memory).

-- 
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 v2 0/6] tests: Add device unplug tests

[David Gibson]

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

On Fri, Feb 15, 2019 at 11:09:32PM +0100, David Hildenbrand wrote:
> On 15.02.19 22:27, Michael S. Tsirkin wrote:
> > On Fri, Feb 15, 2019 at 02:29:59PM +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.
> >>
> >> @David, I dropped you t-b and r-b due to many changes but would be happy
> >> to readd them ;)
> > 
> > 
> > Series:
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > 
> > I think it's mostly spapr things so feel free to merge through
> > that tree.
> 
> Thanks, I'll address the minor review comments next week and most
> probably resend on Monday. I also agree that spapr would be a good fit.

Ah, ok.  I can do that.

-- 
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 v2 3/6] tests/device-plug: Add a simple PCI unplug request test

[David Hildenbrand]

On 15.02.19 16:21, Thomas Huth wrote:
> On 15/02/2019 14.30, David Hildenbrand 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).
>>
>> 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
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> BTW, have you spotted the qpci_unplug_acpi_device_test() in libqos
> already? Might be interesting for these kinds of tests, too - we use it
> in a couple of other tests already, though, so I'm not sure if we need
> additional tests with that...
> 

Cool, no I haven't! It's PCI only, something similar for CPU/memory
unplug on x86 would be interesting. I might be looking into that in the
future.

-- 

Thanks,

David / dhildenb

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

[David Hildenbrand]

On 15.02.19 16:35, Thomas Huth wrote:
> On 15/02/2019 14.30, David Hildenbrand wrote:
>> We can easily test this, just like PCI.
> 
> ... maybe add a sentence why this is only done for spapr, and not for
> s390x and x86 ?

Yes, will do!

> 
>> 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 6f7255552a..ec6cb5de7b 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);
>> +}
> 
> My initial thought was: This should go into tests/cpu-plug-test.c
> instead ... but since you need the functions that you defined here,
> looks like this is the better place here...

Yes, I consider the tests in here to test basic unplug (+later plug)
functionality for all kinds of devices. Very specific tests (e.g.
testing different cpu plug combinations) should be handled in different
files.

Thanks!

-- 

Thanks,

David / dhildenb

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

[David Hildenbrand]

On 15.02.19 16:58, Thomas Huth wrote:
> On 15/02/2019 14.30, David Hildenbrand wrote:
>> We can easily test this, just like PCI.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  tests/device-plug-test.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/tests/device-plug-test.c b/tests/device-plug-test.c
>> index ec6cb5de7b..4c581319c0 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 1G,slots=1,maxmem=2G "
>> +                        "-object memory-backend-ram,id=mem0,size=1G "
>> +                        "-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();
>> @@ -140,5 +156,10 @@ int main(int argc, char **argv)
>>                         test_spapr_cpu_unplug_request);
>>      }
>>  
>> +    if (!strcmp(arch, "ppc64")) {
>> +        qtest_add_func("/device-plug/spapr_memory_unplug_request",
>> +                       test_spapr_memory_unplug_request);
>> +    }
> 
> By the way, it's maybe nicer to put all ppc-related tests into the same
> if-statement?

Yes, makes sense!

> 
>  Thomas
> 


-- 

Thanks,

David / dhildenb