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

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

[Jan Kiszka]

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>
---

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);

[Qemu-devel] [RESEND][PATCH 2/2] Deprecate -M command line options

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Superseded by -machine. Therefore, this patch removes -M from the help
list and pushes -machine at the same place in the output.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-options.hx |   45 ++++++++++++++++++++-------------------------
 1 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 195943b..1233f83 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -27,14 +27,29 @@ STEXI
 Display version information and exit
 ETEXI
 
-DEF("M", HAS_ARG, QEMU_OPTION_M,
-    "-M machine      select emulated machine (-M ? for list)\n", QEMU_ARCH_ALL)
+DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
+    "-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 -M @var{machine}
-@findex -M
-Select the emulated @var{machine} (@code{-M ?} for list)
+@item -machine [type=]@var{name}[,prop=@var{value}[,...]]
+@findex -machine
+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
 
+HXCOMM Deprecated by -machine
+DEF("M", HAS_ARG, QEMU_OPTION_M, "", QEMU_ARCH_ALL)
+
 DEF("cpu", HAS_ARG, QEMU_OPTION_cpu,
     "-cpu cpu        select CPU (-cpu ? for list)\n", QEMU_ARCH_ALL)
 STEXI
@@ -2074,26 +2089,6 @@ Enable KVM full virtualization support. This option is only available
 if KVM support is enabled when compiling.
 ETEXI
 
-DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
-    "-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 [type=]@var{name}[,prop=@var{value}[,...]]
-@findex -machine
-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,
     "-xen-domid id   specify xen guest domain id\n", QEMU_ARCH_ALL)
 DEF("xen-create", 0, QEMU_OPTION_xen_create,
-- 
1.7.3.4

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

[Anthony Liguori]

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);
>
>

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

[Richard W.M. Jones]

On Sat, Jul 23, 2011 at 12:38:37PM +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.

This breaks libguestfs which was doing:

  qemu -machine accel=kvm:tcg ...

We are not passing any -M option at all.  We don't particularly care
about the machine type since we're not that performance sensitive and
we don't need to serialize the machine state.

I have checked, and this works:

  qemu -machine pc,accel=kvm:tcg ...

"pc" is the default, right?  What about for other architectures?

Please add qemu capabilities, so we can reasonably detect what an
unknown qemu binary supports and so we don't need to do endless
parsing of the -help output and guesswork.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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

[Jan Kiszka]

On 2011-07-25 11:41, Richard W.M. Jones wrote:
> On Sat, Jul 23, 2011 at 12:38:37PM +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.
> 
> This breaks libguestfs which was doing:
> 
>   qemu -machine accel=kvm:tcg ...
> 
> We are not passing any -M option at all.  We don't particularly care
> about the machine type since we're not that performance sensitive and
> we don't need to serialize the machine state.
> 
> I have checked, and this works:
> 
>   qemu -machine pc,accel=kvm:tcg ...
> 
> "pc" is the default, right?  What about for other architectures?

Yes, pc is the right default. Other arch have other defaults.

> 
> Please add qemu capabilities, so we can reasonably detect what an
> unknown qemu binary supports and so we don't need to do endless
> parsing of the -help output and guesswork.

This syntax was not yet released (but will be with 0.15, so I was
pushing this). Therefore, nothing was "officially" broken by this patch.

I'm sorry if you may have released any libguestfs with the transient
syntax, but my patches were waiting quite a while for being merged since
the introduction of -machine.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Richard W.M. Jones]

On Mon, Jul 25, 2011 at 12:33:01PM +0200, Jan Kiszka wrote:
> On 2011-07-25 11:41, Richard W.M. Jones wrote:
> > On Sat, Jul 23, 2011 at 12:38:37PM +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.
> > 
> > This breaks libguestfs which was doing:
> > 
> >   qemu -machine accel=kvm:tcg ...
> > 
> > We are not passing any -M option at all.  We don't particularly care
> > about the machine type since we're not that performance sensitive and
> > we don't need to serialize the machine state.
> > 
> > I have checked, and this works:
> > 
> >   qemu -machine pc,accel=kvm:tcg ...
> > 
> > "pc" is the default, right?  What about for other architectures?
> 
> Yes, pc is the right default. Other arch have other defaults.

