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 <borntrae...@de.ibm.com>
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" <wall...@linux.vnet.ibm.com>
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 <wall...@linux.vnet.ibm.com>
---
 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


Reply via email to