[libvirt] [PATCH] virsh: lookup interface by name or mac other than one by one
On host without interface eth1, 'virsh iface-dumpxml eth1' it reports error: Interface not found: couldn't find interface with MAC address 'eth1' 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; + /* 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); -- 1.8.1.4
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?
+ /* 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);
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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.
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 -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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
participants (4)
-
Daniel P. Berrange -
Guannan Ren -
Laine Stump -
Osier Yang