So what you're saying is we have to parse qemu -machine \? output by
looking for the string '(default)'?  eg:

$ ./arm-softmmu/qemu-system-arm -machine \?|fgrep '(default)'
integratorcp ARM Integrator/CP (ARM926EJ-S) (default)

$ ./i386-softmmu/qemu -machine \?|fgrep '(default)'
pc-0.14    Standard PC (default)

> > Please add qemu capabilities, so we can reasonably detect what an
> > unknown qemu binary supports and so we don't need to do endless
> > parsing of the -help output and guesswork.
> 
> This syntax was not yet released (but will be with 0.15, so I was
> pushing this). Therefore, nothing was "officially" broken by this patch.
> 
> I'm sorry if you may have released any libguestfs with the transient
> syntax, but my patches were waiting quite a while for being merged since
> the introduction of -machine.

That's an excuse, not a practical solution.  We have to be able to
work with any qemu.  eg. the qemu in current Fedora Rawhide which
supports only -machine accel=, or qemu in other distros which are also
branched from arbitrary git releases, or qemu that people compile
themselves.

Parsing -help output and guesswork isn't scalable, and this is not
exactly the first time that people have complained about this.

(Yes, libvirt and libguestfs do allow callers to mechanically query
their respective APIs for capabilities.)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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

[Jan Kiszka]

On 2011-07-25 12:45, Richard W.M. Jones wrote:
> On Mon, Jul 25, 2011 at 12:33:01PM +0200, Jan Kiszka wrote:
>> On 2011-07-25 11:41, Richard W.M. Jones wrote:
>>> On Sat, Jul 23, 2011 at 12:38:37PM +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.
>>>
>>> This breaks libguestfs which was doing:
>>>
>>>   qemu -machine accel=kvm:tcg ...
>>>
>>> We are not passing any -M option at all.  We don't particularly care
>>> about the machine type since we're not that performance sensitive and
>>> we don't need to serialize the machine state.
>>>
>>> I have checked, and this works:
>>>
>>>   qemu -machine pc,accel=kvm:tcg ...
>>>
>>> "pc" is the default, right?  What about for other architectures?
>>
>> Yes, pc is the right default. Other arch have other defaults.
> 
> So what you're saying is we have to parse qemu -machine \? output by
> looking for the string '(default)'?  eg:
> 
> $ ./arm-softmmu/qemu-system-arm -machine \?|fgrep '(default)'
> integratorcp ARM Integrator/CP (ARM926EJ-S) (default)
> 
> $ ./i386-softmmu/qemu -machine \?|fgrep '(default)'
> pc-0.14    Standard PC (default)

I understand, this is clumsy. Will see if we can do better.

> 
>>> Please add qemu capabilities, so we can reasonably detect what an
>>> unknown qemu binary supports and so we don't need to do endless
>>> parsing of the -help output and guesswork.
>>
>> This syntax was not yet released (but will be with 0.15, so I was
>> pushing this). Therefore, nothing was "officially" broken by this patch.
>>
>> I'm sorry if you may have released any libguestfs with the transient
>> syntax, but my patches were waiting quite a while for being merged since
>> the introduction of -machine.
> 
> That's an excuse, not a practical solution.  We have to be able to
> work with any qemu.  eg. the qemu in current Fedora Rawhide which
> supports only -machine accel=, or qemu in other distros which are also
> branched from arbitrary git releases, or qemu that people compile
> themselves.

In principle, this is first of all a Rawhide problem. Upstream really
can't babysit every distro doing crazy things with arbitrary devel
snapshots. These patches were public, and the maintainers had a fair
chance to realize that the interface was not yet set in stone.

> 
> Parsing -help output and guesswork isn't scalable, and this is not
> exactly the first time that people have complained about this.

