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

[Cornelia Huck <cohuck@redhat.com>]

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.