[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 02/10] Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command



On 07/11/2013 08:05 AM, Michal Privoznik wrote:
> On 08.07.2013 21:20, John Ferlan wrote:
>> Add a new qemuMonitorJSONGetObjectProperty() method to support invocation
>> of the 'qom-get' JSON monitor command with a provided path, property, and
>> expected data type return. The qemuMonitorJSONObjectProperty is similar to
>> virTypedParameter; however, a future patch will extend it a bit to include
>> a void pointer to balloon driver statistic data.
>>
>> The provided test will execute a qom-get on "/machine/i440fx" which will
>> return a property "realized".
>> ---
>>  src/qemu/qemu_monitor_json.c | 86 ++++++++++++++++++++++++++++++++++++++++++++
>>  src/qemu/qemu_monitor_json.h | 34 ++++++++++++++++++

Does the new type need to live in the .h file, or can it be internal to
the .c file?  That is, I think qemuMonitorJSONGetObjectProperty can be
marked static, since it is only used within the same file, and then all
the types that it uses moved into the .c file as well.

>> +struct _qemuMonitorJSONObjectProperty {
>> +    int type;    /* qemuMonitorJSONObjectPropertyType */
>> +    union {
>> +        bool b;
>> +        int i;
> 
> Huh, syntax-check fails at this ^^^ line thinking @i is a loop variable. I guess you'll need to add an exception to cfg.mk:

Or rename the variable 'si'.

> 
> diff --git a/cfg.mk b/cfg.mk
> index c6a097e..2936280 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -985,4 +985,4 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets = \
>    ^(python/|tools/|examples/|include/libvirt/(virterror|libvirt-(qemu|lxc))\.h$$)
>  
>  exclude_file_name_regexp--sc_prohibit_int_ijk = \
> -  ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/)$
> +  ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/|src/qemu/qemu_monitor_json.h)$

Exempting entire files because of one line seems a bit overkill; maybe
we can instead tweak the cfg.mk rule to have an
  exclude='exempt from syntax-check'
line, then place a magic comment on this use (see how sc_prohibit_strtol
uses that mechanism for one-line exemptions instead of entire-file
exemptions).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]