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