Re: [PATCHv2 01/11] qemu_monitor_json: Introduce qemuMonitorCPUModelInfo / JSON conversion

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

 



On Mon, Jul 09, 2018 at 22:56:45 -0500, Chris Venteicher wrote:
> Bidirectional conversion functions between Monitor data structure and
> QMP Message JSON.
> 
> Commit creates reusable functions usable anywhere CPUModelInfo structure
> is input or output from QMP Commands.
> 
> JSON of form:
> {"model": {"name": "IvyBridge", "props": {}}}
> ---
>  src/qemu/qemu_monitor_json.c | 126 ++++++++++++++++++++++++++---------
>  1 file changed, 96 insertions(+), 30 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index f9fe9e35ba..a18a1a1bf1 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -5345,6 +5345,101 @@ qemuMonitorJSONParseCPUModelProperty(const char *key,
>      return 0;
>  }
>  
> +
> +/* model_json: {"name": "z13-base", "props": {}}
> + */
> +static virJSONValuePtr
> +qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model)

This is a static function which is not used anywhere yet, you need to
move its definition to the patch which will make use of it. Otherwise
libvirt would fail to compile at this point. Remember the goal is to be
able to compile (and check + syntax-check) libvirt after every single
commit. It's not enough if it compiles at the end of a series.

...
> +/* model_json: {"name": "IvyBridge", "props": {}}
> + */
> +static qemuMonitorCPUModelInfoPtr
> +qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model)
> +{
> +    virJSONValuePtr cpu_props;
> +    qemuMonitorCPUModelInfoPtr machine_model = NULL;
> +    qemuMonitorCPUModelInfoPtr model = NULL;

I would call this variable 'ret' since it is only used to steal the
pointer from machine_model and return it. Then, you can even do
s/machine_model/model/ if you want.

Otherwise the patch looks good.

Jirka

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux