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

[Laine Stump]

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.

or

2) only check for a ":" after the lookupbyname has failed, and decide
then whether to log the error message or retry with lookupbymac.


>
>> +
>>       /* try it by NAME */
>> -    if (flags & VSH_BYNAME) {
>> +    if (is_name && (flags & VSH_BYNAME)) {
>>           vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as interface
>> NAME\n",
>>                    cmd->def->name, optname);
>>           iface = virInterfaceLookupByName(ctl->conn, n);
>> -    }
>> +
>>       /* try it by MAC */
>> -    if (!iface && (flags & VSH_BYMAC)) {
>> +    } else if (!is_name && (flags & VSH_BYMAC)) {
>>           vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as interface
>> MAC\n",
>>                    cmd->def->name, optname);
>>           iface = virInterfaceLookupByMACString(ctl->conn, n);
>
> -- 
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>