Re: [Qemu-devel] [PATCH 1/2] Rework -boot option

[Anthony Liguori <anthony@codemonkey.ws>]

Jan Kiszka wrote:
> This patch changes the boot command line option to the canonical format
>
>  -boot [order=drives][,interactive=on|off]
>
> where 'drives' is using the same format as the old -boot. The new format
> is required in order to add the 'interactive' option and use the
> existing infrastructure to parse it.
>
> The state of 'interactive' is transfered to the firmware via the new
> configuration value FW_CFG_BOOT_INTERACTIVE.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>   

This breaks compatibility with existing management tools.  Can't we do:

 -boot [order=]drives[,interactive=on|off]

Certainly, -drive introduces a precedence for this type of format.

Regards,

Anthony Liguori

> ---
>
>  hw/fw_cfg.c     |    1 +
>  hw/fw_cfg.h     |    1 +
>  qemu-options.hx |   19 ++++++++++---
>  sysemu.h        |    1 +
>  vl.c            |   80 +++++++++++++++++++++++++++++++++++++------------------
>  5 files changed, 72 insertions(+), 30 deletions(-)
>
> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> index e1b19d7..c21cdb3 100644
> --- a/hw/fw_cfg.c
> +++ b/hw/fw_cfg.c
> @@ -279,6 +279,7 @@ void *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>      fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
>      fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)nographic);
>      fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> +    fw_cfg_add_i16(s, FW_CFG_BOOT_INTERACTIVE, (uint16_t)boot_interactive);
>  
>      register_savevm("fw_cfg", -1, 1, fw_cfg_save, fw_cfg_load, s);
>      qemu_register_reset(fw_cfg_reset, s);
> diff --git a/hw/fw_cfg.h b/hw/fw_cfg.h
> index 41a3dd0..131dbf2 100644
> --- a/hw/fw_cfg.h
> +++ b/hw/fw_cfg.h
> @@ -14,6 +14,7 @@
>  #define FW_CFG_INITRD_ADDR      0x0a
>  #define FW_CFG_INITRD_SIZE      0x0b
>  #define FW_CFG_BOOT_DEVICE      0x0c
> +#define FW_CFG_BOOT_INTERACTIVE 0x0d
>  #define FW_CFG_MAX_ENTRY        0x10
>  
>  #define FW_CFG_WRITE_CHANNEL    0x4000
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 4d15f9b..4cfe199 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -210,11 +210,22 @@ Use 'file' as a parallel flash image.
>  ETEXI
>  
>  DEF("boot", HAS_ARG, QEMU_OPTION_boot,
> -    "-boot [a|c|d|n] boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)\n")
> +    "-boot [order=drives][,interactive=on|off]\n"
> +    "                'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)\n")
>  STEXI
> -@item -boot [a|c|d|n]
> -Boot on floppy (a), hard disk (c), CD-ROM (d), or Etherboot (n). Hard disk boot
> -is the default.
> +@item -boot [order=@var{drives}][,interactive=on|off]
> +
> +Specify boot order @var{drives} as a string of drive letters. Valid
> +drive letters depend on the target achitecture. The x86 PC uses: a, b
> +(floppy 1 and 2), c (first hard disk), d (first CD-ROM), n-p (Etherboot
> +from network adapter 1-4), hard disk boot is the default. Furthermore,
> +interactive boot menus/prompts can be enabled as far as firmware/BIOS
> +supports them. The default is non-interactive boot.
> +
> +@example
> +# try to boot from network first, then from hard disk
> +qemu -boot order=nc
> +@end example
>  ETEXI
>  
>  DEF("snapshot", 0, QEMU_OPTION_snapshot,
> diff --git a/sysemu.h b/sysemu.h
> index 2b1677c..5aed865 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -103,6 +103,7 @@ extern int graphic_rotate;
>  extern int no_quit;
>  extern int semihosting_enabled;
>  extern int old_param;
> +extern int boot_interactive;
>  
>  #ifdef USE_KQEMU
>  extern int kqemu_allowed;
> diff --git a/vl.c b/vl.c
> index 72ea0e4..5b2f2c8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -264,6 +264,7 @@ const char *prom_envs[MAX_PROM_ENVS];
>  #endif
>  int nb_drives_opt;
>  struct drive_opt drives_opt[MAX_DRIVES];
> +int boot_interactive;
>  
>  static CPUState *cur_cpu;
>  static CPUState *next_cpu;
> @@ -4254,7 +4255,7 @@ int main(int argc, char **argv, char **envp)
>      int snapshot, linux_boot, net_boot;
>      const char *initrd_filename;
>      const char *kernel_filename, *kernel_cmdline;
> -    const char *boot_devices = "";
> +    char boot_devices[33] = "";
>      DisplayState *ds;
>      DisplayChangeListener *dcl;
>      int cyls, heads, secs, translation;
> @@ -4529,33 +4530,60 @@ int main(int argc, char **argv, char **envp)
>                  drive_add(optarg, CDROM_ALIAS);
>                  break;
>              case QEMU_OPTION_boot:
> -                boot_devices = optarg;
> -                /* We just do some generic consistency checks */
>                  {
> -                    /* Could easily be extended to 64 devices if needed */
> -                    const char *p;
> -                    
> -                    boot_devices_bitmap = 0;
> -                    for (p = boot_devices; *p != '\0'; p++) {
> -                        /* Allowed boot devices are:
> -                         * a b     : floppy disk drives
> -                         * c ... f : IDE disk drives
> -                         * g ... m : machine implementation dependant drives
> -                         * n ... p : network devices
> -                         * It's up to each machine implementation to check
> -                         * if the given boot devices match the actual hardware
> -                         * implementation and firmware features.
> -                         */
> -                        if (*p < 'a' || *p > 'q') {
> -                            fprintf(stderr, "Invalid boot device '%c'\n", *p);
> -                            exit(1);
> +                    static const char * const params[] = {
> +                        "order", "interactive", NULL
> +                    };
> +                    char buf[16];
> +
> +                    if (check_params(buf, sizeof(buf), params, optarg) < 0) {
> +                        fprintf(stderr,
> +                                "qemu: unknown boot parameter '%s' in '%s'\n",
> +                                buf, optarg);
> +                        exit(1);
> +                    }
> +
> +                    if (get_param_value(boot_devices, sizeof(boot_devices),
> +                                        "order", optarg)) {
> +                        /* We just do some generic consistency checks */
> +                        const char *p;
> +
> +                        boot_devices_bitmap = 0;
> +                        for (p = boot_devices; *p != '\0'; p++) {
> +                            /* Allowed boot devices are:
> +                             * a-b: floppy disk drives
> +                             * c-f: IDE disk drives
> +                             * g-m: machine implementation dependant drives
> +                             * n-p: network devices
> +                             * It's up to each machine implementation to check
> +                             * if the given boot devices match the actual
> +                             * hardware implementation and firmware features.
> +                             */
> +                            if (*p < 'a' || *p > 'p') {
> +                                fprintf(stderr, "Invalid boot device '%c'\n",
> +                                        *p);
> +                                exit(1);
> +                            }
> +                            if (boot_devices_bitmap & (1 << (*p - 'a'))) {
> +                                fprintf(stderr,
> +                                        "Boot device '%c' was given twice\n",
> +                                        *p);
> +                                exit(1);
> +                             }
> +                             boot_devices_bitmap |= 1 << (*p - 'a');
>                          }
> -                        if (boot_devices_bitmap & (1 << (*p - 'a'))) {
> -                            fprintf(stderr,
> -                                    "Boot device '%c' was given twice\n",*p);
> -                            exit(1);
> +                    }
> +                    if (get_param_value(buf, sizeof(buf), "interactive",
> +                                        optarg)) {
> +                        if (!strcmp(buf, "on")) {
> +                            boot_interactive = 1;
> +                        } else if (!strcmp(buf, "off")) {
> +                            boot_interactive = 0;
> +                        } else {
> +                           fprintf(stderr,
> +                                   "qemu: invalid option value '%s'\n", buf);
> +                           exit(1);
>                          }
> -                        boot_devices_bitmap |= 1 << (*p - 'a');
>                      }
>                  }
>                  break;
> @@ -5074,7 +5102,7 @@ int main(int argc, char **argv, char **envp)
>  
>      /* boot to floppy or the default cd if no hard disk defined yet */
>      if (!boot_devices[0]) {
> -        boot_devices = "cad";
> +        strcpy(boot_devices, "cad");
>      }
>      setvbuf(stdout, NULL, _IOLBF, 0);
>  
>
>
>
>