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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list