Re: [Qemu-devel] [PATCH 10/19] Add a query-machines command to QMP

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 07 Jun 2010 12:07:56 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 06/07/2010 11:44 AM, Daniel P. Berrange wrote:
> > On Mon, Jun 07, 2010 at 10:13:27AM -0500, Anthony Liguori wrote:
> >    
> >> On 06/07/2010 09:42 AM, Daniel P. Berrange wrote:
> >>      
> >>> Add a new QMP monitor command 'query-machines' to discover what
> >>> machines are defined in the QEMU binary. This is an easily
> >>> parsable replacement for 'qemu -M ?'
> >>>
> >>>      [
> >>>          {
> >>>              "name": "pc-0.13",
> >>>              "description": "Standard PC",
> >>>              "default": 0
> >>>          },
> >>>          {
> >>>              "name": "pc",
> >>>              "description": "Standard PC",
> >>>              "canonical": "pc-0.13",
> >>>              "default": 1
> >>>          },
> >>>          {
> >>>              "name": "pc-0.12",
> >>>              "description": "Standard PC",
> >>>              "default": 0
> >>>          },
> >>>          ....
> >>>      ]
> >>>
> >>> No legacy readline monitor output is provided.
> >>>
> >>> In the future it would be desirable for each machine's QDict to
> >>> also include details of any qdev devices that are included by
> >>> default in the machine type.
> >>>
> >>> Signed-off-by: Daniel P. Berrange<berrange@redhat.com>
> >>> ---
> >>>   hw/boards.h |    1 +
> >>>   monitor.c   |    9 +++++++
> >>>   vl.c        |   70
> >>>   +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>   3 files changed, 80 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/hw/boards.h b/hw/boards.h
> >>> index 6f0f0d7..2f6003d 100644
> >>> --- a/hw/boards.h
> >>> +++ b/hw/boards.h
> >>> @@ -32,6 +32,7 @@ typedef struct QEMUMachine {
> >>>   } QEMUMachine;
> >>>
> >>>   int qemu_register_machine(QEMUMachine *m);
> >>> +void do_info_machines(Monitor *mon, QObject **data);
> >>>
> >>>   extern QEMUMachine *current_machine;
> >>>
> >>> diff --git a/monitor.c b/monitor.c
> >>> index f0406e8..b6aa2b4 100644
> >>> --- a/monitor.c
> >>> +++ b/monitor.c
> >>> @@ -55,6 +55,7 @@
> >>>   #include "json-streamer.h"
> >>>   #include "json-parser.h"
> >>>   #include "osdep.h"
> >>> +#include "hw/boards.h"
> >>>
> >>>   //#define DEBUG
> >>>   //#define DEBUG_COMPLETION
> >>> @@ -2449,6 +2450,14 @@ static const mon_cmd_t info_cmds[] = {
> >>>           .mhandler.info_new = do_info_version,
> >>>       },
> >>>       {
> >>> +        .name       = "machines",
> >>> +        .args_type  = "",
> >>> +        .params     = "",
> >>> +        .help       = "show the machine boards",
> >>> +        .user_print = monitor_user_noop,
> >>> +        .mhandler.info_new = do_info_machines,
> >>> +    },
> >>> +    {
> >>>           .name       = "commands",
> >>>           .args_type  = "",
> >>>           .params     = "",
> >>> diff --git a/vl.c b/vl.c
> >>> index 0b38d62..8043fac 100644
> >>> --- a/vl.c
> >>> +++ b/vl.c
> >>> @@ -1537,6 +1537,76 @@ static QEMUMachine *find_default_machine(void)
> >>>       return NULL;
> >>>   }
> >>>
> >>> +
> >>> +/**
> >>> + * do_info_machines(): Show machine boards
> >>> + *
> >>> + * Returns a QList object listing all machine boards
> >>> + * available with this QEMU target. Each element in
> >>> + * the list is a QDict object containing the following
> >>> + * keys
> >>> + *
> >>> + *  - "name": short name for the machine board
> >>> + *  - "description": long description of the board
> >>> + *  - "alias": name of an alias
> >>> + *
> >>> + * Example:
> >>> + *
> >>> + *  [
> >>> + *     {
> >>> + *       "name": "pc-0.13",
> >>> + *        "description": "Standard PC",
> >>> + *        "default": 0
> >>> + *     },
> >>> + *     {
> >>> + *       "name": "pc",
> >>> + *       "description": "Standard PC",
> >>> + *       "canonical": "pc-0.13",
> >>> + *       "default": 1
> >>> + *     },
> >>> + *     {
> >>> + *       "name": "pc-0.12",
> >>> + *       "description": "Standard PC",
> >>> + *       "default": 0
> >>> + *     },
> >>> + *     ....
> >>> + *  ]
> >>> + *
> >>> + */
> >>>
> >>>        
> >> My only suggestion would be to:
> >>
> >> { 'default-machine': 'pc',
> >>     'machines': [{
> >>       'name': 'pc',
> >>       'description': 'Standard PC',
> >>     },...]
> >> }
> >>
> >> I'd drop 'canonical' too.  Aliasing is an implementation detail and
> >> should not be exposed via the ABI.
> >>      
> > The alias/canonical mapping is something that libvirt needs to know in
> > order guarentee stable guest ABI for all configs it has.
> >
> > What happens is that an app using libvirt will request a guest config
> > with machine type 'pc',
> 
> Does the guest config encode the machine type?  Does that mean that I 
> can't specify pc-0.12 or pc-0.13?

 What about this string format 'pc-0.12', do we really want that kind
of string in QMP? Shouldn't it be broken into 'machine_type' and
'machine_version'?

> 
> Since most guests don't specify an explicit machine, can't libvirt just 
> use pc-<version> and be done with it?
> 
> Regards,
> 
> Anthony Liguori
> 
> >   and libvirt resolves that to the canonical
> > type 'pc-0.12', and then launches QEMU with '-M pc-0.12' instead of
> > the '-M pc'. If libvirt passed '-M pc' and let QEMU do the canonicalization,
> > the guest ABI would no longer be stable across QEMU upgrades because the
> > canonicalization changes to point to the newer version.
> >
> > Regards,
> > Daniel
> >    
> 
>