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

[Collin L. Walling]

On 02/22/2018 11:45 AM, Collin L. Walling wrote:
> 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.
This should do the trick (this can also be squished painlessly into 6/13 
if desired):

From dea9f22429cb3e4ccc81980974adc9f55ead87bf Mon Sep 17 00:00:00 2001
From: "Collin L. Walling" <address@hidden>
Date: Thu, 22 Feb 2018 14:26:14 -0500
Subject: [PATCH] s390-ccw: set boot menu options iff a bootindex was 
provided

The boot menu options should be set iff a device was specified
with bootindex=1, otherwise an error message will appear claiming
that the boot menu is not available for a particular device type.
This can be misleading.

To amend this, only set the boot menu options when

    -device ...,bootindex=1

is provded on the command line for a valid IPL device. The absence
of a bootindex provided with -boot menu=on will display an error
message, and -boot menu=off (or absence of -boot options) will
return silently.

Signed-off-by: Collin L. Walling <address@hidden>
---
 hw/s390x/ipl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 566248e..edbec90 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -231,6 +231,14 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
     const char *tmp;
     unsigned long splash_time = 0;

+    if (!get_boot_device(0)) {
+        if (boot_menu) {
+            error_report("boot menu requires a bootindex to be 
specified for "
+                         "the IPL device.");
+        }
+        return;
+    }
+
     switch (ipl->iplb.pbt) {
     case S390_IPL_TYPE_CCW:
         /* In the absence of -boot menu, use zipl parameters */
--
2.7.4

--
- Collin L Walling