On 05/14/2014 12:15 AM, Li Yang wrote:
Long subject line; it's better to keep subject under 60 columns, for the
sake of 'git shortlog' in an 80-column window. I went with:
virsh: reject undefine --wipe-storage without also naming storage
> For now, if only '--wipe-storage' is assigned, user can undefine a
> domain normally. But actually '--wipe-storage' doesn't work, this
More that it silently does nothing, not that it doesn't work.
> may confuse user. And since '--wipe-storage' wipes data on the
> removed volumes, if no removed volume storage assigned, we'd better
> raise an error message.
>
> Before:
> $ virsh undefine virt-tests-vm1 --wipe-storage
> Domain virt-tests-vm1 has been undefined
>
> After:
> $ virsh undefine virt-tests-vm1 --wipe-storage
> error: '--wipe-storage' needs storage volume deletion: '--stroage <string>' or '--remove-all-storage' is necessary.
s/stroage/storage/
>
> Signed-off-by: Li Yang <liyang.fnst@xxxxxxxxxxxxxx>
> ---
> tools/virsh-domain.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 3a7c260..25236a0 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2982,6 +2982,14 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
>
> ignore_value(vshCommandOptString(cmd, "storage", &vol_string));
>
> + if (!(vol_string || remove_all_storage) && wipe_storage) {
> + vshError(ctl,
> + _("'--wipe-storage' needs storage volume deletion: "
> + "'--stroage <string>' or '--remove-all-storage' "
> + "is necessary."));
We tend to avoid trailing '.' in error messages (although we aren't
consistent). This is also long-winded and has a typo.
ACK to the idea, so I'm pushing with this modification:
diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c
index a5ddb93..84a6706 100644
--- i/tools/virsh-domain.c
+++ w/tools/virsh-domain.c
@@ -2979,14 +2979,12 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
size_t i;
size_t j;
-
ignore_value(vshCommandOptString(cmd, "storage", &vol_string));
if (!(vol_string || remove_all_storage) && wipe_storage) {
vshError(ctl,
- _("'--wipe-storage' needs storage volume deletion: "
- "'--stroage <string>' or '--remove-all-storage' "
- "is necessary."));
+ _("'--wipe-storage' requires '--storage <string>' or "
+ "'--remove-all-storage'"));
return false;
}
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list