I agree. That's why we try hard to release stable interfaces and then
maintain them.

> 
> (Yes, libvirt and libguestfs do allow callers to mechanically query
> their respective APIs for capabilities.)

Maybe Anthony's (Liguori) rework of the QEMU config interfaces will
provide a better reflections, haven't checked. But for now you need to
stick with this model, specifically when you want to maintain all the
distro forks.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Markus Armbruster]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 2011-07-25 12:45, Richard W.M. Jones wrote:
>> On Mon, Jul 25, 2011 at 12:33:01PM +0200, Jan Kiszka wrote:
>>> On 2011-07-25 11:41, Richard W.M. Jones wrote:
>>>> On Sat, Jul 23, 2011 at 12:38:37PM +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.
>>>>
>>>> This breaks libguestfs which was doing:
>>>>
>>>>   qemu -machine accel=kvm:tcg ...
>>>>
>>>> We are not passing any -M option at all.  We don't particularly care
>>>> about the machine type since we're not that performance sensitive and
>>>> we don't need to serialize the machine state.
>>>>
>>>> I have checked, and this works:
>>>>
>>>>   qemu -machine pc,accel=kvm:tcg ...
>>>>
>>>> "pc" is the default, right?  What about for other architectures?
>>>
>>> Yes, pc is the right default. Other arch have other defaults.
>> 
>> So what you're saying is we have to parse qemu -machine \? output by
>> looking for the string '(default)'?  eg:
>> 
>> $ ./arm-softmmu/qemu-system-arm -machine \?|fgrep '(default)'
>> integratorcp ARM Integrator/CP (ARM926EJ-S) (default)
>> 
>> $ ./i386-softmmu/qemu -machine \?|fgrep '(default)'
>> pc-0.14    Standard PC (default)
>
> I understand, this is clumsy. Will see if we can do better.

Is there a technical reason why type isn't optional with -machine?

[...]

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

[Peter Maydell]

On 25 July 2011 11:45, Richard W.M. Jones <rjones@redhat.com> wrote:
> So what you're saying is we have to parse qemu -machine \? output by
> looking for the string '(default)'?  eg:
>
> $ ./arm-softmmu/qemu-system-arm -machine \?|fgrep '(default)'
> integratorcp ARM Integrator/CP (ARM926EJ-S) (default)

For ARM you absolutely should not be relying on the default
machine type (not least because it's an incredibly ancient
dev board which nobody uses any more). An ARM kernel is
generally fairly specific to the hardware platform being
emulated, so you should know which machine you're intending
to run on and specify it explicitly.

-- PMM

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

[Jan Kiszka]

On 2011-07-25 13:39, Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> On 2011-07-25 12:45, Richard W.M. Jones wrote:
>>> On Mon, Jul 25, 2011 at 12:33:01PM +0200, Jan Kiszka wrote:
>>>> On 2011-07-25 11:41, Richard W.M. Jones wrote:
>>>>> On Sat, Jul 23, 2011 at 12:38:37PM +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.
>>>>>
>>>>> This breaks libguestfs which was doing:
>>>>>
>>>>>   qemu -machine accel=kvm:tcg ...
>>>>>
>>>>> We are not passing any -M option at all.  We don't particularly care
>>>>> about the machine type since we're not that performance sensitive and
>>>>> we don't need to serialize the machine state.
>>>>>
>>>>> I have checked, and this works:
>>>>>
>>>>>   qemu -machine pc,accel=kvm:tcg ...
>>>>>
>>>>> "pc" is the default, right?  What about for other architectures?
>>>>
>>>> Yes, pc is the right default. Other arch have other defaults.
>>>
>>> So what you're saying is we have to parse qemu -machine \? output by
>>> looking for the string '(default)'?  eg:
>>>
>>> $ ./arm-softmmu/qemu-system-arm -machine \?|fgrep '(default)'
>>> integratorcp ARM Integrator/CP (ARM926EJ-S) (default)
>>>
>>> $ ./i386-softmmu/qemu -machine \?|fgrep '(default)'
>>> pc-0.14    Standard PC (default)
>>
>> I understand, this is clumsy. Will see if we can do better.
> 
> Is there a technical reason why type isn't optional with -machine?
> 
> [...]

