Bug 871931 - get_host_network_devices returns empty list, drop it
Summary: get_host_network_devices returns empty list, drop it
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Virtualization Tools
Classification: Community
Component: virt-manager
Version: unspecified
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
Assignee: Cole Robinson
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-10-31 18:13 UTC by Richard Marko
Modified: 2016-02-01 02:22 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-07-07 18:27:56 UTC
Embargoed:


Attachments (Terms of Use)

Description Richard Marko 2012-10-31 18:13:30 UTC
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 22:35:59 UTC
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 22:43:44 UTC
Forgot to set needinfo, richard please see comment #1

Comment 3 Richard Marko 2013-01-02 09:19:40 UTC
(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 15:18:27 UTC
(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 14:00:55 UTC
(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 19:12:24 UTC
virtinst has been merged into virt-manager.git. Moving all virtinst bugs to the virt-manager component.

Comment 7 Cole Robinson 2013-07-07 18:27:56 UTC
This has been dropped now as planned in Comment #4


Note You need to log in before you can comment on or make changes to this bug.