Re: [libvirt] [PATCH 2/2] virsh-snapshot: Reject --no-metadata together with --print-xml
On 02/11/13 14:47, Osier Yang wrote:
On 2013年02月11日 21:10, Peter Krempa wrote:
> Manual for "virsh snapshot-create-as" states that --no-metadata and
> --print-xml are incompatible. Honor this detail in the code.
> ---
> tools/virsh-snapshot.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index fe1cee9..d9659d4 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -412,8 +412,14 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd
> *cmd)
> unsigned int flags = 0;
> const vshCmdOpt *opt = NULL;
>
> - if (vshCommandOptBool(cmd, "no-metadata"))
> + if (vshCommandOptBool(cmd, "no-metadata")) {
> + if (vshCommandOptBool(cmd, "print-xml")) {
Considering using a variable to store the return value instead of
calling the function twice.
I was considering that option too, but it's used just twice in the
function and gets called twice only if --no-metadata is specified. This
case is ultra-rare.
> + vshError(ctl, "%s",
> + _("--print-xml is incompatible with
> --no-metadata"));
> + return false;
> + }
> flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA;
> + }
> if (vshCommandOptBool(cmd, "halt"))
> flags |= VIR_DOMAIN_SNAPSHOT_CREATE_HALT;
> if (vshCommandOptBool(cmd, "disk-only"))
But I'm fine if you keep the twice calling, as it's trivial. ACK.
I've pushed the series. Thanks for the review.
Peter