On 02/23/2018 11:07 AM, Thomas Huth wrote: > On 22.02.2018 20:40, Collin L. Walling wrote: >> 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) > > Patch looks fine to me. I can either take it directly like this, or in > case you have to respin (depends on the problem that Christian reported > with the Ubuntu guest), I'm also fine if you squash it into an earlier > patch instead.
FWIW, my problem (a menu happens even without -boot menu=on or loadparm) also happens with other guests (e.g. fedora).