From: Peter Krempa <pkrempa@redhat.com>
Date: Mon, 3 Mar 2014 14:21:37 +0000 (+0100)
Subject: virsh: volume: Fix lookup of volumes to provide better error messages
X-Git-Tag: v1.2.3-rc1~349
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a751e3452b0280df64a9a67a6c96a09e1045026e;p=thirdparty%2Flibvirt.git

virsh: volume: Fix lookup of volumes to provide better error messages

If a user specifies the pool explicitly, we should make sure to point
out that it's inactive instead of falling back to lookup by key/path and
failing at the end. Also if the pool isn't found there's no use in
continuing the lookup.

This changes the error in case the user-selected pool is inactive from:

 $ virsh vol-upload --pool inactivepool --vol somevolname volcontents
 error: failed to get vol 'somevolname'
 error: Storage volume not found: no storage vol with matching path
 somevolname

To a more descriptive:

 $ virsh vol-upload --pool inactivepool --vol somevolname volcontents
 error: pool 'inactivepool' is not active

And in case a user specifies an invalid pool from:

 $ virsh vol-upload --pool invalidpool --vol somevolname volcontents
 error: failed to get pool 'invalidpool'
 error: failed to get vol 'somevolname', specifying --pool might help
 error: Storage volume not found: no storage vol with matching path somevolname

To something less confusing:

 $ virsh vol-upload --pool invalidpool --vol somevolname volcontents
 error: failed to get pool 'invalidpool'
 error: Storage pool not found: no storage pool with matching name 'invalidpool'
---

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 898b34b33e..d7c4823730 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -60,8 +60,16 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
         vshCommandOptStringReq(ctl, cmd, pooloptname, &p) < 0)
         return NULL;
 
-    if (p)
-        pool = vshCommandOptPoolBy(ctl, cmd, pooloptname, name, flags);
+    if (p) {
+        if (!(pool = vshCommandOptPoolBy(ctl, cmd, pooloptname, name, flags)))
+            return NULL;
+
+        if (virStoragePoolIsActive(pool) != 1) {
+            vshError(ctl, _("pool '%s' is not active"), p);
+            virStoragePoolFree(pool);
+            return NULL;
+        }
+    }
 
     vshDebug(ctl, VSH_ERR_DEBUG, "%s: found option <%s>: %s\n",
              cmd->def->name, optname, n);