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

[Cole Robinson]

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.

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

Thanks,
Cole