Maybe it's just the

assert(!permit_abbrev || list->implied_opt_name);

in qemu_opts_parse, but I haven't looked at all details (and all other
users) yet.

>From -machine POV, I see no reason that prevents defaulting to some
machine type if "[type=]XXX" is missing in the command line.

Jan

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Peter Maydell]

On 25 July 2011 12:48, Peter Maydell <peter.maydell@linaro.org> wrote:
> For ARM you absolutely should not be relying on the default
> machine type (not least because it's an incredibly ancient
> dev board which nobody uses any more). An ARM kernel is
> generally fairly specific to the hardware platform being
> emulated, so you should know which machine you're intending
> to run on and specify it explicitly.

In fact having thought about it a bit I'm going to go further
and say that the whole idea of a "default machine" is a rather
x86-centric idea -- most architectures don't really have a
single machine type that's used by just about everybody,
always has been, and isn't likely to become obsolete in the
future. So if we're reworking the command line API to
supersede "-M" then we shouldn't have a default at all.

(Consider also the possibility of eventually having a single
qemu binary that supports multiple architectures -- that
would make a 'default machine' definitely a bit odd.)

-- PMM

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

[Jan Kiszka]

On 2011-07-25 14:05, Peter Maydell wrote:
> On 25 July 2011 12:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>> For ARM you absolutely should not be relying on the default
>> machine type (not least because it's an incredibly ancient
>> dev board which nobody uses any more). An ARM kernel is
>> generally fairly specific to the hardware platform being
>> emulated, so you should know which machine you're intending
>> to run on and specify it explicitly.
> 
> In fact having thought about it a bit I'm going to go further
> and say that the whole idea of a "default machine" is a rather
> x86-centric idea -- most architectures don't really have a
> single machine type that's used by just about everybody,
> always has been, and isn't likely to become obsolete in the
> future. So if we're reworking the command line API to
> supersede "-M" then we shouldn't have a default at all.

Then you may want to drop is_default = 1 from integratorcp and prepare
the main loop to face a NULL machine.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Peter Maydell]

On 25 July 2011 13:18, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-07-25 14:05, Peter Maydell wrote:
>> In fact having thought about it a bit I'm going to go further
>> and say that the whole idea of a "default machine" is a rather
>> x86-centric idea -- most architectures don't really have a
>> single machine type that's used by just about everybody,
>> always has been, and isn't likely to become obsolete in the
>> future. So if we're reworking the command line API to
>> supersede "-M" then we shouldn't have a default at all.
>
> Then you may want to drop is_default = 1 from integratorcp and prepare
> the main loop to face a NULL machine.

We can't change the default machine for -M, that would break
backwards compatibility. All we can do is avoid having a notion
of "default machine" in new command line syntax.

-- PMM

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

[Anthony Liguori]

On 07/25/2011 05:59 AM, Jan Kiszka wrote:
> On 2011-07-25 12:45, Richard W.M. Jones wrote:
>> That's an excuse, not a practical solution.  We have to be able to
>> work with any qemu.  eg. the qemu in current Fedora Rawhide which
>> supports only -machine accel=, or qemu in other distros which are also
>> branched from arbitrary git releases, or qemu that people compile
>> themselves.
>
> In principle, this is first of all a Rawhide problem. Upstream really
> can't babysit every distro doing crazy things with arbitrary devel
> snapshots. These patches were public, and the maintainers had a fair
> chance to realize that the interface was not yet set in stone.
>
>>
>> Parsing -help output and guesswork isn't scalable, and this is not
>> exactly the first time that people have complained about this.
>
> I agree. That's why we try hard to release stable interfaces and then
> maintain them.
>
>>
>> (Yes, libvirt and libguestfs do allow callers to mechanically query
>> their respective APIs for capabilities.)
>
> Maybe Anthony's (Liguori) rework of the QEMU config interfaces will
> provide a better reflections, haven't checked. But for now you need to
> stick with this model, specifically when you want to maintain all the
> distro forks.

