[virt-tools-list] [virt-manager PATCH 0/2] show ip in details page of domain interface

[Cole Robinson]

On 09/06/2018 03:07 AM, Lin Ma wrote:
> 
> 
> On 09/06/2018 03:13 AM, Cole Robinson wrote:
>> On 08/31/2018 06:37 AM, Lin Ma wrote:
>>> Uses lease, agent and arp to try to get ipv4 and ipv6 address of 
>>> given interface
>>>
>>> Lin Ma (2):
>>>    domain: Add function interface_addresses to return ipv4 and ipv6
>>>    details: Show ipv4 and ipv6 address in details page of interface
>>>
>>>   ui/details.ui          | 79 
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   virtManager/details.py | 59 +++++++++++++++++++++++++++++++++++++
>>>   virtManager/domain.py  | 27 +++++++++++++++++
>>>   3 files changed, 165 insertions(+)
>>>
>>
>> Thanks for these! I've pushed them now. I added a commit on top that 
>> aimed to improve a few things: tweak the UI, handle errors, cache 
>> results better, and open it up to work on more hypervisor drivers. 
>> Please give it a try and make sure it still meets your usecases:
>>
>> commit b4b7c7b5203dac9c5583b130bb8808d7a0b528fd (HEAD -> ip)
>> Author: Cole Robinson <crobinso at redhat.com>
>> Date:   Tue Sep 4 19:02:40 2018 -0400
>>
>>     details: IP address fixes/improvements
>>
>>     * Tweak the UI
>>     * Add accelerator for the refresh button
>>     * Make the IP labels selectable
>>     * Drop the IP prefix from the UI, it's not the important bit
>>     * Call DHCPLeases on the network instead to support this for more
>>       drivers, like LXC
>>     * Cache the IP results in the domain/network object wrappers
>>     * Catch and log errors
>>     * Poll for IP address when first visiting the interface page
>>
> The commit is great, It makes code more generic and make more sense!
> 
> It seems that the commit introduces 4 issues:
> 
> *1 If guest no any ip, it causes exception, say:
> summary=Error refreshing hardware page: 'NoneType' object is not iterable
> details=Traceback (most recent call last):
>    File "/home/lin/projects/virt-manager/virtManager/details.py", line 
> 1235, in hw_selected
>      self.refresh_network_page()
>    File "/home/lin/projects/virt-manager/virtManager/details.py", line 
> 2758, in refresh_network_page
>      ipv4, ipv6 = self.vm.get_interface_addresses(net)
>    File "/home/lin/projects/virt-manager/virtManager/domain.py", line 
> 1235, in get_interface_addresses
>      ipv4, ipv6 = extract_dom(ips)
>    File "/home/lin/projects/virt-manager/virtManager/domain.py", line 
> 1213, in extract_dom
>      for addr in addrs["addrs"]:
> TypeError: 'NoneType' object is not iterable
> 
> *2 If guest no ipv4 but ipv6, it causes exception, say:
> [Thu, 06 Sep 2018 13:03:49 virt-manager 9835] DEBUG (error:83) error 
> dialog message:
> summary=Error refreshing hardware page: unsupported operand type(s) for 
> +=: 'NoneType' and 'str'
> details=Traceback (most recent call last):
>    File "/home/lin/projects/virt-manager/virtManager/details.py", line 
> 1235, in hw_selected
>      self.refresh_network_page()
>    File "/home/lin/projects/virt-manager/virtManager/details.py", line 
> 2763, in refresh_network_page
>      label += ipv6
> TypeError: unsupported operand type(s) for +=: 'NoneType' and 'str'
> 
> *3 It looks that the ip information is showed in details network page of 
> vm even though the vm is inactive.
> 
> *4 We know that the ip information onto network card details page only 
> can be updated by refresh_network_page.
>     That means if the virtual nic's ip is changed and users click the 
> refresh button want to see the new ip,
>     the new ip information won't be updated on page until users refresh 
> this virtual nic's details page.
> 
> I'm goint to post patches to fix issue 1 and issue 2.
> moreover, I'd like to modify the code to show the IP address to 
> 'Unknown' if vm is inactive,
> this can reduce the api overhead while we use a slow remote connection 
> and can workaround issue 3.
> 

I replied to your patch series about these points.

> About issue4, I havn't got a proper idea yet to fix it, If you think it 
> is an issue, Could you please fix it?
> 

Good catch, I've fixed it upstream now:

commit 7baa296aeda372aba6fc7f41b11183221d581062 (HEAD -> master)
Author: Cole Robinson <crobinso at redhat.com>
Date:   Thu Sep 6 14:04:00 2018 -0400

     details: Update IP addr UI after refresh_ip call

Thanks,
Cole