Re: [Qemu-devel] [qemu-s390x] [PATCH v1 3/5] s390-ccw: parse and set boot menu options

[Collin L. Walling]

On 11/28/2017 06:04 AM, Cornelia Huck wrote:
> On Mon, 27 Nov 2017 15:55:34 -0500
> "Collin L. Walling" <address@hidden> wrote:
>
> > Set boot menu options for an s390 guest and store them in
> > the iplb. These options are set via the QEMU command line
> > option:
> >
> >      -boot menu=on|off[,splash-time=X]
> >
> > or via the libvirt domain xml:
> >
> >      <os>
> >        <bootmenu enable='yes|no' timeout='X'/>
> >      </os>
> >
> > Where X represents some positive integer representing
> > milliseconds.
> >
> > Signed-off-by: Collin L. Walling <address@hidden>
> > Reviewed-by: Janosch Frank <address@hidden>
> > ---
> >   hw/s390x/ipl.c          | 23 +++++++++++++++++++++++
> >   hw/s390x/ipl.h          |  8 ++++++--
> >   pc-bios/s390-ccw/iplb.h |  8 ++++++--
> >   3 files changed, 35 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> > index 0d06fc1..0ca9e37 100644
> > --- a/hw/s390x/ipl.c
> > +++ b/hw/s390x/ipl.c
> > @@ -23,6 +23,8 @@
> >   #include "hw/s390x/ebcdic.h"
> >   #include "ipl.h"
> >   #include "qemu/error-report.h"
> > +#include "qemu/config-file.h"
> > +#include "qemu/cutils.h"
> >   
> >   #define KERN_IMAGE_START                0x010000UL
> >   #define KERN_PARM_AREA                  0x010480UL
> > @@ -219,6 +221,23 @@ static Property s390_ipl_properties[] = {
> >       DEFINE_PROP_END_OF_LIST(),
> >   };
> >   
> > +static void s390_ipl_set_boot_menu(uint8_t *boot_menu_enabled,
> > +                                   uint16_t *boot_menu_timeout)
> > +{
> > +    QemuOptsList *plist = qemu_find_opts("boot-opts");
> > +    QemuOpts *opts = QTAILQ_FIRST(&plist->head);
> > +    const char *p;
> > +    long result = 0;
> > +
> > +    *boot_menu_enabled = qemu_opt_get_bool(opts, "menu", false);
> This is already parsed in vl.c, isn't it? Can you simply check
> boot_menu?


Well then... how convenient! :)

> > +
> > +    if (*boot_menu_enabled) {
> > +        p = qemu_opt_get(opts, "splash-time");
> hw/nvram/fw_cfg.c parses splash-time as well, but I'm not sure if it is
> worth parsing it in vl.c as well.

It's probably best we parse it ourselves.  I see that fw_cfg.c also does
some validation on the splash-time value, so I'll add something similar.

> > +        qemu_strtol(p, NULL, 10, &result);
> > +        *boot_menu_timeout = result;
> > +    }
> > +}
> > +
> >   static bool s390_gen_initial_iplb(S390IPLState *ipl)
> >   {
> >       DeviceState *dev_st;


--
- Collin L Walling