|
>>> Cole Robinson <crobinso@xxxxxxxxxx> 2017/11/22 星期三 上午 6:28 >>> >On 11/06/2017 07:52 AM, Lin Ma wrote: >> It helps users to recognize controllers <-> disks mapping relationship. >> >> Signed-off-by: Lin Ma <lma@xxxxxxxx> >> --- >> ui/details.ui | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >> virtManager/details.py | 25 ++++++++++++++++++++++++- >> 2 files changed, 72 insertions(+), 1 deletion(-) >> >> diff --git a/virtManager/details.py b/virtManager/details.py >> index 22e0786e..a9e0cb79 100644 >> --- a/virtManager/details.py >> +++ b/virtManager/details.py >> @@ -1042,6 +1042,16 @@ class vmmDetails(vmmGObjectUI): >> uiutil.init_combo_text_column(combo, 1) >> combo.set_active(-1) >> >> + attached_disk_list = self.widget("scsi-disk-list") >> + model = Gtk.ListStore(str) >> + attached_disk_list.set_model(model) >> + attached_disk_list.set_headers_visible(False) >> + diskTextCol = Gtk.TreeViewColumn() >> + disk_txt = Gtk.CellRendererText() >> + diskTextCol.pack_start(disk_txt, True) >> + diskTextCol.add_attribute(disk_txt, 'text', 0) >> + attached_disk_list.append_column(diskTextCol) >> + >> > >Can you generalize all the naming here? This UI could easily be reused >for all attached controller devices, let's not add 'disk' into all the >naming that just needs to be changed later. So "controller-device-list" >for the UI name, just name the variables combo/model/col/text here since >it's just init code. will do. >Also in the UI change the label from 'Disks:' to 'Devices:' ok >> ########################## >> # Window state listeners # >> @@ -2995,17 +3005,30 @@ class vmmDetails(vmmGObjectUI): >> if not dev: >> return >> >> + self.widget("disk-list-label").set_visible(False) >> + self.widget("scsi-disk-box").set_visible(False) > >Change these names too. And you should be able to use >uiutil.set_grid_row_visible(self.widget("XXX"), False) to hide the UI ok, will change the corresponding names, and use uiutil.set_grid_row_visible instead. >> can_remove = True >> if self.vm.get_xmlobj().os.is_x86() and dev.type == "usb": >> can_remove = False >> if dev.type == "pci": >> can_remove = False >> if dev.type == "scsi": >> + model = self.widget("scsi-disk-list").get_model() >> + model.clear() >> for disk in self.vm.get_disk_devices(inactive=True): >> if (dev.type == disk.bus and >> dev.index == disk.address.controller): > >Should have mentioned in the first patch. I think you should be checking >disk.address.type here instead of disk.bus. Doesn't matter in practice >but it more correctly identifies what you are checking for. A scsi disk or a sata disk's address information looks like:
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
So the value of disk.address.type field is 'drive', I think we can't check it here. Or use 'if (disk.address.type == "drive" and' instead of 'if (dev.type == disk.bus and'.
Does this make sense?
>Also if you move this to a function in VirtualDeviceAddress like >compare_controller, we can use a central place to store logic for other >devices as well. I guess what you mean is: if dev.type == "scsi":
model = self.widget("controller-device-list").get_model() model.clear() for disk in self.vm.get_disk_devices(): if disk.address.compare_controller(dev): ......
am I right?
>> can_remove = False >> - break >> + name = _label_for_device(disk) >> + bus = disk.address.bus >> + target = disk.address.target >> + unit = disk.address.unit >> + infoStr = ("%s: bus = %s, target = %s, unit = %s" % >> + (name, bus, target, unit)) >> + model.append([infoStr]) > >Can we just use _label_for_device instead of inventing a new label >format? If that's no sufficient and the full address is desired I'd >rather put some address pretty printing into virtinst and use that instead. In my opinion, only _label_for_device is not sufficient, I lean to provide full address information.
If we'll provide full address, I'll move them into a function in VirtualDeviceAddress.
could you please suggest an example of string format for pretty printing?
>> + self.widget("disk-list-label").set_visible(not can_remove) >> + self.widget("scsi-disk-box").set_visible(not can_remove) >> + > >I say always show the UI for scsi controllers, even if they have no >disks attached. ok, will do. Thanks,
Lin
|
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
