Patchwork [v3,1/2] Generalize -machine command line option

login
register
mail settings
Submitter Jan Kiszka
Date May 25, 2011, 7:03 a.m.
Message ID <4DDCA9BE.1050801@web.de>
Download mbox | patch
Permalink /patch/97354/
State New
Headers show

Comments

Jan Kiszka - May 25, 2011, 7:03 a.m.
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.

CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Ian Campbell <ijc@hellion.org.uk>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

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(-)
Ian Campbell - May 25, 2011, 8:17 a.m.
On Wed, 2011-05-25 at 09:03 +0200, 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.
> 
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Ian Campbell <ijc@hellion.org.uk>

Thanks Jan, this one works for me.

Tested-by: Ian Campbell <ian.campbell@citrix.com>

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> 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 5d7ffa2..01751b4 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -452,9 +452,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 82e085a..0dbc028 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2033,13 +2033,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 b362871..1865a8d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1891,6 +1891,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;
> @@ -2144,20 +2165,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 */
> @@ -2675,11 +2683,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;
> @@ -2941,8 +2950,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);
Jan Kiszka - June 7, 2011, 3:58 p.m.
On 2011-05-25 09:03, 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.

Ping (and also for
http://thread.gmane.org/gmane.comp.emulators.qemu/103348).

Jan

> 
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Ian Campbell <ijc@hellion.org.uk>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> 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 5d7ffa2..01751b4 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -452,9 +452,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 82e085a..0dbc028 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2033,13 +2033,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 b362871..1865a8d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1891,6 +1891,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;
> @@ -2144,20 +2165,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 */
> @@ -2675,11 +2683,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;
> @@ -2941,8 +2950,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);

Patch

diff --git a/qemu-config.c b/qemu-config.c
index 5d7ffa2..01751b4 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -452,9 +452,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 82e085a..0dbc028 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2033,13 +2033,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 b362871..1865a8d 100644
--- a/vl.c
+++ b/vl.c
@@ -1891,6 +1891,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;
@@ -2144,20 +2165,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 */
@@ -2675,11 +2683,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;
@@ -2941,8 +2950,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);