[virt-tools-list] [virt-manager PATCH 3/3] Try to get ip of vm's nic only when the vm is active

[Lin Ma]

On 09/07/2018 02:00 AM, Cole Robinson wrote:
> On 09/06/2018 03:46 AM, Lin Ma wrote:
>> Signed-off-by: Lin Ma <lma at suse.com>
>> ---
>>   virtManager/details.py | 15 +++++++++------
>>   virtManager/domain.py  |  4 ++++
>>   2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/virtManager/details.py b/virtManager/details.py
>> index 0d02805f..64bed185 100644
>> --- a/virtManager/details.py
>> +++ b/virtManager/details.py
>> @@ -2755,12 +2755,15 @@ class vmmDetails(vmmGObjectUI):
>>           state = net.link_state == "up" or net.link_state is None
>> self.widget("network-link-state-checkbox").set_active(state)
>>   -        ipv4, ipv6 = self.vm.get_interface_addresses(net)
>> -        label = ipv4 or ""
>> -        if ipv6:
>> -            if label:
>> -                label += "\n"
>> -            label += ipv6
>> +        if self.vm.is_active():
>> +            ipv4, ipv6 = self.vm.get_interface_addresses(net)
>> +            label = ipv4 or ""
>> +            if ipv6:
>> +                if label:
>> +                    label += "\n"
>> +                label += ipv6
>> +        else:
>> +            label = ""
>>           self.widget("network-ip").set_text(label or _("Unknown"))
>>             self.netlist.set_dev(net)
>> diff --git a/virtManager/domain.py b/virtManager/domain.py
>> index b628b5cd..9da43c03 100644
>> --- a/virtManager/domain.py
>> +++ b/virtManager/domain.py
>> @@ -1150,6 +1150,8 @@ class vmmDomain(vmmLibvirtObject):
>>           self._backend.snapshotCreateXML(xml, flags)
>>         def refresh_interface_addresses(self, iface):
>> +        if not self.is_active():
>> +            return
>>           def agent_ready():
>>               for dev in self.xmlobj.devices.channel:
>>                   if (dev.type == "unix" and
>> @@ -1183,6 +1185,8 @@ class vmmDomain(vmmLibvirtObject):
>>           return {}
>>         def get_interface_addresses(self, iface):
>> +        if not self.is_active():
>> +            return
>>           if self._ip_cache is None:
>>               self.refresh_interface_addresses(iface)
>>
>
> I pushed the first two patches, thanks!
>
> This patch sprinkles is_active() in too many places. If we wanted to 
> properly 'fix' this I think we have to blank the _ip_cache when the VM 
> is shut down, and only attempt to refresh if the guest is active, or 
> maybe hide the refresh button altogether when the VM is offline
>
> That said I left it this way deliberately. Partly out of laziness of 
> not wanting to deal with invalidating the cache :) But partly because 
> I wasn't sure how much IP addresses change across VM runs, like if we 
> reboot the VM should we essentially insist the user click 'refresh' 
> again to get the IP?
>
> And even if the VM is offline it seems semi-useful to keep the last 
> used IP address visible rather than immediately blanking it out.

Yeah, I agreed, That makes sense. and keeping the last used IP visible 
is useful.

>
> I dunno I'm not really sold either way, I was just kinda thinking to 
> leave it as is and see if anyone complains :)

OK.


Thanks for your help and review,
Lin