03/10/13 18:09, Cole Robinson wrote:
> On 10/02/2013 10:39 AM, Fred A. Kemp wrote:
> For virt-install, don't worry about adding man page docs unless you want to.
> Extending the cli is pretty easy, you'll want to edit
> virtinst/cli.py:parse_disk, see parse_features acpi handling for an example of
> doing on|off.
Hmm. I assume you mean something like:
--- a/virtinst/cli.py
+++ b/virtinst/cli.py
@@ -1460,6 +1460,7 @@ def parse_disk(guest, optstr, dev=None,
validate=True):
set_param("device", "device")
set_param("bus", "bus")
+ set_param("removable", "removable", convert_cb=_on_off_convert)
set_param("driver_cache", "cache")
set_param("driver_name", "driver_name")
set_param("driver_type", "driver_type")
This fails since it tries to set 'removable' in virtinst to True/False,
but I made it into a "on"/"off"/None tristate. It works for acpi since
it's a bool in virtinst. Perhaps what you mean in my next quote that you
want me to make 'removable' into a bool too (then it works)? If so,
there's an issue with the "future proof[ing]" of the check box for
'removable' in virt-manager you requested (see below).
> Also, for the UI, I realize that for libvirt this is more or less a tristate
> value of on/off/default, but since qemu is the only implementer so far, just
> use a checkbox in the UI and have default == off.
Ok, using a check box is certainly simpler than a combo box.
>> [...]
>> + # Disk removable combo
>> + disk_removable = self.widget("disk-removable")
>> + uihelpers.build_disk_removable_combo(self.vm, disk_removable)
>> +
>
> uihelpers should only be for shared code. since this UI isn't shared, open
> code that bit here.
Good to know. I just tried to follow what seemed the convention for
other combo boxes (they all have a build_*_combo in uihelpers, is that a
mistake?). It won't be necessary since we'll move to a checkbox, though.
>> [...]
>> + self.widget("disk-removable").set_sensitive(is_usb and self.conn.is_qemu())
>
> Rather than desensitive it, I'd prefer to hide it. You can do
>
> can_set_removable = (is_usb and (self.conn.is_qemu() or self.conn.is_test_conn())
> uihelpers.set_grid_row_visible(self.widget("disk-removable"), can_set_removable)
Alright, thanks!
> Probably also want to show it if there's an explicit removable setting already
> set to future proof it a wee bit.
I assume by "explicit" you mean when 'removable' is set to 'on' or
'off', i.e. non-default? If so, I'll do it, but it feels a bit strange
as it fails when a device has the default value, which ought to be by
far the most probable scenario (the exception being when you've manually
edited the XML or similar before firing up virt-manager).
Also, as alluded to above, this will be impossible if you want me to
make 'removable' into a bool instead of the tristate. Due to the point
made in my previous paragraph, that isn't much of a problem, IMHO.
Or have I misunderstood you completely?
Cheers!
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
