[Qemu-devel] [PATCH v2 for-2.11] hw/ppc/spapr_cpu_core: Add a proper check for spapr machine

[Qemu-devel] [PATCH v2 for-2.11] hw/ppc/spapr_cpu_core: Add a proper check for spapr machine

[Thomas Huth]

QEMU currently crashes when the user tries to add a spapr-cpu-core
on a non-pseries machine:

$ qemu-system-ppc64 -S -machine ppce500,accel=tcg \
                    -device POWER5+_v2.1-spapr-cpu-core
hw/ppc/spapr_cpu_core.c:178:spapr_cpu_core_realize_child:
Object 0x55cee1f55160 is not an instance of type spapr-machine
Aborted (core dumped)

So let's add a proper check for the correct machine time with
a more friendly error message here.

Reported-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2:
 - Add the check to spapr_cpu_core_realize() instead of adding it to
   spapr_cpu_core_realize_child()
 - Use device/machine whitelist for the device-crash-test instead of
   adding a log entry there

 hw/ppc/spapr_cpu_core.c   | 5 +++++
 scripts/device-crash-test | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index ea278ce..dd92326 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -220,6 +220,11 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
     void *obj;
     int i, j;
 
+    if (!object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE)) {
+        error_setg(errp, "spapr-cpu-core needs a pseries machine");
+        return;
+    }
+
     sc->threads = g_malloc0(size * cc->nr_threads);
     for (i = 0; i < cc->nr_threads; i++) {
         char id[32];
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index 81d65b9..043b24a 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -160,6 +160,9 @@ ERROR_WHITELIST = [
     {'machine':'q35|pc.*', 'device':'kvm-ioapic', 'expected':True}, # Only 1 ioapics allowed
     {'machine':'q35|pc.*', 'device':'ioapic', 'expected':True},     # Only 1 ioapics allowed
 
+    # "spapr-cpu-core needs a pseries machine"
+    {'machine':'(?!pseries).*', 'device':'.*-spapr-cpu-core', 'expected':True},
+
     # KVM-specific devices shouldn't be tried without accel=kvm:
     {'accel':'(?!kvm).*', 'device':'kvmclock', 'expected':True},
     {'accel':'(?!kvm).*', 'device':'kvm-pci-assign', 'expected':True},
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 for-2.11] hw/ppc/spapr_cpu_core: Add a proper check for spapr machine

[David Gibson]

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

On Thu, Aug 24, 2017 at 05:52:32AM +0200, Thomas Huth wrote:
> QEMU currently crashes when the user tries to add a spapr-cpu-core
> on a non-pseries machine:
> 
> $ qemu-system-ppc64 -S -machine ppce500,accel=tcg \
>                     -device POWER5+_v2.1-spapr-cpu-core
> hw/ppc/spapr_cpu_core.c:178:spapr_cpu_core_realize_child:
> Object 0x55cee1f55160 is not an instance of type spapr-machine
> Aborted (core dumped)
> 
> So let's add a proper check for the correct machine time with
> a more friendly error message here.
> 
> Reported-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Applied to ppc-for-2.11, thanks.

> ---
>  v2:
>  - Add the check to spapr_cpu_core_realize() instead of adding it to
>    spapr_cpu_core_realize_child()
>  - Use device/machine whitelist for the device-crash-test instead of
>    adding a log entry there
> 
>  hw/ppc/spapr_cpu_core.c   | 5 +++++
>  scripts/device-crash-test | 3 +++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index ea278ce..dd92326 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -220,6 +220,11 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
>      void *obj;
>      int i, j;
>  
> +    if (!object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE)) {
> +        error_setg(errp, "spapr-cpu-core needs a pseries machine");
> +        return;
> +    }
> +
>      sc->threads = g_malloc0(size * cc->nr_threads);
>      for (i = 0; i < cc->nr_threads; i++) {
>          char id[32];
> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> index 81d65b9..043b24a 100755
> --- a/scripts/device-crash-test
> +++ b/scripts/device-crash-test
> @@ -160,6 +160,9 @@ ERROR_WHITELIST = [
>      {'machine':'q35|pc.*', 'device':'kvm-ioapic', 'expected':True}, # Only 1 ioapics allowed
>      {'machine':'q35|pc.*', 'device':'ioapic', 'expected':True},     # Only 1 ioapics allowed
>  
> +    # "spapr-cpu-core needs a pseries machine"
> +    {'machine':'(?!pseries).*', 'device':'.*-spapr-cpu-core', 'expected':True},
> +
>      # KVM-specific devices shouldn't be tried without accel=kvm:
>      {'accel':'(?!kvm).*', 'device':'kvmclock', 'expected':True},
>      {'accel':'(?!kvm).*', 'device':'kvm-pci-assign', 'expected':True},

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