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

[Osier Yang]

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