On Tue, Feb 12, 2008 at 04:34:34AM +0000, Daniel P. Berrange wrote:
> This patch adds new virsh commands for all the libvirt storage APIs
> allowing their use from the shell. The style follows that of the
> existing domain & network commands, so I won't bother listing them
> in great detail.
>
> The only complexity is in dealing with the vol-XXX commands. These
> can accept either a full volume path, a volume key (equiv of UUID)
> or a pool name + volume name. The latter presents a difficulty
> because our GetOpt() style parser does not know how to have commands
> which take either 1 or 2 args - it can do, one or the other but not
> both. So to specify a pool name + volume name requires the slightly
> ugly use of --pool flag. Most people will probably just use paths
> though.
Sounds fine.
> @@ -331,7 +349,7 @@ cmdMonitorProgress(vshControl *ctl, vshC
>
> do {
> if (virJobGetInfo(job, info) < 0) {
> - vshError(ctl, FALSE, _("Failed to get job status"));
> + vshError(ctl, FALSE, "%s", _("Failed to get job status"));
> return -1;
> }
>
Those vshError localization changes are fine and independant from storage,
let's push them in CVS in any case.
[...]
> +static int
> +cmdPoolDelete(vshControl * ctl, vshCmd * cmd)
> +{
> + virStoragePoolPtr pool;
> + int ret = TRUE;
> + char *name;
> +
> + if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
> + return FALSE;
> +
> + if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name)))
> + return FALSE;
> +
> + if (virStoragePoolDelete(pool, 0) == 0) {
> + vshPrint(ctl, _("Pool %s deleteed\n"), name);
> + } else {
> + vshError(ctl, FALSE, _("Failed to delete pool %s"), name);
> + ret = FALSE;
> + virStoragePoolFree(pool);
> + }
> +
> + return ret;
> +}
just wondering, assuming the Delete operation really destroys on-disk
storage and potentially a large set, shouldn't we add some kind of
interactive confirmation ? Contrary to destroying a domain where state
is preserved on the disk and rather easy to recover and destroying a network
which has very little state, maybe here we need to do something special,
optionally adding a -f flag to bypass confirmation like in rm.
> +
> +/*
> + * "vol-delete" command
> + */
> +static vshCmdInfo info_vol_delete[] = {
> + {"syntax", "vol-delete <vol>"},
> + {"help", gettext_noop("delete a vol")},
> + {"desc", gettext_noop("Delete a given vol.")},
> + {NULL, NULL}
> +};
> +
> +static vshCmdOptDef opts_vol_delete[] = {
> + {"pool", VSH_OT_STRING, 0, gettext_noop("pool name or uuid")},
> + {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("vol name, key or path")},
> + {NULL, 0, 0, NULL}
> +};
same for a volume.
No surprise in that code, +1
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list