[RFC,v2,03/12] VMState test: query command to extract the qdevified device names

I have provided a qmp interface for getting the list of qdevified devices
that have been registered with SaveVMHandlers.

Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
---
 qapi-schema.json | 22 ++++++++++++++++++++++
 qmp-commands.hx  | 25 +++++++++++++++++++++++++
 savevm.c         | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+)

On 07/25/2014 09:39 AM, Sanidhya Kashyap wrote:
> I have provided a qmp interface for getting the list of qdevified devices
> that have been registered with SaveVMHandlers.
> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  qapi-schema.json | 22 ++++++++++++++++++++++
>  qmp-commands.hx  | 25 +++++++++++++++++++++++++
>  savevm.c         | 34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+)
> 

> +# @device: list of qdevified device names
> +#
> +# Since 2.2
> +##
> +{ 'type': 'VMStatesQdevDevices',
> +  'data': { 'device': ['str'] } }

Here, you name it 'device' [1]

> +
> +##
> +# @query-qdev-devices
> +#
> +# returns the VMStatesQdevDevices that have the associated value
> +#
> +# Since 2.2
> +##
> +{ 'command': 'query-qdev-devices',
> +  'returns': 'VMStatesQdevDevices' }

and state that it returns a single struct [2]

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..2e20032 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3755,3 +3755,28 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +
> +    {
> +        .name       = "query-qdev-devices",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_qdev_devices,
> +    },
> +
> +SQMP
> +query-qdev-devices
> +------------------
> +
> +Shows registered Qdevified devices
> +
> +
> +Example (1):
> +
> +-> { "execute": "query-qdev-devices" }
> +<- { "return": [

But here, your example shows it returning an array of structs [2]

> +       {
> +         "devices": [ "kvm-tpr-opt", "piix4_pm" ]

where each struct contains a member named 'devices' [1] that is also an
array.

I actually think the most extensible thing would be to return something
more like this QMP wire contents:

{ "return": [
  { "device": "kvm-tpr-opt" },
  { "device": "piix4_pm" }
] }

which would match this .json content:

{ 'type': 'VMStatesQdevDevices',
  'data': { 'device': 'str' } }
{ 'command': 'query-qdev-devices',
  'returns': [ 'VMStatesQdevDevices' ] }

and also be the most extensible for future tweaks, such as adding an
optional boolean flag to the json to tell us more about certain devices:

{ 'type': 'VMStatesQdevDevices',
  'data': { 'device': 'str', '*foo': 'bool' } }

and lead to this QMP wire transaction:

{ "return": [
  { "device": "kvm-tpr-opt" },
  { "device": "piix4_pm", 'foo': true }
] }


At any rate, you MUST make your example match your documentation.

Sanidhya Kashyap <sanidhya.iiith@gmail.com> wrote:
> I have provided a qmp interface for getting the list of qdevified devices
> that have been registered with SaveVMHandlers.
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  qapi-schema.json | 22 ++++++++++++++++++++++
>  qmp-commands.hx  | 25 +++++++++++++++++++++++++
>  savevm.c         | 34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b11aad2..996e6b5 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3480,3 +3480,25 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @VMstatesQdevDevices
> +#
> +# list of qdevified devices that are registered with SaveStateEntry
> +#
> +# @device: list of qdevified device names


Should we use qdev on the name?  Or just list of devices?  My
understanding is that all devices are on this list, no?

> +#
> +# Since 2.2
> +##
> +{ 'type': 'VMStatesQdevDevices',
> +  'data': { 'device': ['str'] } }
> +
> +##
> +# @query-qdev-devices
> +#
> +# returns the VMStatesQdevDevices that have the associated value
> +#
> +# Since 2.2
> +##
> +{ 'command': 'query-qdev-devices',
> +  'returns': 'VMStatesQdevDevices' }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..2e20032 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3755,3 +3755,28 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +
> +    {
> +        .name       = "query-qdev-devices",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_qdev_devices,
> +    },
> +
> +SQMP
> +query-qdev-devices
> +------------------
> +
> +Shows registered Qdevified devices
> +
> +
> +Example (1):
> +
> +-> { "execute": "query-qdev-devices" }
> +<- { "return": [
> +       {
> +         "devices": [ "kvm-tpr-opt", "piix4_pm" ]

Once here, can we change this to also include the device version?

i.e. something like:

 "devices": [ [ "device": [ "name": "kvm-tpr-opt", "version", 15]]], ...]

Or somesuch?

> +       }
> +     ]
> +   }
> +
> +EQMP
> diff --git a/savevm.c b/savevm.c
> index 0255fa0..7c1600a 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1167,6 +1167,40 @@ void do_savevm(Monitor *mon, const QDict *qdict)
>      }
>  }
>  
> +static strList *create_qdev_list(const char *name, strList *list)
> +{
> +    strList *temp_list;
> +    int len;
> +
> +    if (!list) {
> +        list = g_malloc0(sizeof(strList));
> +        len = strlen(name);
> +        list->value = g_malloc0(sizeof(char)*(len+1));
> +        strcpy(list->value, name);
> +        list->next = NULL;
> +        return list;
> +    }
> +    temp_list = g_malloc0(sizeof(strList));
> +    len = strlen(name);
> +    temp_list->value = g_malloc0(sizeof(char)*(len+1));
> +    strcpy(temp_list->value, name);
> +    temp_list->next = list;
> +    list = temp_list;
> +    return list;
> +}
> +
> +VMStatesQdevDevices *qmp_query_qdev_devices(Error **errp)
> +{
> +    VMStatesQdevResetEntry *qre;
> +    VMStatesQdevDevices *qdev_devices = g_malloc0(sizeof(VMStatesQdevDevices));
> +
> +    QTAILQ_FOREACH(qre, &vmstate_reset_handlers, entry) {
> +        qdev_devices->device = create_qdev_list(qre->device_name,
> +                                                 qdev_devices->device);
> +    }
> +    return qdev_devices;
> +}
> +
>  void qmp_xen_save_devices_state(const char *filename, Error **errp)
>  {
>      QEMUFile *f;

On 07/29/2014 06:45 AM, Juan Quintela wrote:
> Sanidhya Kashyap <sanidhya.iiith@gmail.com> wrote:
>> I have provided a qmp interface for getting the list of qdevified devices
>> that have been registered with SaveVMHandlers.
>>
>> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
>> ---
>>  qapi-schema.json | 22 ++++++++++++++++++++++
>>  qmp-commands.hx  | 25 +++++++++++++++++++++++++
>>  savevm.c         | 34 ++++++++++++++++++++++++++++++++++
>>  3 files changed, 81 insertions(+)

>> +
>> +Example (1):
>> +
>> +-> { "execute": "query-qdev-devices" }
>> +<- { "return": [
>> +       {
>> +         "devices": [ "kvm-tpr-opt", "piix4_pm" ]
> 
> Once here, can we change this to also include the device version?
> 
> i.e. something like:
> 
>  "devices": [ [ "device": [ "name": "kvm-tpr-opt", "version", 15]]], ...]

Not valid JSON, but see my other messages in the thread.  My idea of an
array of structs would be nice for this:

{ "return": [
  { "device": "kvm-tpr-opt", "version": 15 },
  { "device": "piix4_pm", "version": 1 }
] }

>> +##
>> +# @VMstatesQdevDevices
>> +#
>> +# list of qdevified devices that are registered with SaveStateEntry
>> +#
>> +# @device: list of qdevified device names
> 
> 
> Should we use qdev on the name?  Or just list of devices?  My
> understanding is that all devices are on this list, no?
> 

The problem is that there are some devices which have not been
qdevified. So, that's why I used the term qdev. If you want, I can
remove the term qdev. Sooner or later all the devices will be qdevified.

>> +
>> +-> { "execute": "query-qdev-devices" }
>> +<- { "return": [
>> +       {
>> +         "devices": [ "kvm-tpr-opt", "piix4_pm" ]
> 
> Once here, can we change this to also include the device version?
> 
> i.e. something like:
> 
>  "devices": [ [ "device": [ "name": "kvm-tpr-opt", "version", 15]]], ...]
> 
> Or somesuch?
> 

That is possible. Do you want any other information to be printed?