[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [PATCH RFC] Add support for the new 'removable' disk flag.



On 10/02/2013 10:39 AM, Fred A. Kemp wrote:
> From: "Fred A. Kemp" <anonym 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]