1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] Revert "s390x/ccw: create s390 phb conditionally"

Christian Borntraeger posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170928131831.81393-1-borntraeger@de.ibm.com
Test checkpatch passed
Test docker passed
Test s390x passed
hw/s390x/s390-virtio-ccw.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
[Qemu-devel] [PATCH] Revert "s390x/ccw: create s390 phb conditionally"
Posted by Christian Borntraeger 6 years, 6 months ago 
This reverts commit d32bd032d8fde41281aae34c16a4aa97e9acfeac.

Turns out that old QEMUs always created a pci host bridge
and for many CPU models the migration from old QEMUs to new
QEMUs will fail with
qemu-system-s390x: Unknown savevm section or instance 'PCIBUS' 0
qemu-system-s390x: load of migration failed: Invalid argument

As a quick fix we will revert the commit and always create the
pci host bridge.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
[fixed revert to keep the comment fixup, added a comment in the code]
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index b6aa2da..a77336f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -275,6 +275,7 @@ static void ccw_init(MachineState *machine)
 {
     int ret;
     VirtualCssBus *css_bus;
+    DeviceState *dev;
 
     s390_sclp_init();
     s390_memory_init(machine->ram_size);
@@ -290,13 +291,14 @@ static void ccw_init(MachineState *machine)
                       machine->initrd_filename, "s390-ccw.img",
                       "s390-netboot.img", true);
 
-    if (s390_has_feat(S390_FEAT_ZPCI)) {
-        DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
-        object_property_add_child(qdev_get_machine(),
-                                  TYPE_S390_PCI_HOST_BRIDGE,
-                                  OBJECT(dev), NULL);
-        qdev_init_nofail(dev);
-    }
+    /*
+     * We cannot easily make the pci host bridge conditional as older QEMUs
+     * always created it. Doing so would break migration across QEMU versions
+     */
+    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
+    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
+                              OBJECT(dev), NULL);
+    qdev_init_nofail(dev);
 
     /* register hypercalls */
     virtio_ccw_register_hcalls();
-- 
2.9.4
Re: [Qemu-devel] [PATCH] Revert "s390x/ccw: create s390 phb conditionally"
Posted by David Hildenbrand 6 years, 6 months ago 
On 28.09.2017 15:18, Christian Borntraeger wrote:
> This reverts commit d32bd032d8fde41281aae34c16a4aa97e9acfeac.
> 
> Turns out that old QEMUs always created a pci host bridge
> and for many CPU models the migration from old QEMUs to new
> QEMUs will fail with
> qemu-system-s390x: Unknown savevm section or instance 'PCIBUS' 0
> qemu-system-s390x: load of migration failed: Invalid argument
> 
> As a quick fix we will revert the commit and always create the
> pci host bridge.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> [fixed revert to keep the comment fixup, added a comment in the code]
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index b6aa2da..a77336f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -275,6 +275,7 @@ static void ccw_init(MachineState *machine)
>  {
>      int ret;
>      VirtualCssBus *css_bus;
> +    DeviceState *dev;
>  
>      s390_sclp_init();
>      s390_memory_init(machine->ram_size);
> @@ -290,13 +291,14 @@ static void ccw_init(MachineState *machine)
>                        machine->initrd_filename, "s390-ccw.img",
>                        "s390-netboot.img", true);
>  
> -    if (s390_has_feat(S390_FEAT_ZPCI)) {
> -        DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
> -        object_property_add_child(qdev_get_machine(),
> -                                  TYPE_S390_PCI_HOST_BRIDGE,
> -                                  OBJECT(dev), NULL);
> -        qdev_init_nofail(dev);
> -    }
> +    /*
> +     * We cannot easily make the pci host bridge conditional as older QEMUs
> +     * always created it. Doing so would break migration across QEMU versions
> +     */
> +    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
> +    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
> +                              OBJECT(dev), NULL);
> +    qdev_init_nofail(dev);
>  
>      /* register hypercalls */
>      virtio_ccw_register_hcalls();
> 

This zPCI stuff is just nasty.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David
Re: [Qemu-devel] [PATCH] Revert "s390x/ccw: create s390 phb conditionally"
Posted by Cornelia Huck 6 years, 6 months ago 
On Thu, 28 Sep 2017 15:18:31 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> This reverts commit d32bd032d8fde41281aae34c16a4aa97e9acfeac.
> 
> Turns out that old QEMUs always created a pci host bridge
> and for many CPU models the migration from old QEMUs to new
> QEMUs will fail with
> qemu-system-s390x: Unknown savevm section or instance 'PCIBUS' 0
> qemu-system-s390x: load of migration failed: Invalid argument
> 
> As a quick fix we will revert the commit and always create the
> pci host bridge.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> [fixed revert to keep the comment fixup, added a comment in the code]
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index b6aa2da..a77336f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -275,6 +275,7 @@ static void ccw_init(MachineState *machine)
>  {
>      int ret;
>      VirtualCssBus *css_bus;
> +    DeviceState *dev;
>  
>      s390_sclp_init();
>      s390_memory_init(machine->ram_size);
> @@ -290,13 +291,14 @@ static void ccw_init(MachineState *machine)
>                        machine->initrd_filename, "s390-ccw.img",
>                        "s390-netboot.img", true);
>  
> -    if (s390_has_feat(S390_FEAT_ZPCI)) {
> -        DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
> -        object_property_add_child(qdev_get_machine(),
> -                                  TYPE_S390_PCI_HOST_BRIDGE,
> -                                  OBJECT(dev), NULL);
> -        qdev_init_nofail(dev);
> -    }
> +    /*
> +     * We cannot easily make the pci host bridge conditional as older QEMUs
> +     * always created it. Doing so would break migration across QEMU versions

Added the missing trailing period.

> +     */
> +    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
> +    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
> +                              OBJECT(dev), NULL);
> +    qdev_init_nofail(dev);
>  
>      /* register hypercalls */
>      virtio_ccw_register_hcalls();

Thanks, applied.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.