[virt-tools-list] [virt-manager PATCH] virt-manager: close the SPICE main channel on an AUTH error

[Cole Robinson]

On 10/28/2014 03:42 AM, Giuseppe Scrivano wrote:
> Cole Robinson <crobinso at redhat.com> writes:
> 
>> On 10/21/2014 09:45 AM, Giuseppe Scrivano wrote:
>>> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1152981
>>>
>>> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
>>> ---
>>>  virtManager/console.py | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/virtManager/console.py b/virtManager/console.py
>>> index 67bfe10..030a5c1 100644
>>> --- a/virtManager/console.py
>>> +++ b/virtManager/console.py
>>> @@ -405,6 +405,10 @@ class SpiceViewer(Viewer):
>>>              self.console.disconnected()
>>>          elif event == SpiceClientGLib.ChannelEvent.ERROR_AUTH:
>>>              self.console.activate_auth_page()
>>> +            for i in self._main_channel_hids:
>>> +                self.main_channel.handler_disconnect(i)
>>> +            self._main_channel_hids = []
>>> +            self.main_channel = None
>>>          elif event in [SpiceClientGLib.ChannelEvent.ERROR_CONNECT,
>>>                         SpiceClientGLib.ChannelEvent.ERROR_IO,
>>>                         SpiceClientGLib.ChannelEvent.ERROR_LINK,
>>>
>> This just duplicates similar code in SpiceDisplay.close(), should we call that
>> instead?
> 
> here we close just the main channel, instead SpiceDisplay.close() does
> more things and if I just use it then the auth won't work.
> 
> If you prefer, I can amend this code to factor out the common code in a
> function:
> 
> diff --git a/virtManager/console.py b/virtManager/console.py
> index 030a5c1..a214538 100644
> --- a/virtManager/console.py
> +++ b/virtManager/console.py
> @@ -377,6 +377,13 @@ class SpiceViewer(Viewer):
>          self._display.set_property("grab-keyboard",
>              self.config.get_keyboard_grab_default())
>  
> +    def close_main_channel(self):
> +        for i in self._main_channel_hids:
> +            self.main_channel.handler_disconnect(i)
> +        self._main_channel_hids = []
> +
> +        self.main_channel = None
> +
>      def close(self):
>          if self.spice_session is not None:
>              self.spice_session.disconnect()
> @@ -387,11 +394,7 @@ class SpiceViewer(Viewer):
>          self._display = None
>          self._display_channel = None
>  
> -        for i in self._main_channel_hids:
> -            self.main_channel.handler_disconnect(i)
> -        self._main_channel_hids = []
> -
> -        self.main_channel = None
> +        self.close_main_channel()
>          self.usbdev_manager = None
>  
>      def is_open(self):
> @@ -405,10 +408,7 @@ class SpiceViewer(Viewer):
>              self.console.disconnected()
>          elif event == SpiceClientGLib.ChannelEvent.ERROR_AUTH:
>              self.console.activate_auth_page()
> -            for i in self._main_channel_hids:
> -                self.main_channel.handler_disconnect(i)
> -            self._main_channel_hids = []
> -            self.main_channel = None
> +            self.close_main_channel()
>          elif event in [SpiceClientGLib.ChannelEvent.ERROR_CONNECT,
>                         SpiceClientGLib.ChannelEvent.ERROR_IO,
>                         SpiceClientGLib.ChannelEvent.ERROR_LINK,
> 
> Thanks,
> Giuseppe
> 

ACK to that, but please make it _close_main_channel so it's private to SpiceViewer

Thanks,
Cole