Yes, it will, but it doesn't fix this particular.  Problem.  Until we do 
a release, we reserve the right to change the syntax of newly introduced 
command line options.

I don't think any level of introspection changes this.

Regards,

Anthony Liguori

>
> Jan
>

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

[Jan Kiszka]

On 2011-07-25 14:22, Peter Maydell wrote:
> On 25 July 2011 13:18, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2011-07-25 14:05, Peter Maydell wrote:
>>> In fact having thought about it a bit I'm going to go further
>>> and say that the whole idea of a "default machine" is a rather
>>> x86-centric idea -- most architectures don't really have a
>>> single machine type that's used by just about everybody,
>>> always has been, and isn't likely to become obsolete in the
>>> future. So if we're reworking the command line API to
>>> supersede "-M" then we shouldn't have a default at all.
>>
>> Then you may want to drop is_default = 1 from integratorcp and prepare
>> the main loop to face a NULL machine.
> 
> We can't change the default machine for -M, that would break
> backwards compatibility. All we can do is avoid having a notion
> of "default machine" in new command line syntax.

The new syntax can't change is that as we cannot tell apart the omitting
of -M from that of -machine. Both will have the semantic "use default
machine".

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

[Alexander Graf]

On 25.07.2011, at 14:05, Peter Maydell wrote:

> On 25 July 2011 12:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>> For ARM you absolutely should not be relying on the default
>> machine type (not least because it's an incredibly ancient
>> dev board which nobody uses any more). An ARM kernel is
>> generally fairly specific to the hardware platform being
>> emulated, so you should know which machine you're intending
>> to run on and specify it explicitly.
> 
> In fact having thought about it a bit I'm going to go further
> and say that the whole idea of a "default machine" is a rather
> x86-centric idea -- most architectures don't really have a
> single machine type that's used by just about everybody,
> always has been, and isn't likely to become obsolete in the
> future. So if we're reworking the command line API to
> supersede "-M" then we shouldn't have a default at all.

That's not exactly true. For PPC, everyone so far expects a Mac to pop up. For S390x, we only have a single machine implemented. I think having a default makes it easier for users to run something you give to them - if it runs on the default target.

> (Consider also the possibility of eventually having a single
> qemu binary that supports multiple architectures -- that
> would make a 'default machine' definitely a bit odd.)

It would be a different machine depending on the emulated target.


Alex

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

[Anthony Liguori]

On 07/25/2011 07:44 AM, Alexander Graf wrote:
>
> On 25.07.2011, at 14:05, Peter Maydell wrote:
>
>> On 25 July 2011 12:48, Peter Maydell<peter.maydell@linaro.org>  wrote:
>>> For ARM you absolutely should not be relying on the default
>>> machine type (not least because it's an incredibly ancient
>>> dev board which nobody uses any more). An ARM kernel is
>>> generally fairly specific to the hardware platform being
>>> emulated, so you should know which machine you're intending
>>> to run on and specify it explicitly.
>>
>> In fact having thought about it a bit I'm going to go further
>> and say that the whole idea of a "default machine" is a rather
>> x86-centric idea -- most architectures don't really have a
>> single machine type that's used by just about everybody,
>> always has been, and isn't likely to become obsolete in the
>> future. So if we're reworking the command line API to
>> supersede "-M" then we shouldn't have a default at all.
>
> That's not exactly true. For PPC, everyone so far expects a Mac to pop up.

Except if you're running on an IBM Power box, then you definitely expect 
a pseries guest to pop up.

We really need to enable the default config file (yes, we have a default 
config file) can express the default machine.

Regards,

Anthony Liguori

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

[Richard W.M. Jones]

