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