qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-5.2 5/6] pc-bios/s390-ccw: Scan through all boot devices


From: Cornelia Huck
Subject: Re: [PATCH for-5.2 5/6] pc-bios/s390-ccw: Scan through all boot devices if none has been specified
Date: Wed, 5 Aug 2020 11:36:33 +0200

On Tue, 28 Jul 2020 20:37:33 +0200
Thomas Huth <thuth@redhat.com> wrote:

> If no boot device has been specified (via "bootindex=..."), the s390-ccw
> bios scans through all devices to find a bootable device. But so far, it
> stops at the very first block device (including virtio-scsi controllers
> without attached devices) that it finds, no matter whether it is bootable
> or not. That leads to some weird situatation where it is e.g. possible
> to boot via:
> 
>  qemu-system-s390x -hda /path/to/disk.qcow2
> 
> but not if there is e.g. a virtio-scsi controller specified before:
> 
>  qemu-system-s390x -device virtio-scsi -hda /path/to/disk.qcow2
> 
> While using "bootindex=..." is clearly the preferred way of booting
> on s390x, we still can make the life for the users at least a little
> bit easier if we look at all available devices to find a bootable one.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1846975
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  pc-bios/s390-ccw/main.c | 46 +++++++++++++++++++++++++++--------------
>  1 file changed, 31 insertions(+), 15 deletions(-)

(...)

>  int main(void)
>  {
>      sclp_setup();
>      css_setup();
>      boot_setup();
> -    find_boot_device();
> -    enable_subchannel(blk_schid);
> -    ipl_boot_device();
> +    if (have_iplb) {
> +        find_boot_device();
> +        enable_subchannel(blk_schid);
> +        ipl_boot_device();
> +    } else {
> +        probe_boot_device();
> +    }

The one thing that's a bit surprising with the code is that
enable_subchannel() sticking out now. The code looking for a boot
device does that for all subchannels it looks at... but I think
find_boot_device() did that for specified devices already as well, so
it seems redundant?

Anyway, that's something that can be looked at later.

>  
>      panic("Failed to load OS from hard disk\n");
>      return 0; /* make compiler happy */

Reviewed-by: Cornelia Huck <cohuck@redhat.com>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]