[virt-tools-list] [virt-manager PATCH 4/4]RFC: addhardware:enable setting controller model

[Cole Robinson]

On 04/16/2014 12:09 PM, Chen Hanxiao wrote:
> 
> On 04/16/2014 08:32 PM, Giuseppe Scrivano wrote:
>> Chen Hanxiao <chenhanxiao at cn.fujitsu.com> writes:
>>
>>> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
>>> ---
>>>   virtManager/addhardware.py | 18 +++++++++++-------
>>>   1 file changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
>>> index 60e8033..531282c 100644
>>> --- a/virtManager/addhardware.py
>>> +++ b/virtManager/addhardware.py
>>> @@ -325,9 +325,6 @@ class vmmAddHardware(vmmGObjectUI):
>>>           target_model = Gtk.ListStore(str, str)
>>>           combo.set_model(target_model)
>>>           uiutil.set_combo_text_column(combo, 1)
>>> -        # FIXME: we should deal with controller model
>>> -        combo.set_visible(False)
>>> -        self.widget("controller-model-label").set_visible(False)
>>>             # Available HW options
>>>           is_local = not self.conn.is_remote()
>>> @@ -1813,12 +1810,19 @@ class vmmAddHardware(vmmGObjectUI):
>>>           controller_type = self.get_config_controller_type()
>>>           model = self.get_config_controller_model()
>>>           self._dev = VirtualController(conn)
>>> -        # FIXME
>>> -        model = "none"
>>> +
>>> +        for ctrl in self.vm.get_controller_devices():
>>> +            # FIXME: some sync issue
>>> +            if ctrl.type == "usb":
>>> +                self.vm.remove_device(ctrl)
>> why do we remove the device here?
> We could not get rid of USB controller.
> UI forbid us from removing it either. (Although we still could delete it by
> right-click)
> 

Whoops, that's a bug. We should be synchronizing the right click option and
the button. Please either file a bug or provide a patch :)

> So I think we should remove the existed USB controller before adding a new one.
> 
> USB 2 maps to multi device and we need to deal with some complex scenario.
> We also support modifying controller model, remove the existed USB controller
> may simplify codes.
> 
> But I met some sync issue: when I remove USB controllers, it will be added
> back automatically.
> We would see more than one USB controllers, sometimes error would be thrown.
> 
> So maybe we need a better method to deal with.
> 

Indeed this is kind of tricky. Part of what makes it difficult is the legacy
libvirt behavior:

- qemu has usb enabled by default
- removing the usb controller is done by setting <controller type='usb'
model='none'/>

However not all libvirt drivers have USB by default, so 'adding' a new USB
controller is still useful. But it's hard to do the right thing for a qemu VM
with a USB controller already attached.

I say for now let's just avoid the problem:

1) If the guest has no USB controller or usb model=none, allow adding a USB
controller.
2) If the guest has a USB controller, disable the USB option the
addhw->controller->type combo box, and add a tooltip like:

'$vmname already has a USB controller attached. Adding more than one USB
controller is not supported. You can change the USB controller type in the VM
details screen.'

We should also make USB controller removal actually work from the details
screen, but that's a separate task.

- Cole