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

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




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...


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


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).


John


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