On Mon, Jul 25, 2011 at 07:47:51AM -0500, Anthony Liguori wrote:
> On 07/25/2011 07:44 AM, Alexander Graf wrote:
> >
> >On 25.07.2011, at 14:05, Peter Maydell wrote:
> >
> >>On 25 July 2011 12:48, Peter Maydell<peter.maydell@linaro.org>  wrote:
> >>>For ARM you absolutely should not be relying on the default
> >>>machine type (not least because it's an incredibly ancient
> >>>dev board which nobody uses any more). An ARM kernel is
> >>>generally fairly specific to the hardware platform being
> >>>emulated, so you should know which machine you're intending
> >>>to run on and specify it explicitly.
> >>
> >>In fact having thought about it a bit I'm going to go further
> >>and say that the whole idea of a "default machine" is a rather
> >>x86-centric idea -- most architectures don't really have a
> >>single machine type that's used by just about everybody,
> >>always has been, and isn't likely to become obsolete in the
> >>future. So if we're reworking the command line API to
> >>supersede "-M" then we shouldn't have a default at all.
> >
> >That's not exactly true. For PPC, everyone so far expects a Mac to pop up.
> 
> Except if you're running on an IBM Power box, then you definitely
> expect a pseries guest to pop up.
> 
> We really need to enable the default config file (yes, we have a
> default config file) can express the default machine.

+1 to this.

I was going to say there's missing information here, ie. if I had a
Debian/arm kernel know, what machine should I use, but it looks like a
config file would provide this missing information.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v

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

[Alexander Graf]

On 25.07.2011, at 14:49, Richard W.M. Jones wrote:

> On Mon, Jul 25, 2011 at 07:47:51AM -0500, Anthony Liguori wrote:
>> On 07/25/2011 07:44 AM, Alexander Graf wrote:
>>> 
>>> On 25.07.2011, at 14:05, Peter Maydell wrote:
>>> 
>>>> On 25 July 2011 12:48, Peter Maydell<peter.maydell@linaro.org>  wrote:
>>>>> For ARM you absolutely should not be relying on the default
>>>>> machine type (not least because it's an incredibly ancient
>>>>> dev board which nobody uses any more). An ARM kernel is
>>>>> generally fairly specific to the hardware platform being
>>>>> emulated, so you should know which machine you're intending
>>>>> to run on and specify it explicitly.
>>>> 
>>>> In fact having thought about it a bit I'm going to go further
>>>> and say that the whole idea of a "default machine" is a rather
>>>> x86-centric idea -- most architectures don't really have a
>>>> single machine type that's used by just about everybody,
>>>> always has been, and isn't likely to become obsolete in the
>>>> future. So if we're reworking the command line API to
>>>> supersede "-M" then we shouldn't have a default at all.
>>> 
>>> That's not exactly true. For PPC, everyone so far expects a Mac to pop up.
>> 
>> Except if you're running on an IBM Power box, then you definitely
>> expect a pseries guest to pop up.
>> 
>> We really need to enable the default config file (yes, we have a
>> default config file) can express the default machine.
> 
> +1 to this.
> 
> I was going to say there's missing information here, ie. if I had a
> Debian/arm kernel know, what machine should I use, but it looks like a
> config file would provide this missing information.

Well, what we really want is an image file format that also pulls along machine config files. Or just have 2 files for now - an image and a machine description config. VMs simply are more than just their hard disks ;).


Alex

[Qemu-devel] [PATCH] Allow to leave type on default in -machine

[Jan Kiszka]

