>>> Cole Robinson <[email protected]> 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 <[email protected]>
>> ---
>> 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
[email protected]
https://www.redhat.com/mailman/listinfo/virt-tools-list
