[libvirt] [PATCH 0/4] qemu_monitor_json: Refactor even more

[Michal Privoznik]

On 06.05.2016 20:03, John Ferlan wrote:
> 
> 
> On 05/04/2016 08:33 AM, Michal Privoznik wrote:
>> All these patches will be squashed into one, but I've split them
>> into multiple because of easier review.
>>
>> Michal Privoznik (4):
>>   qemu_monitor_json: Follow refactor
>>   qemu_monitor_json: Follow refactor
>>   qemu_monitor_json: Follow refactor
>>   qemu_monitor_json: Follow refactor
>>
>>  src/qemu/qemu_monitor_json.c | 673 ++++++++++++++++++++++---------------------
>>  1 file changed, 349 insertions(+), 324 deletions(-)
>>
> 
> Not a deal breaker, but most code has:
> 
>     if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>         goto cleanup;
> 
>     ret = 0;
>  cleanup:
> 
> while a few instances [1] have:
> 
>      ret = qemuMonitorJSONCheckError(cmd, reply);
> 
>   cleanup:
> 
> IDC either way since the result is the same, but I suppose they "could"
> be all done the same way.  I'd probably lean towards the latter, but
> since you already went with the former in commit '7884d089' it's
> probably best to keep using that.
> 
> [1]
> qemuMonitorJSONSetMigrationCompression
> qemuMonitorJSONBlockCommit
> qemuMonitorJSONInjectNMI  [2]
> qemuMonitorJSONSendKey  [2]
> qemuMonitorJSONSetMigrationCapability
> 
> [2]
> both instances use the same HMP call attempt to return ret - they could
> follow the qemuMonitorJSONSetCPU model...
> 

Oh. Thanks for catching that. I'll fix those too.

> 
> Finally, I see the comments move 4 spaces to the right in
> qemuMonitorJSONSetObjectProperty?   (patch 4)...  Was that a remnant or
> expected?

While working on this I didn't bother with adjusting the code that much
initially. After that I've fired up my code alignment key shortcut over
each function I've touched. I will leave the comments as they were.

> 
> 
> ACK series (your call if you want to adjust [2] now or in a followup
> patch. it's trivial enough as long as you remember to also remove the {}
> around what would become "one line" if then else statements).
> 

Thanks, pushed now.

Michal