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

[Anthony Liguori <anthony@codemonkey.ws>]

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?

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
>