On Thu, May 21, 2015 at 11:40:26 +0200, Andrea Bolognani wrote: > This will allow us to use vshError() to report errors from inside > vshCommandOpt*(), instead of replicating the same logic and error > messages all over the place. > > We also have more context inside the vshCommandOpt*() functions, > for example the actual value used on the command line, which means > we can produce more detailed error messages. > --- > tools/virsh-domain-monitor.c | 90 +++---- > tools/virsh-domain.c | 598 > +++++++++++++++++++++---------------------- > tools/virsh-host.c | 46 ++-- > tools/virsh-interface.c | 14 +- > tools/virsh-network.c | 34 +-- > tools/virsh-nodedev.c | 6 +- > tools/virsh-pool.c | 26 +- > tools/virsh-secret.c | 8 +- > tools/virsh-snapshot.c | 88 +++---- > tools/virsh-volume.c | 34 +-- > tools/virsh.c | 113 ++++---- > tools/virsh.h | 77 +++--- > 12 files changed, 571 insertions(+), 563 deletions(-) >
...
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 4425774..9f94b75 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
...
> -vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
> +vshCommandOpt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd,
> + const char *name, vshCmdOpt **opt,
> bool needData)
So this helper is not using ctl nor reporting any errors. Even in the
next patch.
> {
> vshCmdOpt *candidate = cmd->opts;
...
> @@ -1829,11 +1831,12 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char
> *name,
> * option is present without actually using that data.
> */
> bool
> -vshCommandOptBool(const vshCmd *cmd, const char *name)
> +vshCommandOptBool(vshControl *ctl, const vshCmd *cmd,
> + const char *name)
> {
> vshCmdOpt *dummy;
>
> - return vshCommandOpt(cmd, name, &dummy, false) == 1;
> + return vshCommandOpt(ctl, cmd, name, &dummy, false) == 1;
And vshCommandOptBool is designed not to report any error. I'm not a big
fan of changing half of virsh by changing the prototype and then not
using the parameter at all.
> }
>
> /**
I didn't go through the rest of the changes, but reporting malformed
integers right in the parser makes sense to me.
Peter
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
