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

[chenhanxiao at cn.fujitsu.com]

> -----Original Message-----
> From: Cole Robinson [mailto:crobinso at redhat.com]
> Sent: Thursday, April 17, 2014 12:40 AM
> To: Chen Hanxiao; Giuseppe Scrivano; Chen, Hanxiao/陈 晗霄
> Cc: virt-tools-list at redhat.com
> Subject: Re: [virt-tools-list] [virt-manager PATCH 4/4]RFC: addhardware:enable
> setting controller model
> 
> 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 :)
> 
Ok, patch of hiding right click will come soon.

> > 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

Thanks for your comments.
I'll try to finish it in the next version.

- Chen