| Message ID | 20201026143028.3034018-13-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | remove bios_name variable | expand |
Paolo Bonzini <pbonzini@redhat.com> writes:
Might be worth mentioning the trixy path from qdev_prop_set_string to
end up in ipl->firmware because it's not obvious just from the diff.
Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On 26/10/2020 15.30, Paolo Bonzini wrote: > Cc: Thomas Huth <thuth@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/s390x/ipl.c | 8 ++------ > hw/s390x/s390-virtio-ccw.c | 3 ++- > 2 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 3d2652d75a..61e8c967d3 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -128,11 +128,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) > if (!ipl->kernel || ipl->enforce_bios) { > uint64_t fwbase = (MIN(ram_size, 0x80000000U) - 0x200000) & ~0xffffUL; > > - if (bios_name == NULL) { > - bios_name = ipl->firmware; > - } > - > - bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); > + bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ipl->firmware); > if (bios_filename == NULL) { > error_setg(errp, "could not find stage1 bootloader"); > return; > @@ -154,7 +150,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) > g_free(bios_filename); > > if (bios_size == -1) { > - error_setg(errp, "could not load bootloader '%s'", bios_name); > + error_setg(errp, "could not load bootloader '%s'", ipl->firmware); > return; > } > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index e52182f946..a521eba673 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -258,7 +258,8 @@ static void ccw_init(MachineState *machine) > /* get a BUS */ > css_bus = virtual_css_bus_init(); > s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline, > - machine->initrd_filename, "s390-ccw.img", > + machine->initrd_filename, > + machine->firmware ?: "s390-ccw.img", > "s390-netboot.img", true); > > dev = qdev_new(TYPE_S390_PCI_HOST_BRIDGE); > Reviewed-by: Thomas Huth <thuth@redhat.com>
On Mon, 26 Oct 2020 10:30:25 -0400 Paolo Bonzini <pbonzini@redhat.com> wrote: > Cc: Thomas Huth <thuth@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/s390x/ipl.c | 8 ++------ > hw/s390x/s390-virtio-ccw.c | 3 ++- > 2 files changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 26.10.20 15:30, Paolo Bonzini wrote: > Cc: Thomas Huth <thuth@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/s390x/ipl.c | 8 ++------ > hw/s390x/s390-virtio-ccw.c | 3 ++- > 2 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 3d2652d75a..61e8c967d3 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -128,11 +128,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) > if (!ipl->kernel || ipl->enforce_bios) { > uint64_t fwbase = (MIN(ram_size, 0x80000000U) - 0x200000) & ~0xffffUL; > > - if (bios_name == NULL) { > - bios_name = ipl->firmware; > - } > - > - bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); > + bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ipl->firmware); > if (bios_filename == NULL) { > error_setg(errp, "could not find stage1 bootloader"); > return; > @@ -154,7 +150,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) > g_free(bios_filename); > > if (bios_size == -1) { > - error_setg(errp, "could not load bootloader '%s'", bios_name); > + error_setg(errp, "could not load bootloader '%s'", ipl->firmware); > return; > } > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index e52182f946..a521eba673 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -258,7 +258,8 @@ static void ccw_init(MachineState *machine) > /* get a BUS */ > css_bus = virtual_css_bus_init(); > s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline, > - machine->initrd_filename, "s390-ccw.img", > + machine->initrd_filename, > + machine->firmware ?: "s390-ccw.img", Adding the elvis operator is actually a fix, no?
On 27/10/20 09:38, Christian Borntraeger wrote: >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index e52182f946..a521eba673 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -258,7 +258,8 @@ static void ccw_init(MachineState *machine) >> /* get a BUS */ >> css_bus = virtual_css_bus_init(); >> s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline, >> - machine->initrd_filename, "s390-ccw.img", >> + machine->initrd_filename, >> + machine->firmware ?: "s390-ccw.img", > Adding the elvis operator is actually a fix, no? > I think it was already doing the equivalent here in s390_ipl_realize if (bios_name == NULL) { bios_name = ipl->firmware; } bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); so it was just an encapsulation violation on part of the IPL device. Paolo
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 3d2652d75a..61e8c967d3 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -128,11 +128,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) if (!ipl->kernel || ipl->enforce_bios) { uint64_t fwbase = (MIN(ram_size, 0x80000000U) - 0x200000) & ~0xffffUL; - if (bios_name == NULL) { - bios_name = ipl->firmware; - } - - bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); + bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ipl->firmware); if (bios_filename == NULL) { error_setg(errp, "could not find stage1 bootloader"); return; @@ -154,7 +150,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp) g_free(bios_filename); if (bios_size == -1) { - error_setg(errp, "could not load bootloader '%s'", bios_name); + error_setg(errp, "could not load bootloader '%s'", ipl->firmware); return; } diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index e52182f946..a521eba673 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -258,7 +258,8 @@ static void ccw_init(MachineState *machine) /* get a BUS */ css_bus = virtual_css_bus_init(); s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline, - machine->initrd_filename, "s390-ccw.img", + machine->initrd_filename, + machine->firmware ?: "s390-ccw.img", "s390-netboot.img", true); dev = qdev_new(TYPE_S390_PCI_HOST_BRIDGE);
Cc: Thomas Huth <thuth@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/s390x/ipl.c | 8 ++------ hw/s390x/s390-virtio-ccw.c | 3 ++- 2 files changed, 4 insertions(+), 7 deletions(-)