Re: [Qemu-devel] [RESEND][PATCH v3] Generalize -machine command line option

[Anthony Liguori <anthony@codemonkey.ws>]

On 07/23/2011 05:38 AM, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> -machine somehow suggests that it selects the machine, but it doesn't.
> Fix that before this command is set in stone.
>
> Actually, -machine should supersede -M and allow to introduce arbitrary
> per-machine options to the command line. That will change the internal
> realization again, but we will be able to keep the user interface
> stable.
>
> Tested-by: Ian Campbell<ian.campbell@citrix.com>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>
> NOTE: This patch is a MUST HAVE for 0.15 as we otherwise set a half
> done command line interface into stone!
>
> Changes in v3:
>   - fix regression of default machine options handling, -machine xenfv
>     selects accel=xen again
>     (I really hope we can clean up the defaults, make them more
>     generally useful when switching to some QCFG.)
>
> Changes in v2:
>   - fix regression of -M my factoring out machine_parse and using it for
>     both old and new command.
>
>   qemu-config.c   |    5 +++++
>   qemu-options.hx |   20 +++++++++++++++-----
>   vl.c            |   43 ++++++++++++++++++++++++++-----------------
>   3 files changed, 46 insertions(+), 22 deletions(-)
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 93d20c6..b2ec40b 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -464,9 +464,14 @@ QemuOptsList qemu_option_rom_opts = {
>
>   static QemuOptsList qemu_machine_opts = {
>       .name = "machine",
> +    .implied_opt_name = "type",
>       .head = QTAILQ_HEAD_INITIALIZER(qemu_machine_opts.head),
>       .desc = {
>           {
> +            .name = "type",
> +            .type = QEMU_OPT_STRING,
> +            .help = "emulated machine"
> +        }, {
>               .name = "accel",
>               .type = QEMU_OPT_STRING,
>               .help = "accelerator list",
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 64114dd..195943b 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2075,13 +2075,23 @@ if KVM support is enabled when compiling.
>   ETEXI
>
>   DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
> -    "-machine accel=accel1[:accel2]    use an accelerator (kvm,xen,tcg), default is tcg\n", QEMU_ARCH_ALL)
> +    "-machine [type=]name[,prop[=value][,...]]\n"
> +    "                selects emulated machine (-machine ? for list)\n"
> +    "                property accel=accel1[:accel2[:...]] selects accelerator\n"
> +    "                supported accelerators are kvm, xen, tcg (default: tcg)\n",
> +    QEMU_ARCH_ALL)
>   STEXI
> -@item -machine accel=@var{accels}
> +@item -machine [type=]@var{name}[,prop=@var{value}[,...]]
>   @findex -machine
> -This is use to enable an accelerator, in kvm,xen,tcg.
> -By default, it use only tcg. If there a more than one accelerator
> -specified, the next one is used if the first don't work.
> +Select the emulated machine by @var{name}. Use @code{-machine ?} to list
> +available machines. Supported machine properties are:
> +@table @option
> +@item accel=@var{accels1}[:@var{accels2}[:...]]
> +This is used to enable an accelerator. Depending on the target architecture,
> +kvm, xen, or tcg can be available. By default, tcg is used. If there is more
> +than one accelerator specified, the next one is used if the previous one fails
> +to initialize.
> +@end table
>   ETEXI
>
>   DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
> diff --git a/vl.c b/vl.c
> index fcd7395..acfff85 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1899,6 +1899,27 @@ static int debugcon_parse(const char *devname)
>       return 0;
>   }
>
> +static QEMUMachine *machine_parse(const char *name)
> +{
> +    QEMUMachine *m, *machine = NULL;
> +
> +    if (name) {
> +        machine = find_machine(name);
> +    }
> +    if (machine) {
> +        return machine;
> +    }
> +    printf("Supported machines are:\n");
> +    for (m = first_machine; m != NULL; m = m->next) {
> +        if (m->alias) {
> +            printf("%-10s %s (alias of %s)\n", m->alias, m->desc, m->name);
> +        }
> +        printf("%-10s %s%s\n", m->name, m->desc,
> +               m->is_default ? " (default)" : "");
> +    }
> +    exit(!name || *name != '?');
> +}
> +
>   static int tcg_init(void)
>   {
>       return 0;
> @@ -2155,20 +2176,7 @@ int main(int argc, char **argv, char **envp)
>               }
>               switch(popt->index) {
>               case QEMU_OPTION_M:
> -                machine = find_machine(optarg);
> -                if (!machine) {
> -                    QEMUMachine *m;
> -                    printf("Supported machines are:\n");
> -                    for(m = first_machine; m != NULL; m = m->next) {
> -                        if (m->alias)
> -                            printf("%-10s %s (alias of %s)\n",
> -                                   m->alias, m->desc, m->name);
> -                        printf("%-10s %s%s\n",
> -                               m->name, m->desc,
> -                               m->is_default ? " (default)" : "");
> -                    }
> -                    exit(*optarg != '?');
> -                }
> +                machine = machine_parse(optarg);
>                   break;
>               case QEMU_OPTION_cpu:
>                   /* hw initialization will check this */
> @@ -2698,11 +2706,12 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_machine:
>                   olist = qemu_find_opts("machine");
>                   qemu_opts_reset(olist);
> -                opts = qemu_opts_parse(olist, optarg, 0);
> +                opts = qemu_opts_parse(olist, optarg, 1);
>                   if (!opts) {
>                       fprintf(stderr, "parse error: %s\n", optarg);
>                       exit(1);
>                   }
> +                machine = machine_parse(qemu_opt_get(opts, "type"));
>                   break;
>               case QEMU_OPTION_usb:
>                   usb_enabled = 1;
> @@ -2976,8 +2985,8 @@ int main(int argc, char **argv, char **envp)
>               p = qemu_opt_get(QTAILQ_FIRST(&list->head), "accel");
>           }
>           if (p == NULL) {
> -            opts = qemu_opts_parse(qemu_find_opts("machine"),
> -                                   machine->default_machine_opts, 0);
> +            qemu_opts_reset(list);
> +            opts = qemu_opts_parse(list, machine->default_machine_opts, 0);
>               if (!opts) {
>                   fprintf(stderr, "parse error for machine %s: %s\n",
>                           machine->name, machine->default_machine_opts);
>
>