On 05/28/2015 05:31 AM, Andrea Bolognani wrote: > As suggested by Michal: now that we have a generic error message for > failures related to the parsing of integer options, it makes sense to > perform the corresponding check in a single spot instead of replicating > it every time vshCommandOpt*() is used. > > Andrea Bolognani (5): > virsh: Use standard error message in vshCommandOptTimeoutToMs(). > virsh: Improve vshCommandOptTimeoutToMs(). > virsh: Make vshCommandOptScaledInt() use vshCommandOpt(). > virsh: Pass vshControl to all vshCommandOpt*() calls. > virsh: Move error messages inside vshCommandOpt*() functions. > > tests/vcpupin | 4 +- > tools/virsh-domain-monitor.c | 17 +-- > tools/virsh-domain.c | 226 ++++++++++++--------------------------- > tools/virsh-host.c | 67 +++--------- > tools/virsh-interface.c | 6 +- > tools/virsh-network.c | 10 +- > tools/virsh-nodedev.c | 4 +- > tools/virsh-snapshot.c | 2 +- > tools/virsh-volume.c | 26 +---- > tools/virsh.c | 248 +++++++++++++++++++++++++------------------ > tools/virsh.h | 66 ++++++------ > 11 files changed, 278 insertions(+), 398 deletions(-) > Patch 1 was fine Patch 2 had some comments which should be simple to fix Patch 3 was previously ACK'd by Michal (v2 1/3) Patch 4 had a couple of NIT's about going beyond 80 columns on a line, but since it's so pervasive in the rest of the module that can be a future cleanup ;-)... I do note that 'vshCommandOpt' now has an unrelated difference - I assume it had a ctl argument at one point that's since been removed... In any case, ACK'd by Michal module OptBool which appears to have been addressed (v2 2/3) Patch 5 still seemed to need some sort of error message in vshCommandOptString... Some callers ignore return status, so even adding an error may still be "ignored". Overall seems OK to me with some minor cleanups. John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list