[virt-tools-list] [PATCH 4/5] virt-manager: Add checkbox controlling disk 'removable' flag

[Cole Robinson]

On 10/03/2013 09:39 PM, Fred A. Kemp wrote:
> From: "Fred A. Kemp" <anonym at riseup.net>
> 
> ---
>  ui/details.ui          |   36 +++++++++++++++++++++++++++++++++++-
>  virtManager/details.py |   20 +++++++++++++++++++-
>  virtManager/domain.py  |    4 ++++
>  3 files changed, 58 insertions(+), 2 deletions(-)
> 

ACK, I adjusted this to work with my table->grid refactoring and pushed it
(with proper attribution).

One note:

> @@ -2157,6 +2159,12 @@ class vmmDetails(vmmGObjectUI):
>              add_define(self.vm.define_disk_shareable,
>                         dev_id_info, do_shareable)
>  
> +        if self.edited(EDIT_DISK_REMOVABLE):
> +            do_removable = self.widget("disk-removable").get_active()
> +            if not do_removable:
> +                do_removable = None
> +            add_define(self.vm.define_disk_removable, dev_id_info, do_removable)
> +
>          if self.edited(EDIT_DISK_CACHE):
>              cache = self.get_combo_entry("disk-cache")
>              add_define(self.vm.define_disk_cache, dev_id_info, cache)

This is probably where the confusion comes from. We don't want to unset
removeable when the checkbox is False, we want to explicitly set
removable=off. Yeah this could be a little magical if the user clicks the
check button twice, then hits apply, we change the XML rather than leaving it
where it is, but I don't think that matters in practice, and I'd rather keep
the UI simple than deal with the subtle tri-state nature of of elements like
these (really you could possibly maybe call it a libvirt bug, in that it
should fill the hypervisor default into the XML so we know what's going on,
but there's places like that all over the code and tracking qemu defaults can
be difficult over the long term so there are side effects to that as well).

- Cole