[Libvir] PATCH: 7/16: virsh core commands

[Daniel Veillard]

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