Re: [Qemu-devel] [PATCH] Add args for quiet and prompt

[Anthony Liguori <anthony@codemonkey.ws>]

Cory Fields wrote:
> Syntax is -boot [cad],prompt=x,quiet=x where x is [0,1,true,false]
> Boot drive is mandatory
> If prompt is set user will see "Press f12 for boot menu"
> This behavior has changed, it is now off by default
>
> If quiet is set user will see no normal text output from bios.
> Errors will stil be displayed.
>
> Signed-off-by: Cory Fields <FOSS@AtlasTechnologiesInc.com>
> ---
>  vl.c |   48 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 3676537..b2da109 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -230,6 +230,9 @@ int no_shutdown = 0;
>  int cursor_hide = 1;
>  int graphic_rotate = 0;
>  int daemonize = 0;
> +int biosprompt = 0;
> +int quietbios = 0;
> +
>  const char *option_rom[MAX_OPTION_ROMS];
>  int nb_option_roms;
>  int semihosting_enabled = 0;
> @@ -3875,6 +3878,10 @@ static void help(int exitcode)
>             "-sd file        use 'file' as SecureDigital card image\n"
>             "-pflash file    use 'file' as a parallel flash image\n"
>             "-boot [a|c|d|n] boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)\n"
> +           "      [,quiet=true|false]\n"
> +           "                hide bios text [default=false]\n"
> +           "      [,prompt=true|false]\n"
> +           "                hide boot menu prompt [default=true]\n"
>             "-snapshot       write to temporary files instead of disk image files\n"
>             "-m megs         set virtual RAM size to megs MB [default=%d]\n"
>  #ifndef _WIN32
> @@ -4556,7 +4563,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[32] = "";
>      DisplayState *ds;
>      DisplayChangeListener *dcl;
>      int cyls, heads, secs, translation;
> @@ -4829,14 +4836,15 @@ 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;
> +                    int num_drive_letters = 0;  
> +                    char boot_opts[128];
>                      
>                      boot_devices_bitmap = 0;
> -                    for (p = boot_devices; *p != '\0'; p++) {
> +                    for (p = optarg; *p != '\0' && *p != ','; p++) {
>                          /* Allowed boot devices are:
>                           * a b     : floppy disk drives
>                           * c ... f : IDE disk drives
> @@ -4856,6 +4864,38 @@ int main(int argc, char **argv, char **envp)
>                              exit(1);
>                          }
>                          boot_devices_bitmap |= 1 << (*p - 'a');
> +                        boot_devices[num_drive_letters]=*p;
> +                        num_drive_letters++;
> +
> +                    }
> +                    
> +                    strcpy(boot_opts,optarg);
>   

snprintf(), optarg has no bounds and boot_opts does.

> +                    if (*p == ',') p++;
> +                    if (get_param_value(boot_opts, sizeof(boot_opts), "quiet", p)) {
> +                      if (!strcmp(boot_opts, "true") || !strcmp(boot_opts, "1")) {
> +                        quietbios=1;
> +                        biosprompt=0;
> +                         }
>   

The whitespace looks a little weird to me.

> +                      else if (!strcmp(boot_opts, "false") || !strcmp(boot_opts, "0")) {
> +                        quietbios=0;
> +                      }
> +                      else {
> +                        printf("Boot option not recognized\n");
> +                        exit(1);
> +                      }
> +                    }
> +
> +                    if (get_param_value(boot_opts, sizeof(boot_opts), "prompt", p)){
> +                      if (!strcmp(boot_opts, "true") || !strcmp(boot_opts, "1")) {
> +                        biosprompt=1;
> +                      }
> +                      else if (!strcmp(boot_opts, "false") || !strcmp(boot_opts, "0"))  {
> +                        biosprompt=0;
> +                      }
> +                      else {
> +                        printf("Boot option not recognized\n");
> +                        exit(1);
> +                      }
>                      }
>   

This all looks like 2-space tabs, QEMU uses 4-space tabs.  I'd suggest 
factoring this out into a separate function too.

Regards,

Anthony Liguori

>                  }
>                  break;
> @@ -5343,7 +5383,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);
>  
>