[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] virsh: lookup interface by name or mac other than one by one



On 14/05/13 19:59, Daniel P. Berrange wrote:
On Tue, May 14, 2013 at 07:54:48AM -0400, Laine Stump wrote:
On 05/14/2013 05:15 AM, Osier Yang wrote:
On 14/05/13 16:35, Guannan Ren wrote:
On host without interface eth1, 'virsh iface-dumpxml eth1'
it reports
error: Interface not found: couldn't find interface with MAC address
'eth1'
It should be similar for other objects. E.g.

vshCommandOptVolumeBy

So, perhaps what we need is a general method to fix the problems.

after fix, it reports
error: Interface not found: couldn't find interface named 'eth1'
---
   tools/virsh-interface.c | 10 +++++++---
   1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index f75c572..47883ae 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -46,6 +46,7 @@ vshCommandOptInterfaceBy(vshControl *ctl, const
vshCmd *cmd,
   {
       virInterfacePtr iface = NULL;
       const char *n = NULL;
+    bool is_name = false;
       virCheckFlags(VSH_BYNAME | VSH_BYMAC, NULL);
         if (!optname)
@@ -62,14 +63,17 @@ vshCommandOptInterfaceBy(vshControl *ctl, const
vshCmd *cmd,
       if (name)
           *name = n;
   +    if (!strchr(n, ':'))
+        is_name = true;
Is it guaranteed that a network interface name can't contain a colon?
No.

Although it's not a true interface (the kernel doesn't see it as a
separate interface), an "alias" interface can be created to add another
IP address to an interface. It would be named something like "eth0:1",
"eth0:2", etc. Use of alias interfaces to put multiple IP addresses on a
single physical interface has been deprecated in favor of simply using
netlink to add an IP address to the interface, but it's still supported,
and quite common.

I would say that we should either:

1) remove "mac address"/"name" from the error message (or put in both)
to make a single generic message.

I think 1) is better, as said, the problem should be common for other
objects too, not restricted to interface.  E.g. vshCommandOptVolBy,
vshCommandOptPoolBy, I don't think one will want to check the passed
argument for each of these helpers, e.g. Checking if it's a name or a
UUID. It's overkill.

And removing the specific string like "with MAC address" doesn't hurt,
one knows what he inputs in the command line.


or

2) only check for a ":" after the lookupbyname has failed, and decide
then whether to log the error message or retry with lookupbymac.
Or actually see if it parses as a mac address or not, rather than
just looking for a single ':'.

Daniel


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]