Re: [Qemu-devel] [qemu-s390x] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Collin L. Walling]

On 02/22/2018 10:44 AM, Christian Borntraeger wrote:
> On 02/22/2018 04:40 PM, Collin L. Walling wrote:
> > On 02/22/2018 07:23 AM, Viktor Mihajlovski wrote:
> > > On 22.02.2018 12:51, Christian Borntraeger wrote:
> > > > Series
> > > > Acked-by: Christian Borntraeger <address@hidden>
> > Thanks!!!
> >
> > > > menu on scsi and dasd bootmaps tested successfully.
> > > >
> > > > There is one thing that we might want to fix (can be an addon patch since this 
> > > > is a non-customer
> > > > scenario (no libvirt)).
> > > >
> > > > If you start QEMU manually without a bootindex, the -boot menu=on is ignored
> > > > if no drive has a bootindex.
> > > >
> > > > For example:
> > > >
> > > > -drive file=/dev/dasda,if=none,id=d1 -device 
> > > > virtio-blk-ccw,drive=d1,bootindex=1 -boot menu=on
> > > > does work
> > > >
> > > > -drive file=/dev/dasda -boot menu=on
> > > > does not
> > > >
> > > > instead it prints:
> > > > qemu-system-s390x: boot menu is not supported for this device type.
> > > >
> > > > and the boots up the default entry.
> > > That should indeed be a separate patch, as it would move logic currently
> > > in the BIOS up to QEMU (find the first defined virtio disk and select it
> > > as boot disk).
> > > In fact it's more complicated than that, because it would have to
> > > properly account for -boot order=[acdn] and produce the respective IPLB.
> > > While it makes sense, I wouldn't rush that in but rather change the
> > > error message to indicate that -device bootindex is needed to activate
> > > the menu, at least for the time being.
> > > [...]
> > I can look into it.  Theoretically, the easier fix should just involve parsing 
> > all
> > of the -device commands and looking for a "bootindex=1" field. The Qemu options
> > code already handles a bulk of this work, so it's just a matter of putting it 
> > all
> > together.
> >
> > Shall I whip something up and post what I have as a reply to this email chain?
> In fact, it should already be there.
>
> static bool s390_gen_initial_iplb(S390IPLState *ipl)
> {
>      DeviceState *dev_st;
>
>      dev_st = get_boot_device(0);
>
> --> if this returns 0 we have no bootindex statement anywhere and the BIOS will 
> IPL the default
> disk.
Makes sense.  I'm working on making this patch look as clean as 
possible. The fact that no boot menu
options present means we fallback to using zipl values for CCW being 
tied into the switch statement
is making things a bit tricky. Just have to think the logic through a 
bit.  Will get back to you once
I have something good.

--
- Collin L Walling