[virt-tools-list] [PATCH RFC] Add support for the new 'removable' disk flag.
Cole Robinson
crobinso at redhat.com
Thu Oct 3 16:09:04 UTC 2013
On 10/02/2013 10:39 AM, Fred A. Kemp wrote:
> From: "Fred A. Kemp" <anonym at riseup.net>
>
> ---
> ui/details.ui | 39 ++++++++++++++++++++++++++++++++++++++-
> virtManager/details.py | 16 +++++++++++++++-
> virtManager/domain.py | 4 ++++
> virtManager/uihelpers.py | 13 +++++++++++++
> virtinst/devicedisk.py | 1 +
> 5 files changed, 71 insertions(+), 2 deletions(-)
>
Please split this into two patches: one for virtinst and virt-install and one
for the virt-manager bits.
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. Also I suspect the test suite is broken after the virtinst
change, python setup.py test. Should be a couple line addition to fix, see
tests/xmlparse.py testAlterDisk, see the recent commit for startupPolicy as a
reference for all of this.
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.
> diff --git a/ui/details.ui b/ui/details.ui
> index 5d1917f..73fe127 100644
> --- a/ui/details.ui
> +++ b/ui/details.ui
> @@ -3575,7 +3575,7 @@
> <property name="visible">True</property>
> <property name="can_focus">False</property>
> <property name="border_width">3</property>
> - <property name="n_rows">5</property>
> + <property name="n_rows">6</property>
> <property name="n_columns">2</property>
> <property name="column_spacing">8</property>
> <property name="row_spacing">4</property>
> @@ -3612,6 +3612,22 @@
> </packing>
> </child>
> <child>
> + <object class="GtkLabel" id="disk-removable-label">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="xalign">1</property>
> + <property name="label" translatable="yes">Removab_le:</property>
> + <property name="use_underline">True</property>
> + <property name="mnemonic_widget">disk-removable</property>
> + </object>
> + <packing>
> + <property name="top_attach">5</property>
> + <property name="bottom_attach">6</property>
> + <property name="x_options">GTK_FILL</property>
> + <property name="y_options">GTK_FILL</property>
> + </packing>
> + </child>
> + <child>
> <object class="GtkCheckButton" id="disk-readonly">
> <property name="visible">True</property>
> <property name="can_focus">True</property>
> @@ -3648,6 +3664,27 @@
> </packing>
> </child>
> <child>
> + <object class="GtkComboBox" id="disk-removable">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="has_entry">True</property>
> + <signal name="changed" handler="on_disk_removable_combo_changed" swapped="no"/>
> + <child internal-child="entry">
> + <object class="GtkEntry" id="combobox-entry-disk-removable">
> + <property name="can_focus">True</property>
> + </object>
> + </child>
> + </object>
> + <packing>
> + <property name="left_attach">1</property>
> + <property name="right_attach">2</property>
> + <property name="top_attach">5</property>
> + <property name="bottom_attach">6</property>
> + <property name="x_options">GTK_FILL</property>
> + <property name="y_options">GTK_FILL</property>
> + </packing>
> + </child>
> + <child>
> <object class="GtkLabel" id="label4">
> <property name="visible">True</property>
> <property name="can_focus">False</property>
> diff --git a/virtManager/details.py b/virtManager/details.py
> index 1b3f6fd..e0212fe 100644
> --- a/virtManager/details.py
> +++ b/virtManager/details.py
> @@ -66,6 +66,7 @@ EDIT_INIT,
>
> EDIT_DISK_RO,
> EDIT_DISK_SHARE,
> +EDIT_DISK_REMOVABLE,
> EDIT_DISK_CACHE,
> EDIT_DISK_IO,
> EDIT_DISK_BUS,
> @@ -94,7 +95,7 @@ EDIT_WATCHDOG_ACTION,
> EDIT_CONTROLLER_MODEL,
>
> EDIT_TPM_TYPE,
> -) = range(1, 41)
> +) = range(1, 42)
>
>
> # Columns in hw list model
> @@ -475,6 +476,7 @@ class vmmDetails(vmmGObjectUI):
>
> "on_disk_readonly_changed": lambda *x: self.enable_apply(x, EDIT_DISK_RO),
> "on_disk_shareable_changed": lambda *x: self.enable_apply(x, EDIT_DISK_SHARE),
> + "on_disk_removable_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_REMOVABLE),
> "on_disk_cache_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_CACHE),
> "on_disk_io_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_IO),
> "on_disk_bus_combo_changed": lambda *x: self.enable_apply(x, EDIT_DISK_BUS),
> @@ -958,6 +960,10 @@ class vmmDetails(vmmGObjectUI):
> if not (self.conn.is_qemu() or self.conn.is_test_conn()):
> self.widget("iotune-expander").set_visible(False)
>
> + # 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.
> # Network source
> net_source = self.widget("network-source")
> net_bridge = self.widget("network-bridge-box")
> @@ -2157,6 +2163,10 @@ class vmmDetails(vmmGObjectUI):
> add_define(self.vm.define_disk_shareable,
> dev_id_info, do_shareable)
>
> + if self.edited(EDIT_DISK_REMOVABLE):
> + do_removable = self.get_combo_entry("disk-removable")
> + 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)
> @@ -2797,6 +2807,7 @@ class vmmDetails(vmmGObjectUI):
> ro = disk.read_only
> share = disk.shareable
> bus = disk.bus
> + removable = disk.removable
> addr = disk.address.type
> idx = disk.disk_bus_index
> cache = disk.driver_cache
> @@ -2828,6 +2839,7 @@ class vmmDetails(vmmGObjectUI):
>
> is_cdrom = (devtype == virtinst.VirtualDisk.DEVICE_CDROM)
> is_floppy = (devtype == virtinst.VirtualDisk.DEVICE_FLOPPY)
> + is_usb = (bus == "usb")
>
> if addr == "spapr-vio":
> bus = "spapr-vscsi"
> @@ -2840,6 +2852,8 @@ class vmmDetails(vmmGObjectUI):
> self.widget("disk-readonly").set_active(ro)
> self.widget("disk-readonly").set_sensitive(not is_cdrom)
> self.widget("disk-shareable").set_active(share)
> + self.set_combo_entry("disk-removable", removable)
> + 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)
Probably also want to show it if there's an explicit removable setting already
set to future proof it a wee bit.
Thanks,
Cole
More information about the virt-tools-list
mailing list