On 2011-07-25 13:48, Jan Kiszka wrote:
> On 2011-07-25 13:39, Markus Armbruster wrote:
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>
>>> On 2011-07-25 12:45, Richard W.M. Jones wrote:
>>>> On Mon, Jul 25, 2011 at 12:33:01PM +0200, Jan Kiszka wrote:
>>>>> On 2011-07-25 11:41, Richard W.M. Jones wrote:
>>>>>> On Sat, Jul 23, 2011 at 12:38:37PM +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.
>>>>>>
>>>>>> This breaks libguestfs which was doing:
>>>>>>
>>>>>>   qemu -machine accel=kvm:tcg ...
>>>>>>
>>>>>> We are not passing any -M option at all.  We don't particularly care
>>>>>> about the machine type since we're not that performance sensitive and
>>>>>> we don't need to serialize the machine state.
>>>>>>
>>>>>> I have checked, and this works:
>>>>>>
>>>>>>   qemu -machine pc,accel=kvm:tcg ...
>>>>>>
>>>>>> "pc" is the default, right?  What about for other architectures?
>>>>>
>>>>> Yes, pc is the right default. Other arch have other defaults.
>>>>
>>>> So what you're saying is we have to parse qemu -machine \? output by
>>>> looking for the string '(default)'?  eg:
>>>>
>>>> $ ./arm-softmmu/qemu-system-arm -machine \?|fgrep '(default)'
>>>> integratorcp ARM Integrator/CP (ARM926EJ-S) (default)
>>>>
>>>> $ ./i386-softmmu/qemu -machine \?|fgrep '(default)'
>>>> pc-0.14    Standard PC (default)
>>>
>>> I understand, this is clumsy. Will see if we can do better.
>>
>> Is there a technical reason why type isn't optional with -machine?
>>
>> [...]
> 
> Maybe it's just the
> 
> assert(!permit_abbrev || list->implied_opt_name);
> 
> in qemu_opts_parse, but I haven't looked at all details (and all other
> users) yet.

I was incorrectly pointing the core, the problem is solvable at the
level where we parse -machine:

-------8<--------

This allows to specify -machine options without setting an explicit
machine type. We will pick the default machine in this case. Requesting
the list of available machines is still possible via '-machine ?' e.g.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 vl.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 8256504..5e53ddc 100644
--- a/vl.c
+++ b/vl.c
@@ -2720,7 +2720,10 @@ int main(int argc, char **argv, char **envp)
                     fprintf(stderr, "parse error: %s\n", optarg);
                     exit(1);
                 }
-                machine = machine_parse(qemu_opt_get(opts, "type"));
+                optarg = qemu_opt_get(opts, "type");
+                if (optarg) {
+                    machine = machine_parse(optarg);
+                }
                 break;
             case QEMU_OPTION_usb:
                 usb_enabled = 1;
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH] Allow to leave type on default in -machine

[Richard W.M. Jones]

On Mon, Jul 25, 2011 at 06:11:20PM +0200, Jan Kiszka wrote:
> I was incorrectly pointing the core, the problem is solvable at the
> level where we parse -machine:
> 
> -------8<--------
> 
> This allows to specify -machine options without setting an explicit
> machine type. We will pick the default machine in this case. Requesting
> the list of available machines is still possible via '-machine ?' e.g.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  vl.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 8256504..5e53ddc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2720,7 +2720,10 @@ int main(int argc, char **argv, char **envp)
>                      fprintf(stderr, "parse error: %s\n", optarg);
>                      exit(1);
>                  }
> -                machine = machine_parse(qemu_opt_get(opts, "type"));
> +                optarg = qemu_opt_get(opts, "type");
> +                if (optarg) {
> +                    machine = machine_parse(optarg);
> +                }
>                  break;
>              case QEMU_OPTION_usb:
>                  usb_enabled = 1;
> -- 
> 1.7.3.4

I have tested this patch, and it allows libguestfs to work without
modifications.  ie qemu -machine accel=[...] works as before.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH] Allow to leave type on default in -machine

[Alexander Graf]

On 25.07.2011, at 18:21, Richard W.M. Jones wrote:

