[libvirt] [PATCH] virsh: More friendly err if no pool is specified for looking up a vol
There are 3 ways to lookup a volume, only virStorageVolLookupByName needs pool object. So if no --pool is specified, it will tries to get the volume via virStorageVolLookupByPath/virStorageVolLookupByKey. But if all 3 ways fails, and no --pool is specified, a friendly error might help the user get right way quickly. --- tools/virsh.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 371346a..4b9e662 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14714,8 +14714,13 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, vol = virStorageVolLookupByPath(ctl->conn, n); } - if (!vol) - vshError(ctl, _("failed to get vol '%s'"), n); + if (!vol) { + if (pool) + vshError(ctl, _("failed to get vol '%s'"), n); + else + vshError(ctl, _("failed to get vol '%s', specifying --pool " + "might help"), n); + } if (pool) virStoragePoolFree(pool); -- 1.7.6
On 09/21/2011 01:12 AM, Osier Yang wrote:
There are 3 ways to lookup a volume, only virStorageVolLookupByName needs pool object. So if no --pool is specified, it will tries to get the volume via virStorageVolLookupByPath/virStorageVolLookupByKey.
But if all 3 ways fails, and no --pool is specified, a friendly error might help the user get right way quickly. --- tools/virsh.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 371346a..4b9e662 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14714,8 +14714,13 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, vol = virStorageVolLookupByPath(ctl->conn, n); }
- if (!vol) - vshError(ctl, _("failed to get vol '%s'"), n); + if (!vol) { + if (pool) + vshError(ctl, _("failed to get vol '%s'"), n); + else + vshError(ctl, _("failed to get vol '%s', specifying --pool " + "might help"), n); + }
ACK - this is a minimal patch. A nicer patch might be to search by name in all pools, and if an unambiguous name is found, use that volume, but it requires more work, so there's nothing wrong with using this patch in the meantime. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2011年09月21日 23:19, Eric Blake 写道:
On 09/21/2011 01:12 AM, Osier Yang wrote:
There are 3 ways to lookup a volume, only virStorageVolLookupByName needs pool object. So if no --pool is specified, it will tries to get the volume via virStorageVolLookupByPath/virStorageVolLookupByKey.
But if all 3 ways fails, and no --pool is specified, a friendly error might help the user get right way quickly. --- tools/virsh.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 371346a..4b9e662 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14714,8 +14714,13 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, vol = virStorageVolLookupByPath(ctl->conn, n); }
- if (!vol) - vshError(ctl, _("failed to get vol '%s'"), n); + if (!vol) { + if (pool) + vshError(ctl, _("failed to get vol '%s'"), n); + else + vshError(ctl, _("failed to get vol '%s', specifying --pool " + "might help"), n); + }
ACK - this is a minimal patch. A nicer patch might be to search by name in all pools, and if an unambiguous name is found, use that volume, but it requires more work, so there's nothing wrong with using this patch in the meantime.
Pushed, thanks, by the way, it might need to introduce some new flags again, ;-) such as "*_FUZZY_MATCH/PRECISE_MATCH". By default it uses PRECISE_MATCH. FUZZY_MATCH will search all pools and use the specified string as a RE to match out the vols. But the problem is the already existed API virStorageVolLookupByName just returns one virStorageVolPtr obj. So we might need to introduce a new API then. Osier
participants (2)
-
Eric Blake -
Osier Yang