[virt-tools-list] [virt-manager PATCH] host: catch KeyError in interface_selected

[Cole Robinson]

On 03/11/2014 09:35 PM, Chen Hanxiao wrote:
> 
> 
>> -----Original Message-----
>> From: Cole Robinson [mailto:crobinso at redhat.com]
>> Sent: Tuesday, March 11, 2014 11:54 PM
>> To: Chen Hanxiao; Chen Hanxiao; virt-tools-list at redhat.com
>> Subject: Re: [virt-tools-list] [virt-manager PATCH] host: catch KeyError
> in
>> interface_selected
>>
>> On 03/11/2014 11:34 AM, Chen Hanxiao wrote:
>>>
>>> On 03/11/2014 08:26 PM, Cole Robinson wrote:
>>>> On 03/11/2014 03:12 AM, Chen Hanxiao wrote:
>>>>> We should catch KeyError in interface_selected,
>>>>> for that error could happen when
>>>>> singal on_interface_list_changed comes.
>>>>>
>>>>> How to reproduce:
>>>>> 1. create 3 bridge by Edit->Connection Details->Network Interface
>>>>> 2. delete them
>>>>> 3. We would get a KeyError
>>>>>
>>>>> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
>>>>> ---
>>>>>   virtManager/host.py | 9 +++++++++
>>>>>   1 file changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/virtManager/host.py b/virtManager/host.py
>>>>> index c8d7ee0..97a4f74 100644
>>>>> --- a/virtManager/host.py
>>>>> +++ b/virtManager/host.py
>>>>> @@ -1098,6 +1098,15 @@ class vmmHost(vmmGObjectUI):
>>>>>           name = model[treeiter][0]
>>>>>             try:
>>>>> +            self.conn.get_interface(name)
>>>>> +        except KeyError:
>>>>> +            self.widget("interface-apply").set_sensitive(False)
>>>>> +            return
>>>>> +        except Exception, e:
>>>>> +            logging.exception(e)
>>>>> +            self.set_interface_error_page(_("Error selecting
> interface:
>>>>> %s") %
>>>>> +                    e)
>>>>> +        try:
>>>>>               self.populate_interface_state(name)
>>>>>           except Exception, e:
>>>>>               logging.exception(e)
>>>>>
>>>> If an interface is removed, vmmConnection should emit
> interface-removed,
>> which
>>>> calls host.py:repopulate_interfaces and should remove the missing
> interface
>>>> from the list before the user has a chance to select anything. If
> that's not
>>>> working we should figure out why that is.
>>>>
>>>> - Cole
>>> When we delete interface by UI, interface list will be changed,
>>> signal "on_interface_list_changed" will be raised and it will also
>>> invoke interface_selected.
>>> I think that caused this issue.
>>
>> Does this reproduce with the test driver? I couldn't reproduce.
>> Does this issue affect storage pools or virtual networks? If so, can we
> find a
>> general solution (since they all use the same pattern). If not, what is
>> different about the interface bits here?
>>
> I spot this issue by manual test. And I could reproduce it on Fedora20 and
> RHEL7.
> 
> Pls open debug logs of virt-manager, we could see the error message there.
> 1. add 2 bridges by UI
> 2. add 1 vlan by UI
> 3. delete the newly added vlan
> 
> Traceback (most recent call last):
>   File "/root/virt-manager/virtManager/host.py", line 1101, in
> interface_selected
>     self.populate_interface_state(name)
>   File "/root/virt-manager/virtManager/host.py", line 1110, in
> populate_interface_state
>     interface = self.conn.get_interface(name)
>   File "/root/virt-manager/virtManager/connection.py", line 691, in
> get_interface
>     return self.interfaces[name]
> KeyError: 'enp6s0f0.0'
> 

Thanks, I can reproduce the same issue with the test driver.

It's weird though: interface_selected is being triggered by model.clear() in
populate_interfaces. It usually calls interface_selected on every interface
after the current selected row: so the gtk code is probably removing the first
row over and over, and when it removes the current selection (the interface we
are deleting), it selects the next row in line.

But sometimes it also triggers a selection event on the current row, which is
we have deleted, and shows that key error. I can't figure out the reasoning.

Either way, we can work around this by doing
iface_list.get_selection().unselect_all() before the model.clear(), and that
also saves the code from churning over and over with the interface_selected
events. Please add a patch to do that, and make a similar change to the other
populate_* functions in host.py

- Cole