[virt-tools-list] [PATCH 4/6] IPv6 support: update the create network wizard

[Gene Czarcinski]

On 03/29/2013 10:07 AM, Gene Czarcinski wrote:
> On 03/28/2013 05:53 PM, Cole Robinson wrote:
>> On 03/28/2013 01:41 PM, Gene Czarcinski wrote:
>>> On 03/28/2013 12:55 PM, Cole Robinson wrote:
>>>> On 03/27/2013 05:37 PM, Gene Czarcinski wrote:
>>>>> On 03/26/2013 06:51 PM, Cole Robinson wrote:
>>>>>> On 03/26/2013 07:05 AM, Gene Czarcinski wrote:
>>>>>>> This update includes changes to both the glade-3 ui and
>>>>>>> the releated python code (createnet.py).  There have
>>>>>>> been some significant changes made to both files for
>>>>>>> IPv6 support and some additional capabilities.
>>>>>>>
>>>>>>> This update includes the fix so color will work in the
>>>>>>> network creation wizard.
>>>>>>>
>>>>>>> The objective was to get the functionality implemented and
>>>>>>> some future effort may be needed to "improve" the
>>>>>>> python code.
>>>>>>> Signed-off-by: Gene Czarcinski <gene at czarc.net>
>>>>>>> ---
>>>>>>>     ui/vmm-create-net.ui     | 1518
>>>>>>> ++++++++++++++++++++++++++++++++--------------
>>>>>>>     virtManager/createnet.py |  612 +++++++++++++++----
>>>>>>>     2 files changed, 1583 insertions(+), 547 deletions(-)
>>>>>>>
>>>>>> A couple bits here:
>>>>>>
>>>>>> - The color of the createnet header changed: it used to be black.
>>>>> What's wrong with blue? ... OK, it is back to black again.
>>>> Blue is the color of the other wizards, but the createnet wizard 
>>>> needs to be
>>>> really redesigned to have a more consistent look before switching 
>>>> the color.
>>> I have tried to be consistent but that does not mean that I am.  If 
>>> you can
>>> point me to some good examples and/or guidelines, I will give it a 
>>> shot.
>>>
>> Well it's not a requirement of this patch series to redesign the 
>> whole wizard
>> :) Eventually I'd like it to look more like the 'new vm', 'new 
>> interface', or
>> 'clone' wizard. So shrink them down, blue header with icon, no intro 
>> screen,
>> try to reduce the number of screens in general, etc. While the end 
>> result will
>> have blue in the header it won't look like it does now, so I just 
>> wanted to
>> revert to the black before doing things piecemeal.
> The wizard as I now have it does basic IPv6 support.  However, it also 
> needs to handle things dhcp-host or where dhcp-range is specified for 
> "ra-only".  This "network creation wizard" could (and should) also be 
> used to edit an existing definition.  But, I want to get the basic 
> support in first.
>>
>>> For now, I have completed redoing all of the gui and wizard stuff
>>> incorporating your comments.  Just before wrpaping it up and 
>>> packaging the
>>> patches for submit, I did one more test: create, start, stop, and then
>>> delete/undefine a network ... everything worked fine until the 
>>> delete ... the
>>> network was deleted but I got an exception when net_select() tried 
>>> to access
>>> the just deleted network???  This happens with some net-definitions 
>>> but not
>>> with others??
>>>
>> This may just be a pre-existing bug. I know there is an issue with 
>> deleting VM
>> hardware that I've hit on the gtk3 branch that didn't seem to exist 
>> on master,
>> so it might be affecting every dialog that dynamically removes a list 
>> row. If
>> you can reliably reproduce with the testdriver, please document the 
>> steps and
>> the backtrace in todo.txt
> This may or may not be a pre-existing condition but my changes to 
> vmm-host.ui, host.py, and network.py as well as the basic changes to 
> createnet.py to use ipaddr instead of IPy appear to exacerbate the 
> problem.  To complicate things, this may be a timing or race-condition 
> problem.  Nevertheless, I am going to do more debugging on it.
>
> The problem as I now understand it:
>
> Many networks defined ... IPv4 and IPv6 or IPv4 only, it makes no 
> difference.  Start virt-manager --debug and then bring up Connection 
> Details/Virtual Networks.
>
> Externally with virsh, undefine a network ... everything OK.
> Now select another network and then back to default.  Now, with virsh, 
> undefine another network ... BANG ... an exception for the deleted 
> network by name.
>
> The problem is in net_selected() with:
>     net = self.conn.get_net(selected[0].get_value(selected[1], 0))
>
> net_selected() gets called because the is a signal set up "change" in 
> "net-list"
>
> I cannot get this to reproduce under the base gtk3.2 branch.
>
> If I make progress today or not, I believe I will submit what I now 
> have (v2) later today because this problem (while something that needs 
> to be fixed) does not impact running virt-manager.
>
>
I now understand what the problem is and have two proposed solutions:

1. in host.py, there is:
> self.widget("net-list").get_selection().connect("changed",
> self.net_selected)
2.  Sometimes but not always, this results in set_selected() being 
called.  Again, sometimes when net_selected() is called, the parameters 
it is passed points to a list-entry which has benn or is in the proccess 
of being deleted.  Regardless, when net_selected() executes:
> net = self.conn.get_net(selected[0].get_value(selected[1], 0))
and conn.get_net() blindly trys to lookup the network in the "nets" 
dictionary kept by connections.py.  The result is an exception because, 
by this time, that network has been deleted from the dictionary.

Solutions:

1. In connections.py, change get_net() so that it checks to see if the 
network is defined in the dictionary, if it is return the usual; if it 
is not, return None.  This means that net_selected (and every other 
caller of get_net()) must check the result it gets from get_net().

2. Leave get_net() as is.  Change net_selected() in host.py to "try" the 
call to get_net() and if an exception occurs, simply ignore it (return) 
but issue no error messages.

For now, the simplest is #2 but the whole business of handling changes 
to the lists needs to be examined and the code error-proofed.

I am proceeding with 2.

Gene