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@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/