[libvirt] [PATCHv2 1/7] 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' --- tools/virsh-volume.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 898b34b..d7c4823 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); -- 1.9.0