> On Mon, Jul 25, 2011 at 06:11:20PM +0200, Jan Kiszka wrote:
>> I was incorrectly pointing the core, the problem is solvable at the
>> level where we parse -machine:
>> 
>> -------8<--------
>> 
>> This allows to specify -machine options without setting an explicit
>> machine type. We will pick the default machine in this case. Requesting
>> the list of available machines is still possible via '-machine ?' e.g.
>> 
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> vl.c |    5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>> 
>> diff --git a/vl.c b/vl.c
>> index 8256504..5e53ddc 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2720,7 +2720,10 @@ int main(int argc, char **argv, char **envp)
>>                     fprintf(stderr, "parse error: %s\n", optarg);
>>                     exit(1);
>>                 }
>> -                machine = machine_parse(qemu_opt_get(opts, "type"));
>> +                optarg = qemu_opt_get(opts, "type");
>> +                if (optarg) {
>> +                    machine = machine_parse(optarg);
>> +                }
>>                 break;
>>             case QEMU_OPTION_usb:
>>                 usb_enabled = 1;
>> -- 
>> 1.7.3.4
> 
> I have tested this patch, and it allows libguestfs to work without
> modifications.  ie qemu -machine accel=[...] works as before.

Very nice! It's also a lot more intuitive this way.


Alex

Re: [Qemu-devel] [PATCH] Allow to leave type on default in -machine

[Anthony Liguori]

On 07/25/2011 11:11 AM, Jan Kiszka wrote:
> On 2011-07-25 13:48, Jan Kiszka wrote:
>> On 2011-07-25 13:39, Markus Armbruster wrote:
>>> Jan Kiszka<jan.kiszka@siemens.com>  writes:
>>>
>>>> On 2011-07-25 12:45, Richard W.M. Jones wrote:
>>>>> On Mon, Jul 25, 2011 at 12:33:01PM +0200, Jan Kiszka wrote:
>>>>>> On 2011-07-25 11:41, Richard W.M. Jones wrote:
>>>>>>> On Sat, Jul 23, 2011 at 12:38:37PM +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.
>>>>>>>
>>>>>>> This breaks libguestfs which was doing:
>>>>>>>
>>>>>>>    qemu -machine accel=kvm:tcg ...
>>>>>>>
>>>>>>> We are not passing any -M option at all.  We don't particularly care
>>>>>>> about the machine type since we're not that performance sensitive and
>>>>>>> we don't need to serialize the machine state.
>>>>>>>
>>>>>>> I have checked, and this works:
>>>>>>>
>>>>>>>    qemu -machine pc,accel=kvm:tcg ...
>>>>>>>
>>>>>>> "pc" is the default, right?  What about for other architectures?
>>>>>>
>>>>>> Yes, pc is the right default. Other arch have other defaults.
>>>>>
>>>>> So what you're saying is we have to parse qemu -machine \? output by
>>>>> looking for the string '(default)'?  eg:
>>>>>
>>>>> $ ./arm-softmmu/qemu-system-arm -machine \?|fgrep '(default)'
>>>>> integratorcp ARM Integrator/CP (ARM926EJ-S) (default)
>>>>>
>>>>> $ ./i386-softmmu/qemu -machine \?|fgrep '(default)'
>>>>> pc-0.14    Standard PC (default)
>>>>
>>>> I understand, this is clumsy. Will see if we can do better.
>>>
>>> Is there a technical reason why type isn't optional with -machine?
>>>
>>> [...]
>>
>> Maybe it's just the
>>
>> assert(!permit_abbrev || list->implied_opt_name);
>>
>> in qemu_opts_parse, but I haven't looked at all details (and all other
>> users) yet.
>
> I was incorrectly pointing the core, the problem is solvable at the
> level where we parse -machine:
>
> -------8<--------
>
> This allows to specify -machine options without setting an explicit
> machine type. We will pick the default machine in this case. Requesting
> the list of available machines is still possible via '-machine ?' e.g.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   vl.c |    5 ++++-
>   1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 8256504..5e53ddc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2720,7 +2720,10 @@ int main(int argc, char **argv, char **envp)
>                       fprintf(stderr, "parse error: %s\n", optarg);
>                       exit(1);
>                   }
> -                machine = machine_parse(qemu_opt_get(opts, "type"));
> +                optarg = qemu_opt_get(opts, "type");
> +                if (optarg) {
> +                    machine = machine_parse(optarg);
> +                }
>                   break;
>               case QEMU_OPTION_usb:
>                   usb_enabled = 1;