This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 871931

Summary: get_host_network_devices returns empty list, drop it
Product: [Community] Virtualization Tools Reporter: Richard Marko <rmarko>
Component: virt-managerAssignee: Cole Robinson <crobinso>
Status: CLOSED UPSTREAM QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: unspecifiedCC: acathrow, berrange, crobinso, jberan, jforbes, rmarko, virt-maint
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-07-07 14:27:56 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Richard Marko 2012-10-31 14:13:30 EDT
Description of problem:
ifconfig output format has changed since fedora 16 which results in get_host_network_devices returning wrong data.

Version-Release number of selected component (if applicable):
python-virtinst-0.600.3-1.fc17.noarch

How reproducible:
Always

Steps to Reproduce:
1. python -c 'import virtinst; print virtinst.util.get_host_network_devices()'
  
Actual results:
[]

Expected results:
[['br0', 'link', 'encap:ethernet', 'hwaddr', '00:1a:a0:ad:33:45'], ['em1', 'link', 'encap:ethernet', 'hwaddr', '00:1a:a0:ad:33:45'], ['virbr0', 'link', 'encap:ethernet', 'hwaddr', '52:54:00:50:4e:58'], ['virbr0-nic', 'link', 'encap:ethernet', 'hwaddr', '52:54:00:50:4e:58']]
Comment 1 Cole Robinson 2012-12-16 17:35:59 EST
Was this causing an issue in some way? I think it's only used to make sure the user doesn't specify a MAC address that conflicts with the host NIC, which is not important enough to deal with parsing ifconfig output. I'll probably just drop this function when we merge virt-manager and virtinst repos.

Moving to the upstream tracker.
Comment 2 Cole Robinson 2012-12-16 17:43:44 EST
Forgot to set needinfo, richard please see comment #1
Comment 3 Richard Marko 2013-01-02 04:19:40 EST
(In reply to comment #1)
> Was this causing an issue in some way? I think it's only used to make sure
> the user doesn't specify a MAC address that conflicts with the host NIC,
> which is not important enough to deal with parsing ifconfig output. I'll
> probably just drop this function when we merge virt-manager and virtinst
> repos.
> 

I've found this by accident while trying to use it in my project. 

Public API provided by virtinst is actually quite nice and valuable (at least for me).

You can probably use my simple version which only returns MAC addresses:

def get_host_macs():
    with os.popen('LC_ALL=C /sbin/ip addr') as f:
        return map(lambda x: x.split()[1],
                filter(lambda x: 'link/ether' in x,
                    f.readlines()))
Comment 4 Cole Robinson 2013-01-06 10:18:27 EST
(In reply to comment #3)
> (In reply to comment #1)
> > Was this causing an issue in some way? I think it's only used to make sure
> > the user doesn't specify a MAC address that conflicts with the host NIC,
> > which is not important enough to deal with parsing ifconfig output. I'll
> > probably just drop this function when we merge virt-manager and virtinst
> > repos.
> > 
> 
> I've found this by accident while trying to use it in my project. 
> 
> Public API provided by virtinst is actually quite nice and valuable (at
> least for me).
> 

Is this a public project? If so, virtinst is going to cease to be a public API very soon now so you will be stranded.

https://www.redhat.com/archives/virt-tools-list/2012-February/msg00040.html

libvirt-gconfig and libvirt-designer will eventually provide the important functionality.

If this is a private project you can just keep a copy of the old code around, or even continue to use the API that will eventually exist under /usr/share/virt-manager/virtinst, though understandable there will be no API guarantees.

> You can probably use my simple version which only returns MAC addresses:
> 
> def get_host_macs():
>     with os.popen('LC_ALL=C /sbin/ip addr') as f:
>         return map(lambda x: x.split()[1],
>                 filter(lambda x: 'link/ether' in x,
>                     f.readlines()))

Thanks for the code, however the purpose of the original code (to make sure a manually specified mac address does not conflict with the host address) is pretty obscure, and not worth calling out to an external tool to verify. And if anything we should be using libvirt interface APIs. So I'll just drop this code when the next release goes out.
Comment 5 Richard Marko 2013-01-07 09:00:55 EST
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #1)
> > > Was this causing an issue in some way? I think it's only used to make sure
> > > the user doesn't specify a MAC address that conflicts with the host NIC,
> > > which is not important enough to deal with parsing ifconfig output. I'll
> > > probably just drop this function when we merge virt-manager and virtinst
> > > repos.
> > > 
> > 
> > I've found this by accident while trying to use it in my project. 
> > 
> > Public API provided by virtinst is actually quite nice and valuable (at
> > least for me).
> > 
> 
> Is this a public project? If so, virtinst is going to cease to be a public
> API very soon now so you will be stranded.
> 
> https://www.redhat.com/archives/virt-tools-list/2012-February/msg00040.html
> 
> libvirt-gconfig and libvirt-designer will eventually provide the important
> functionality.
> 
> If this is a private project you can just keep a copy of the old code
> around, or even continue to use the API that will eventually exist under
> /usr/share/virt-manager/virtinst, though understandable there will be no API
> guarantees.
> 

It will be public but it's not a problem for me to rewrite few API calls I'm using.
Comment 6 Cole Robinson 2013-04-21 15:12:24 EDT
virtinst has been merged into virt-manager.git. Moving all virtinst bugs to the virt-manager component.
Comment 7 Cole Robinson 2013-07-07 14:27:56 EDT
This has been dropped now as planned in Comment #4