Re: [Qemu-devel] [PATCH] hw/s390x: Add the possibility to specify the netboot image on the command line

[Viktor Mihajlovski]

On 27.02.2018 12:32, Thomas Huth wrote:
> The file name of the netboot binary is currently hard-coded to
> "s390-netboot.img", without a possibility for the user to select
> an alternative firmware image here. That's unfortunate, especially
> since the basics are already there: The filename is a property of
> the s390-ipl device. So we just have to add a check whether the user
> already provided the property and only set the default if the string
> is still empty. Now it is possible to select a different firmware
> image with "-global s390-ipl.netboot_fw=/path/to/s390-netboot.img".
> While I sympathize with the concept, it will be a bit hard to consume
since most of the QEMU instances will be started by libvirt and that has
no provisions for this kind of firmware replacement. We could craft some
s390-specific variety of the <loader> element.
Another thing to consider is that, while the current netboot firmware
doesn't rely on any special QEMU <-> loader interfaces, it might happen
that QEMU and the network firmware must match similar to QEMU and the
s390-ccw firmware image and I have no idea on how to ensure that.
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  hw/s390x/s390-virtio-ccw.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4abbe89..7b3fb5f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -254,8 +254,10 @@ static void s390_init_ipl_dev(const char 
> *kernel_filename,
>      }
>      qdev_prop_set_string(dev, "cmdline", kernel_cmdline);
>      qdev_prop_set_string(dev, "firmware", firmware);
Wouldn't it be consequent to allow firmware replacement? But then, see
above.
> -    qdev_prop_set_string(dev, "netboot_fw", netboot_fw);
>      qdev_prop_set_bit(dev, "enforce_bios", enforce_bios);
> +    if (!strlen(object_property_get_str(new, "netboot_fw", &error_abort))) {
> +        qdev_prop_set_string(dev, "netboot_fw", netboot_fw);
> +    }
>      object_property_add_child(qdev_get_machine(), TYPE_S390_IPL,
>                                new, NULL);
>      object_unref(new);
> 


-- 
Regards,
 Viktor Mihajlovski