[libvirt] [PATCH] virsh: Improve the error for snapshot-list
It reports error "roots and --from are exclusive" even "--current" is specified with "--roots", but no "--from". --- tools/virsh-snapshot.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index b828371..6dd8bf2 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1197,7 +1197,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) } if (from) { vshError(ctl, "%s", - _("--roots and --from are mutually exclusive")); + _("--roots is mutually exclusive with either " + "--from or --current")); goto cleanup; } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; -- 1.7.7.6
On 10/22/2012 03:15 PM, Osier Yang wrote:
It reports error "roots and --from are exclusive" even "--current" is specified with "--roots", but no "--from". --- tools/virsh-snapshot.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index b828371..6dd8bf2 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1197,7 +1197,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) } if (from) { vshError(ctl, "%s", - _("--roots and --from are mutually exclusive")); + _("--roots is mutually exclusive with either " + "--from or --current")); goto cleanup; } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
And is --from and --current also mutually exclusive? If yes, wouldn't it be better to say something like: "--roots, --current and --from are mutually exclusive"? Or, similarly to the idea behind vshLookupSnapshot if (exclusive && current && snapname) { vshError(ctl, _("--%s and --current are mutually exclusive"), arg); return -1; } If it's not the case, than ACK. Martin
On 10/22/2012 07:44 AM, Martin Kletzander wrote:
On 10/22/2012 03:15 PM, Osier Yang wrote:
It reports error "roots and --from are exclusive" even "--current" is specified with "--roots", but no "--from". --- tools/virsh-snapshot.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index b828371..6dd8bf2 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1197,7 +1197,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) } if (from) { vshError(ctl, "%s", - _("--roots and --from are mutually exclusive")); + _("--roots is mutually exclusive with either " + "--from or --current")); goto cleanup; } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
And is --from and --current also mutually exclusive?
--current implies a particular point to list from, so yes, mixing --current and --from should error out, as should mixing --current and --roots, so this is really just a bug in improving error message quality.
If yes, wouldn't it be better to say something like: "--roots, --current and --from are mutually exclusive"? Or, similarly to the idea behind vshLookupSnapshot
if (exclusive && current && snapname) { vshError(ctl, _("--%s and --current are mutually exclusive"), arg); return -1; }
If it's not the case, than ACK.
Indeed, I think we can do better with a v2. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年10月22日 21:44, Martin Kletzander wrote:
On 10/22/2012 03:15 PM, Osier Yang wrote:
It reports error "roots and --from are exclusive" even "--current" is specified with "--roots", but no "--from". --- tools/virsh-snapshot.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index b828371..6dd8bf2 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1197,7 +1197,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) } if (from) { vshError(ctl, "%s", - _("--roots and --from are mutually exclusive")); + _("--roots is mutually exclusive with either " + "--from or --current")); goto cleanup; } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
And is --from and --current also mutually exclusive?
No from the logic, but better to have Eric's input. If yes, wouldn't
it be better to say something like: "--roots, --current and --from are mutually exclusive"? Or, similarly to the idea behind vshLookupSnapshot
if (exclusive&& current&& snapname) { vshError(ctl, _("--%s and --current are mutually exclusive"), arg); return -1; }
If it's not the case, than ACK.
Martin
participants (3)
-
Eric Blake -
Martin Kletzander -
Osier Yang