Patches 2 and 4 add support for changing an LV's VG and for creating new LVs when the only free space is in a preexisting VG, respectively.
The other two are trivial.
--- pyanaconda/storage/__init__.py | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py index 185fabb..4b3ba0f 100644 --- a/pyanaconda/storage/__init__.py +++ b/pyanaconda/storage/__init__.py @@ -1130,7 +1130,7 @@ class Storage(object): swap=swap, mountpoint=mountpoint)
- if name in self.names: + if "%s-%s" % (vg.name, name) in self.names: raise ValueError("name already in use")
return LVMLogicalVolumeDevice(name, *args, **kwargs)
You can choose from any of the defined VGs or you can create a new VG.
Also provided is some basic editing for new VGs: name and disk set. --- pyanaconda/storage/__init__.py | 46 +++++- pyanaconda/ui/gui/spokes/custom.glade | 270 ++++++++++++++++++++++++++++++++- pyanaconda/ui/gui/spokes/custom.py | 256 ++++++++++++++++++++++++++++++- 3 files changed, 560 insertions(+), 12 deletions(-)
diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py index 4b3ba0f..07f2cce 100644 --- a/pyanaconda/storage/__init__.py +++ b/pyanaconda/storage/__init__.py @@ -1832,6 +1832,10 @@ class Storage(object): # short-circuit the logic below for partitions return factory.member_list
+ if container and container.exists: + # don't try to modify an existing container + return container.parents + if factory.container_size_func is None: return []
@@ -1988,9 +1992,17 @@ class Storage(object): self, size, [d.name for d in disks], raid_level)) return factory_class(self, size, disks, raid_level, encrypted)
- def getContainer(self, factory, device=None): + def getContainer(self, factory, device=None, container_name=None): + # XXX would it be useful to implement this as a series of fallbacks + # instead of mutually exclusive branches? container = None - if device: + if container_name: + container = self.devicetree.getDeviceByName(container_name) + if container and container not in factory.container_list: + log.debug("specified container name %s is wrong type (%s)" + % (container_name, container.type)) + container = None + elif device: if hasattr(device, "vg"): container = device.vg elif hasattr(device, "volume"): @@ -2032,6 +2044,9 @@ class Storage(object):
raid_level (btrfs/md/lvm only) RAID level (string)
+ name name for new device + container_name name of requested container + device an already-defined but non-existent device to adjust instead of creating a new device
@@ -2059,11 +2074,19 @@ class Storage(object): disks = kwargs.get("disks") encrypted = kwargs.get("encrypted", self.data.autopart.encrypted)
+ name = kwargs.get("name") + container_name = kwargs.get("container_name") + device = kwargs.get("device")
# md, btrfs raid_level = kwargs.get("raid_level")
+ # we can't do anything with existing devices + if device and device.exists: + log.info("newDevice refusing to change device %s" % device) + return + if not fstype: fstype = self.getFSType(mountpoint=mountpoint) if fstype == "swap": @@ -2082,7 +2105,10 @@ class Storage(object): raise StorageError("no disks specified for new device")
self.size_sets = [] # clear this since there are no growable reqs now - container = self.getContainer(factory, device=device) + + + container = self.getContainer(factory, device=device, + container_name=container_name)
# TODO: striping, mirroring, &c # TODO: non-partition members (pv-on-md) @@ -2129,8 +2155,12 @@ class Storage(object): # set up container if not container and factory.new_container_attr: log.debug("creating new container") + if container_name: + kwa = {"name": container_name} + else: + kwa = {} try: - container = factory.new_container(parents=parents) + container = factory.new_container(parents=parents, **kwa) except StorageError as e: log.error("failed to create new device: %s" % e) # Clean up by destroying the newly created member devices. @@ -2213,12 +2243,18 @@ class Storage(object): else: self.__cleanUpMemberDevices(parents)
+ if name: + kwa = {"name": name} + else: + kwa = {} + try: device = factory.new_device(parents=parents, size=size, fmt_type=fstype, mountpoint=mountpoint, - fmt_args=fmt_args) + fmt_args=fmt_args, + **kwa) except StorageError as e: log.error("device instance creation failed: %s" % e) _container_post_error() diff --git a/pyanaconda/ui/gui/spokes/custom.glade b/pyanaconda/ui/gui/spokes/custom.glade index 70ec694..f4e27eb 100644 --- a/pyanaconda/ui/gui/spokes/custom.glade +++ b/pyanaconda/ui/gui/spokes/custom.glade @@ -1125,7 +1125,7 @@ you'll be able to view their details here.</property> </object> <packing> <property name="left_attach">0</property> - <property name="top_attach">2</property> + <property name="top_attach">3</property> <property name="width">3</property> <property name="height">1</property> </packing> @@ -1147,6 +1147,52 @@ you'll be able to view their details here.</property> <property name="height">1</property> </packing> </child> + <child> + <object class="GtkLabel" id="volumeGroupLabel"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="label" translatable="yes">Volume Group:</property> + <attributes> + <attribute name="weight" value="bold"/> + </attributes> + </object> + <packing> + <property name="left_attach">0</property> + <property name="top_attach">2</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkComboBoxText" id="volumeGroupCombo"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="entry_text_column">0</property> + <property name="id_column">1</property> + <signal name="changed" handler="on_vg_changed" swapped="no"/> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">2</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkButton" id="modifyVGButton"> + <property name="label" translatable="yes">Modify ...</property> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <signal name="clicked" handler="on_modify_vg_clicked" swapped="no"/> + </object> + <packing> + <property name="left_attach">2</property> + <property name="top_attach">2</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> </object> </child> <child type="label"> @@ -1683,6 +1729,228 @@ you'll be able to view their details here.</property> </object> </child> </object> + <object class="GtkDialog" id="vg_dialog"> + <property name="can_focus">False</property> + <property name="border_width">5</property> + <property name="type_hint">dialog</property> + <property name="decorated">False</property> + <child internal-child="vbox"> + <object class="GtkBox" id="dialog-vbox5"> + <property name="can_focus">False</property> + <property name="orientation">vertical</property> + <property name="spacing">6</property> + <child internal-child="action_area"> + <object class="GtkButtonBox" id="dialog-action_area5"> + <property name="can_focus">False</property> + <property name="layout_style">end</property> + <child> + <object class="GtkButton" id="vg_cancel_button"> + <property name="label" translatable="yes">_Cancel</property> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <property name="use_underline">True</property> + <signal name="clicked" handler="on_cancel_clicked" swapped="no"/> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkButton" id="vg_save_button"> + <property name="label" translatable="yes">_Save</property> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <property name="use_underline">True</property> + <signal name="clicked" handler="on_save_clicked" swapped="no"/> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="pack_type">end</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkBox" id="box10"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="orientation">vertical</property> + <property name="spacing">6</property> + <child> + <object class="GtkLabel" id="configure_vg_title_label"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="xalign">0</property> + <property name="label" translatable="yes">CONFIGURE VOLUME GROUP</property> + <attributes> + <attribute name="weight" value="bold"/> + </attributes> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkLabel" id="label14"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="label" translatable="yes">Please create a name for your volume group and select at least one disk below.</property> + <property name="wrap">True</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + <child> + <object class="GtkBox" id="box11"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="spacing">6</property> + <child> + <object class="GtkLabel" id="label15"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="label" translatable="yes">Name:</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkEntry" id="vg_name_entry"> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="invisible_char">●</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">2</property> + </packing> + </child> + <child> + <object class="GtkScrolledWindow" id="scrolledwindow2"> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="hexpand">True</property> + <property name="vexpand">True</property> + <property name="shadow_type">in</property> + <child> + <object class="GtkViewport" id="viewport2"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <child> + <object class="GtkTreeView" id="vg_disk_view"> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="model">disk_store</property> + <property name="search_column">0</property> + <child internal-child="selection"> + <object class="GtkTreeSelection" id="treeview-selection3"> + <property name="mode">multiple</property> + </object> + </child> + <child> + <object class="GtkTreeViewColumn" id="disk_column1"> + <property name="spacing">6</property> + <property name="title" translatable="yes">Disk</property> + <property name="clickable">True</property> + <child> + <object class="GtkCellRendererText" id="disk_renderer1"/> + <attributes> + <attribute name="text">0</attribute> + </attributes> + </child> + </object> + </child> + <child> + <object class="GtkTreeViewColumn" id="capacity_column1"> + <property name="spacing">6</property> + <property name="title" translatable="yes">Capacity</property> + <property name="clickable">True</property> + <child> + <object class="GtkCellRendererText" id="capacity_renderer1"/> + <attributes> + <attribute name="text">1</attribute> + </attributes> + </child> + </object> + </child> + <child> + <object class="GtkTreeViewColumn" id="free_column1"> + <property name="spacing">6</property> + <property name="title" translatable="yes">Free</property> + <property name="clickable">True</property> + <child> + <object class="GtkCellRendererText" id="free_renderer1"/> + <attributes> + <attribute name="text">2</attribute> + </attributes> + </child> + </object> + </child> + <child> + <object class="GtkTreeViewColumn" id="id_column1"> + <property name="spacing">6</property> + <property name="title" translatable="yes">ID</property> + <property name="clickable">True</property> + <child> + <object class="GtkCellRendererText" id="id_renderer1"/> + <attributes> + <attribute name="text">3</attribute> + </attributes> + </child> + </object> + </child> + </object> + </child> + </object> + </child> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">3</property> + </packing> + </child> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + </object> + </child> + <action-widgets> + <action-widget response="0">vg_cancel_button</action-widget> + <action-widget response="1">vg_save_button</action-widget> + </action-widgets> + </object> <object class="GtkListStore" id="disk_store"> <columns> <!-- column-name disk_col --> diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index fe2d9b0..21501af 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -21,13 +21,17 @@ #
# TODO: -# - empty size when adding a mountpoint is no longer using all available space +# - Add a way for users to specify the names of md, lv, subvol devices. +# - We should either remove boot disk selection from the cart we show from here +# or re-partition when it gets changed to make a best-effort at keeping up. +# - Add new LVs to existing VG if creating a new VG fails outright. # - Deleting an LV is not reflected in available space in the bottom left. # - this is only true for preexisting LVs # - Device descriptions, suggested sizes, etc. should be moved out into a support file. # - Tabbing behavior in the accordion is weird. # - Update feature space costs when size spinner changes. -# - Either disable stripe/mirror for LVM or implement it for device creation. +# - Implement striping and mirroring for LVM. +# - Implement container management for btrfs. # - If you click to add a mountpoint while editing a device the lightbox # screenshot is taken prior to the ui update so the background shows the old # size and free space while you're deciding on a size for the new device. @@ -92,6 +96,7 @@ NOTEBOOK_LUKS_PAGE = 2 NOTEBOOK_UNEDITABLE_PAGE = 3
new_install_name = N_("New %s %s Installation") % (productName, productVersion) +new_vg_text = N_("Create a new volume group ...") unrecoverable_error_msg = N_("Storage configuration reset due to unrecoverable " "error. Click for details.") device_configuration_error_msg = N_("Device reconfiguration failed. Click for " @@ -340,6 +345,69 @@ class DisksDialog(GUIObject): def run(self): return self.window.run()
+class VolumeGroupDialog(GUIObject): + builderObjects = ["vg_dialog", "disk_store", "vg_disk_view"] + mainWidgetName = "vg_dialog" + uiFile = "spokes/custom.glade" + + def __init__(self, *args, **kwargs): + self._disks = kwargs.pop("disks") + free = kwargs.pop("free") + self.name = kwargs.pop("name") or "" + self.selected = kwargs.pop("selected")[:] + GUIObject.__init__(self, *args, **kwargs) + + self.builder.get_object("vg_name_entry").set_text(self.name) + + self._store = self.builder.get_object("disk_store") + # populate the store + for disk in self._disks: + self._store.append([disk.description, + str(Size(spec="%dMB" % disk.size)), + str(free[disk.name][0]), + disk.serial, + disk.id]) + + treeview = self.builder.get_object("vg_disk_view") + model = treeview.get_model() + itr = model.get_iter_first() + + selected_ids = [d.id for d in self.selected] + log.debug("selected: %s" % [d.name for d in self.selected]) + log.debug("selected: %s" % selected_ids) + selection = treeview.get_selection() + while itr: + disk_id = model.get_value(itr, 4) + log.debug("store: %d" % disk_id) + if disk_id in selected_ids: + selection.select_iter(itr) + + itr = model.iter_next(itr) + + def on_cancel_clicked(self, button): + self.window.destroy() + + def _get_disk_by_id(self, disk_id): + for disk in self._disks: + if disk.id == disk_id: + return disk + + def on_save_clicked(self, button): + treeview = self.builder.get_object("vg_disk_view") + model, paths = treeview.get_selection().get_selected_rows() + self.selected = [] + for path in paths: + itr = model.get_iter(path) + disk_id = model.get_value(itr, 4) + self.selected.append(self._get_disk_by_id(disk_id)) + + self.name = self.builder.get_object("vg_name_entry").get_text() + + self.window.destroy() + + def run(self): + return self.window.run() + class CustomPartitioningSpoke(NormalSpoke, StorageChecker): builderObjects = ["customStorageWindow", "sizeAdjustment", "partitionStore", @@ -360,7 +428,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._fs_types = [] # list of supported fstypes self._unused_devices = None # None indicates uninitialized self._free_space = Size(bytes=0) + self._device_disks = [] + self._device_container_name = None
self._initialized = False
@@ -826,7 +896,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): prev_encrypted = device.encrypted log.debug("old encryption setting: %s" % prev_encrypted) encryption_checkbox = self.builder.get_object("encryptCheckbox") - encrypted = None encrypted = encryption_checkbox.get_active()
changed_encryption = (prev_encrypted != encrypted) @@ -893,6 +962,13 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._populate_right_side(selector) return
+ # XXX Now we have to do something squirrely with the encryption setting. + # We've done the checks for things that cannot be on an encrypted + # device. From here on out, it means "encrypt new devices we create", + # which we don't want to do in the event that encryption is already + # present below the surface of an existing device stack. + encrypted = encrypted and encryption_checkbox.get_sensitive() + with ui_storage_logger(): # create a new factory using the appropriate size and type factory = self.__storage.getDeviceFactory(device_type, size, @@ -919,9 +995,22 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): old_disk_set = device.disks if hasattr(device, "req_disks") and not device.exists: old_disk_set = device.req_disks + changed_disk_set = (set(old_disk_set) != set(self._device_disks))
- if changed_device_type or changed_raid_level: + changed_container = False + old_container_name = None + container = self.__storage.getContainer(factory) + if not changed_device_type and device_type == DEVICE_TYPE_LVM: + old_container = self.__storage.getContainer(factory, + device=device) + container = self.__storage.getContainer(factory, + container_name=self._device_container_name) + if self._device_container_name != old_container.name: + old_container_name = old_container.name + changed_container = True + + if changed_device_type or changed_raid_level or changed_container: if changed_device_type: log.info("changing device type from %s to %s" % (current_device_type, device_type)) @@ -935,6 +1024,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): % ([d.name for d in device.disks], [d.name for d in self._device_disks]))
+ if changed_container: + log.info("changing container from %s to %s" + % (old_container.name, self._device_container_name)) + # remove the current device self.clear_errors() root = self._current_selector._root @@ -945,7 +1038,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
with ui_storage_logger(): disks = self._device_disks[:] - container = self.__storage.getContainer(factory) if container and changed_device_type: log.debug("overriding disk set with container's") disks = container.disks[:] @@ -955,6 +1047,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): disks=disks, mountpoint=mountpoint, label=label, raid_level=raid_level, encrypted=encrypted, + container_name=self._device_container_name, selector=selector) except ErrorRecoveryFailure as e: self._error = e @@ -979,6 +1072,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): label=old_label, raid_level=old_raid_level, encrypted=prev_encrypted, + container_name=old_container_name, selector=selector) except StorageError as e: # failed to recover. @@ -1043,6 +1137,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): device=device, disks=self._device_disks[:], encrypted=encrypted, + container_name=self._device_container_name, raid_level=raid_level) except ErrorRecoveryFailure as e: self._error = e @@ -1368,6 +1463,13 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
log.debug("updated device_disks to %s" % [d.name for d in self._device_disks])
+ if hasattr(use_dev, "vg"): + self._device_container_name = use_dev.vg.name + else: + self._device_container_name = None + + log.debug("updated device_vg_name to %s" % self._device_container_name) + selectedDeviceLabel.set_text(selector.props.name) selectedDeviceDescLabel.set_text(self._description(selector.props.name))
@@ -1522,9 +1624,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): typeCombo.set_active(type_index_map[device_type])
# you can't change the type of an existing device - typeCombo.set_sensitive(not device.exists) + typeCombo.set_sensitive(not use_dev.exists)
self._populate_raid(raid_level, device.size) + self._populate_lvm(device=use_dev) self.builder.get_object("optionsNotebook").set_sensitive(not device.exists)
### @@ -1807,6 +1910,94 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
self._device_disks = disks
+ def run_vg_editor(self, vg=None, name=None): + if vg: + vg_name = vg.name + elif name: + vg_name = name + + dialog = VolumeGroupDialog(self.data, + name=vg_name, + disks=self._clearpartDevices, + free=self._currentFreeInfo, + selected=self._device_disks) + + with enlightbox(self.window, dialog.window): + rc = dialog.run() + + if rc == 0: + return + + disks = dialog.selected + name = dialog.name + log.debug("new disks for %s: %s" % (name, [d.name for d in disks])) + if not disks: + self._error = "No disks selected. Not saving changes." + self.window.set_info(Gtk.MessageType.INFO, + self._error) + self.window.show_all() + return + + log.debug("new VG name: %s" % name) + if name != vg_name and name in self.__storage.names: + self._error = _("Volume Group name %s is already in use. Not " + "saving changes.") + self.window.set_info(Gtk.MessageType.INFO, + self._error) + self.window.show_all() + return + + self._device_disks = disks + self._device_container_name = name + + def on_modify_vg_clicked(self, button): + vg_name = self.builder.get_object("volumeGroupCombo").get_active_text() + + vg = self.__storage.devicetree.getDeviceByName(vg_name) + + # pass the name along with any found vg since we could be modifying a + # vg that hasn't been instantiated yet + self.run_vg_editor(vg=vg, name=vg_name) + + log.debug("%s -> %s" % (vg_name, self._device_container_name)) + if vg_name == self._device_container_name: + return + + log.debug("renaming VG %s to %s" % (vg_name, self._device_container_name)) + if vg: + self.__storage.devicetree.names.remove(vg.name) + self.__storage.devicetree.names.append(self._device_container_name) + vg.name = self._device_container_name + + vg_combo = self.builder.get_object("volumeGroupCombo") + for idx, data in enumerate(vg_combo.get_model()): + # we're looking for the original vg name + if data[0] == vg_name: + vg_combo.remove(idx) + vg_combo.insert_text(idx, self._device_container_name) + vg_combo.set_active(idx) + break + + def on_vg_changed(self, combo): + vg_name = combo.get_active_text() + log.debug("new vg selection: %s" % vg_name) + if vg_name == new_vg_text: + # run the vg editor dialog with a default name and disk set + hostname = self.data.network.hostname + name = self.__storage.suggestContainerName(hostname=hostname) + self.run_vg_editor(name=name) + for idx, data in enumerate(combo.get_model()): + if data[0] == new_vg_text: + combo.insert_text(idx, self._device_container_name) + combo.set_active(idx) + break + else: + self._device_container_name = vg_name + + vg = self.__storage.devicetree.getDeviceByName(self._device_container_name) + vg_exists = getattr(vg, "exists", False) # might not be in the tree + self.builder.get_object("modifyVGButton").set_sensitive(not vg_exists) + def on_selector_clicked(self, selector): if not self._initialized: return @@ -1971,6 +2162,55 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): hasattr(fmt, "label")) mountPointEntry.set_sensitive(fmt.mountable)
+ def _populate_lvm(self, device=None): + # + # set up the volume group widgets for lvm or hide them for other types + # + device_type = self._get_current_device_type() + if device is None: + if self._current_selector is None: + return + + device = self._current_selector._device + + vg_combo = self.builder.get_object("volumeGroupCombo") + vg_button = self.builder.get_object("modifyVGButton") + vg_label = self.builder.get_object("volumeGroupLabel") + if device_type == DEVICE_TYPE_LVM: + # set up the vg widgets and then bail out + if self._device_container_name: + default_vg = self._device_container_name + else: + with ui_storage_logger(): + factory = self.__storage.getDeviceFactory(DEVICE_TYPE_LVM, + 0) + container = self.__storage.getContainer(factory) + default_vg = getattr(container, "name", None) + + vg_combo.remove_all() + vgs = self.__storage.vgs + for vg in vgs: + vg_combo.append_text(vg.name) + if default_vg and vg.name == default_vg: + vg_combo.set_active(vgs.index(vg)) + + vg_combo.append_text(new_vg_text) + if default_vg is None: + vg_combo.set_active(len(vg_combo.get_model())) + #vg_combo.set_active(-1) + + vg_combo.show() + vg_button.show() + vg_label.show() + + # make the combo and button insensitive for existing LVs + can_change_vg = (device is not None and not device.exists) + vg_combo.set_sensitive(can_change_vg) + else: + vg_label.hide() + vg_combo.hide() + vg_button.hide() + def on_device_type_changed(self, combo): if not self._initialized: return @@ -2011,8 +2251,12 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): elif new_type == DEVICE_TYPE_MD: raid_level = "raid0"
+ # lvm uses the RHS to set disk set. no foolish minds here. + self._configButton.set_sensitive(new_type != DEVICE_TYPE_LVM) + size = self.builder.get_object("sizeSpinner").get_value() self._populate_raid(raid_level, size) + self._populate_lvm()
# begin btrfs magic fsCombo = self.builder.get_object("fileSystemTypeCombo")
See inline comments.
On Wed, 2012-11-21 at 13:44 -0600, David Lehman wrote:
You can choose from any of the defined VGs or you can create a new VG.
Also provided is some basic editing for new VGs: name and disk set.
pyanaconda/storage/__init__.py | 46 +++++- pyanaconda/ui/gui/spokes/custom.glade | 270 ++++++++++++++++++++++++++++++++- pyanaconda/ui/gui/spokes/custom.py | 256 ++++++++++++++++++++++++++++++- 3 files changed, 560 insertions(+), 12 deletions(-)
diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py index 4b3ba0f..07f2cce 100644 --- a/pyanaconda/storage/__init__.py +++ b/pyanaconda/storage/__init__.py @@ -1832,6 +1832,10 @@ class Storage(object): # short-circuit the logic below for partitions return factory.member_list
if container and container.exists:
# don't try to modify an existing container
return container.parents
if factory.container_size_func is None: return []
@@ -1988,9 +1992,17 @@ class Storage(object): self, size, [d.name for d in disks], raid_level)) return factory_class(self, size, disks, raid_level, encrypted)
- def getContainer(self, factory, device=None):
- def getContainer(self, factory, device=None, container_name=None):
# XXX would it be useful to implement this as a series of fallbacks
# instead of mutually exclusive branches? container = None
if device:
if container_name:
container = self.devicetree.getDeviceByName(container_name)
if container and container not in factory.container_list:
log.debug("specified container name %s is wrong type (%s)"
% (container_name, container.type))
container = None
elif device: if hasattr(device, "vg"): container = device.vg elif hasattr(device, "volume"):
@@ -2032,6 +2044,9 @@ class Storage(object):
raid_level (btrfs/md/lvm only) RAID level (string)
name name for new device
container_name name of requested container
device an already-defined but non-existent device to adjust instead of creating a new device
@@ -2059,11 +2074,19 @@ class Storage(object): disks = kwargs.get("disks") encrypted = kwargs.get("encrypted", self.data.autopart.encrypted)
name = kwargs.get("name")
container_name = kwargs.get("container_name")
device = kwargs.get("device") # md, btrfs raid_level = kwargs.get("raid_level")
# we can't do anything with existing devices
if device and device.exists:
log.info("newDevice refusing to change device %s" % device)
return
if not fstype: fstype = self.getFSType(mountpoint=mountpoint) if fstype == "swap":
@@ -2082,7 +2105,10 @@ class Storage(object): raise StorageError("no disks specified for new device")
self.size_sets = [] # clear this since there are no growable reqs now
container = self.getContainer(factory, device=device)
container = self.getContainer(factory, device=device,
container_name=container_name)
Are two empty lines needed here?
# TODO: striping, mirroring, &c # TODO: non-partition members (pv-on-md)
@@ -2129,8 +2155,12 @@ class Storage(object): # set up container if not container and factory.new_container_attr: log.debug("creating new container")
if container_name:
kwa = {"name": container_name}
else:
kwa = {} try:
container = factory.new_container(parents=parents)
container = factory.new_container(parents=parents, **kwa) except StorageError as e: log.error("failed to create new device: %s" % e) # Clean up by destroying the newly created member devices.
@@ -2213,12 +2243,18 @@ class Storage(object): else: self.__cleanUpMemberDevices(parents)
if name:
kwa = {"name": name}
else:
kwa = {}
try: device = factory.new_device(parents=parents, size=size, fmt_type=fstype, mountpoint=mountpoint,
fmt_args=fmt_args)
fmt_args=fmt_args,
**kwa) except StorageError as e: log.error("device instance creation failed: %s" % e) _container_post_error()
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index fe2d9b0..21501af 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -21,13 +21,17 @@ #
# TODO: -# - empty size when adding a mountpoint is no longer using all available space +# - Add a way for users to specify the names of md, lv, subvol devices. +# - We should either remove boot disk selection from the cart we show from here +# or re-partition when it gets changed to make a best-effort at keeping up. +# - Add new LVs to existing VG if creating a new VG fails outright. # - Deleting an LV is not reflected in available space in the bottom left. # - this is only true for preexisting LVs # - Device descriptions, suggested sizes, etc. should be moved out into a support file. # - Tabbing behavior in the accordion is weird. # - Update feature space costs when size spinner changes. -# - Either disable stripe/mirror for LVM or implement it for device creation. +# - Implement striping and mirroring for LVM. +# - Implement container management for btrfs. # - If you click to add a mountpoint while editing a device the lightbox # screenshot is taken prior to the ui update so the background shows the old # size and free space while you're deciding on a size for the new device. @@ -92,6 +96,7 @@ NOTEBOOK_LUKS_PAGE = 2 NOTEBOOK_UNEDITABLE_PAGE = 3
new_install_name = N_("New %s %s Installation") % (productName, productVersion) +new_vg_text = N_("Create a new volume group ...") unrecoverable_error_msg = N_("Storage configuration reset due to unrecoverable " "error. Click for details.") device_configuration_error_msg = N_("Device reconfiguration failed. Click for " @@ -340,6 +345,69 @@ class DisksDialog(GUIObject): def run(self): return self.window.run()
+class VolumeGroupDialog(GUIObject):
- builderObjects = ["vg_dialog", "disk_store", "vg_disk_view"]
- mainWidgetName = "vg_dialog"
- uiFile = "spokes/custom.glade"
- def __init__(self, *args, **kwargs):
self._disks = kwargs.pop("disks")
free = kwargs.pop("free")
self.name = kwargs.pop("name") or ""
I believe you want kwargs.pop("name", "") here. Or could kwargs contain "name": None?
self.selected = kwargs.pop("selected")[:]
GUIObject.__init__(self, *args, **kwargs)
self.builder.get_object("vg_name_entry").set_text(self.name)
self._store = self.builder.get_object("disk_store")
# populate the store
for disk in self._disks:
self._store.append([disk.description,
str(Size(spec="%dMB" % disk.size)),
str(free[disk.name][0]),
disk.serial,
disk.id])
treeview = self.builder.get_object("vg_disk_view")
model = treeview.get_model()
itr = model.get_iter_first()
selected_ids = [d.id for d in self.selected]
log.debug("selected: %s" % [d.name for d in self.selected])
log.debug("selected: %s" % selected_ids)
selection = treeview.get_selection()
while itr:
disk_id = model.get_value(itr, 4)
log.debug("store: %d" % disk_id)
if disk_id in selected_ids:
selection.select_iter(itr)
itr = model.iter_next(itr)
- def on_cancel_clicked(self, button):
self.window.destroy()
- def _get_disk_by_id(self, disk_id):
for disk in self._disks:
if disk.id == disk_id:
return disk
- def on_save_clicked(self, button):
treeview = self.builder.get_object("vg_disk_view")
model, paths = treeview.get_selection().get_selected_rows()
self.selected = []
for path in paths:
itr = model.get_iter(path)
disk_id = model.get_value(itr, 4)
We use model[itr][4] elsewhere.
self.selected.append(self._get_disk_by_id(disk_id))
self.name = self.builder.get_object("vg_name_entry").get_text()
self.window.destroy()
- def run(self):
return self.window.run()
class CustomPartitioningSpoke(NormalSpoke, StorageChecker): builderObjects = ["customStorageWindow", "sizeAdjustment", "partitionStore", @@ -360,7 +428,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._fs_types = [] # list of supported fstypes self._unused_devices = None # None indicates uninitialized self._free_space = Size(bytes=0)
self._device_disks = []
self._device_container_name = None self._initialized = False
@@ -826,7 +896,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): prev_encrypted = device.encrypted log.debug("old encryption setting: %s" % prev_encrypted) encryption_checkbox = self.builder.get_object("encryptCheckbox")
encrypted = None encrypted = encryption_checkbox.get_active() changed_encryption = (prev_encrypted != encrypted)
@@ -893,6 +962,13 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._populate_right_side(selector) return
# XXX Now we have to do something squirrely with the encryption setting.
# We've done the checks for things that cannot be on an encrypted
# device. From here on out, it means "encrypt new devices we create",
# which we don't want to do in the event that encryption is already
# present below the surface of an existing device stack.
encrypted = encrypted and encryption_checkbox.get_sensitive()
with ui_storage_logger(): # create a new factory using the appropriate size and type factory = self.__storage.getDeviceFactory(device_type, size,
@@ -919,9 +995,22 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): old_disk_set = device.disks if hasattr(device, "req_disks") and not device.exists: old_disk_set = device.req_disks
changed_disk_set = (set(old_disk_set) != set(self._device_disks))
if changed_device_type or changed_raid_level:
changed_container = False
old_container_name = None
container = self.__storage.getContainer(factory)
if not changed_device_type and device_type == DEVICE_TYPE_LVM:
old_container = self.__storage.getContainer(factory,
device=device)
container = self.__storage.getContainer(factory,
container_name=self._device_container_name)
if self._device_container_name != old_container.name:
old_container_name = old_container.name
changed_container = True
if changed_device_type or changed_raid_level or changed_container: if changed_device_type: log.info("changing device type from %s to %s" % (current_device_type, device_type))
@@ -935,6 +1024,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): % ([d.name for d in device.disks], [d.name for d in self._device_disks]))
if changed_container:
log.info("changing container from %s to %s"
% (old_container.name, self._device_container_name))
# remove the current device self.clear_errors() root = self._current_selector._root
@@ -945,7 +1038,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
with ui_storage_logger(): disks = self._device_disks[:]
container = self.__storage.getContainer(factory) if container and changed_device_type: log.debug("overriding disk set with container's") disks = container.disks[:]
@@ -955,6 +1047,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): disks=disks, mountpoint=mountpoint, label=label, raid_level=raid_level, encrypted=encrypted,
container_name=self._device_container_name, selector=selector) except ErrorRecoveryFailure as e: self._error = e
@@ -979,6 +1072,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): label=old_label, raid_level=old_raid_level, encrypted=prev_encrypted,
container_name=old_container_name, selector=selector) except StorageError as e: # failed to recover.
@@ -1043,6 +1137,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): device=device, disks=self._device_disks[:], encrypted=encrypted,
container_name=self._device_container_name, raid_level=raid_level) except ErrorRecoveryFailure as e: self._error = e
@@ -1368,6 +1463,13 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
log.debug("updated device_disks to %s" % [d.name for d in self._device_disks])
if hasattr(use_dev, "vg"):
self._device_container_name = use_dev.vg.name
else:
self._device_container_name = None
log.debug("updated device_vg_name to %s" % self._device_container_name)
selectedDeviceLabel.set_text(selector.props.name) selectedDeviceDescLabel.set_text(self._description(selector.props.name))
@@ -1522,9 +1624,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): typeCombo.set_active(type_index_map[device_type])
# you can't change the type of an existing device
typeCombo.set_sensitive(not device.exists)
typeCombo.set_sensitive(not use_dev.exists) self._populate_raid(raid_level, device.size)
self._populate_lvm(device=use_dev) self.builder.get_object("optionsNotebook").set_sensitive(not device.exists)
###
@@ -1807,6 +1910,94 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
self._device_disks = disks
- def run_vg_editor(self, vg=None, name=None):
if vg:
vg_name = vg.name
elif name:
vg_name = name
dialog = VolumeGroupDialog(self.data,
name=vg_name,
disks=self._clearpartDevices,
free=self._currentFreeInfo,
selected=self._device_disks)
with enlightbox(self.window, dialog.window):
rc = dialog.run()
if rc == 0:
return
disks = dialog.selected
name = dialog.name
log.debug("new disks for %s: %s" % (name, [d.name for d in disks]))
if not disks:
self._error = "No disks selected. Not saving changes."
self.window.set_info(Gtk.MessageType.INFO,
self._error)
self.window.show_all()
return
log.debug("new VG name: %s" % name)
if name != vg_name and name in self.__storage.names:
self._error = _("Volume Group name %s is already in use. Not "
"saving changes.")
self.window.set_info(Gtk.MessageType.INFO,
self._error)
self.window.show_all()
return
self._device_disks = disks
self._device_container_name = name
- def on_modify_vg_clicked(self, button):
vg_name = self.builder.get_object("volumeGroupCombo").get_active_text()
vg = self.__storage.devicetree.getDeviceByName(vg_name)
# pass the name along with any found vg since we could be modifying a
# vg that hasn't been instantiated yet
self.run_vg_editor(vg=vg, name=vg_name)
log.debug("%s -> %s" % (vg_name, self._device_container_name))
if vg_name == self._device_container_name:
return
log.debug("renaming VG %s to %s" % (vg_name, self._device_container_name))
if vg:
self.__storage.devicetree.names.remove(vg.name)
self.__storage.devicetree.names.append(self._device_container_name)
vg.name = self._device_container_name
vg_combo = self.builder.get_object("volumeGroupCombo")
for idx, data in enumerate(vg_combo.get_model()):
# we're looking for the original vg name
if data[0] == vg_name:
vg_combo.remove(idx)
vg_combo.insert_text(idx, self._device_container_name)
vg_combo.set_active(idx)
break
- def on_vg_changed(self, combo):
vg_name = combo.get_active_text()
log.debug("new vg selection: %s" % vg_name)
if vg_name == new_vg_text:
# run the vg editor dialog with a default name and disk set
hostname = self.data.network.hostname
name = self.__storage.suggestContainerName(hostname=hostname)
self.run_vg_editor(name=name)
for idx, data in enumerate(combo.get_model()):
if data[0] == new_vg_text:
combo.insert_text(idx, self._device_container_name)
combo.set_active(idx)
break
else:
self._device_container_name = vg_name
vg = self.__storage.devicetree.getDeviceByName(self._device_container_name)
vg_exists = getattr(vg, "exists", False) # might not be in the tree
self.builder.get_object("modifyVGButton").set_sensitive(not vg_exists)
- def on_selector_clicked(self, selector): if not self._initialized: return
@@ -1971,6 +2162,55 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): hasattr(fmt, "label")) mountPointEntry.set_sensitive(fmt.mountable)
- def _populate_lvm(self, device=None):
#
# set up the volume group widgets for lvm or hide them for other types
#
This could be a docstring.
device_type = self._get_current_device_type()
if device is None:
if self._current_selector is None:
return
device = self._current_selector._device
vg_combo = self.builder.get_object("volumeGroupCombo")
vg_button = self.builder.get_object("modifyVGButton")
vg_label = self.builder.get_object("volumeGroupLabel")
if device_type == DEVICE_TYPE_LVM:
# set up the vg widgets and then bail out
if self._device_container_name:
default_vg = self._device_container_name
else:
with ui_storage_logger():
factory = self.__storage.getDeviceFactory(DEVICE_TYPE_LVM,
0)
container = self.__storage.getContainer(factory)
default_vg = getattr(container, "name", None)
vg_combo.remove_all()
vgs = self.__storage.vgs
for vg in vgs:
vg_combo.append_text(vg.name)
if default_vg and vg.name == default_vg:
vg_combo.set_active(vgs.index(vg))
vg_combo.append_text(new_vg_text)
if default_vg is None:
vg_combo.set_active(len(vg_combo.get_model()))
#vg_combo.set_active(-1)
This could probably go away.
vg_combo.show()
vg_button.show()
vg_label.show()
# make the combo and button insensitive for existing LVs
can_change_vg = (device is not None and not device.exists)
vg_combo.set_sensitive(can_change_vg)
else:
vg_label.hide()
vg_combo.hide()
vg_button.hide()
- def on_device_type_changed(self, combo): if not self._initialized: return
@@ -2011,8 +2251,12 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): elif new_type == DEVICE_TYPE_MD: raid_level = "raid0"
# lvm uses the RHS to set disk set. no foolish minds here.
self._configButton.set_sensitive(new_type != DEVICE_TYPE_LVM)
size = self.builder.get_object("sizeSpinner").get_value() self._populate_raid(raid_level, size)
self._populate_lvm() # begin btrfs magic fsCombo = self.builder.get_object("fileSystemTypeCombo")
On Tue, 2012-11-27 at 13:30 +0100, Vratislav Podzimek wrote:
See inline comments.
On Wed, 2012-11-21 at 13:44 -0600, David Lehman wrote:
You can choose from any of the defined VGs or you can create a new VG.
Also provided is some basic editing for new VGs: name and disk set.
pyanaconda/storage/__init__.py | 46 +++++- pyanaconda/ui/gui/spokes/custom.glade | 270 ++++++++++++++++++++++++++++++++- pyanaconda/ui/gui/spokes/custom.py | 256 ++++++++++++++++++++++++++++++- 3 files changed, 560 insertions(+), 12 deletions(-)
diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py index 4b3ba0f..07f2cce 100644 --- a/pyanaconda/storage/__init__.py +++ b/pyanaconda/storage/__init__.py @@ -1832,6 +1832,10 @@ class Storage(object): # short-circuit the logic below for partitions return factory.member_list
if container and container.exists:
# don't try to modify an existing container
return container.parents
if factory.container_size_func is None: return []
@@ -1988,9 +1992,17 @@ class Storage(object): self, size, [d.name for d in disks], raid_level)) return factory_class(self, size, disks, raid_level, encrypted)
- def getContainer(self, factory, device=None):
- def getContainer(self, factory, device=None, container_name=None):
# XXX would it be useful to implement this as a series of fallbacks
# instead of mutually exclusive branches? container = None
if device:
if container_name:
container = self.devicetree.getDeviceByName(container_name)
if container and container not in factory.container_list:
log.debug("specified container name %s is wrong type (%s)"
% (container_name, container.type))
container = None
elif device: if hasattr(device, "vg"): container = device.vg elif hasattr(device, "volume"):
@@ -2032,6 +2044,9 @@ class Storage(object):
raid_level (btrfs/md/lvm only) RAID level (string)
name name for new device
container_name name of requested container
device an already-defined but non-existent device to adjust instead of creating a new device
@@ -2059,11 +2074,19 @@ class Storage(object): disks = kwargs.get("disks") encrypted = kwargs.get("encrypted", self.data.autopart.encrypted)
name = kwargs.get("name")
container_name = kwargs.get("container_name")
device = kwargs.get("device") # md, btrfs raid_level = kwargs.get("raid_level")
# we can't do anything with existing devices
if device and device.exists:
log.info("newDevice refusing to change device %s" % device)
return
if not fstype: fstype = self.getFSType(mountpoint=mountpoint) if fstype == "swap":
@@ -2082,7 +2105,10 @@ class Storage(object): raise StorageError("no disks specified for new device")
self.size_sets = [] # clear this since there are no growable reqs now
container = self.getContainer(factory, device=device)
container = self.getContainer(factory, device=device,
container_name=container_name)
Are two empty lines needed here?
Fixed locally.
# TODO: striping, mirroring, &c # TODO: non-partition members (pv-on-md)
@@ -2129,8 +2155,12 @@ class Storage(object): # set up container if not container and factory.new_container_attr: log.debug("creating new container")
if container_name:
kwa = {"name": container_name}
else:
kwa = {} try:
container = factory.new_container(parents=parents)
container = factory.new_container(parents=parents, **kwa) except StorageError as e: log.error("failed to create new device: %s" % e) # Clean up by destroying the newly created member devices.
@@ -2213,12 +2243,18 @@ class Storage(object): else: self.__cleanUpMemberDevices(parents)
if name:
kwa = {"name": name}
else:
kwa = {}
try: device = factory.new_device(parents=parents, size=size, fmt_type=fstype, mountpoint=mountpoint,
fmt_args=fmt_args)
fmt_args=fmt_args,
**kwa) except StorageError as e: log.error("device instance creation failed: %s" % e) _container_post_error()
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index fe2d9b0..21501af 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -21,13 +21,17 @@ #
# TODO: -# - empty size when adding a mountpoint is no longer using all available space +# - Add a way for users to specify the names of md, lv, subvol devices. +# - We should either remove boot disk selection from the cart we show from here +# or re-partition when it gets changed to make a best-effort at keeping up. +# - Add new LVs to existing VG if creating a new VG fails outright. # - Deleting an LV is not reflected in available space in the bottom left. # - this is only true for preexisting LVs # - Device descriptions, suggested sizes, etc. should be moved out into a support file. # - Tabbing behavior in the accordion is weird. # - Update feature space costs when size spinner changes. -# - Either disable stripe/mirror for LVM or implement it for device creation. +# - Implement striping and mirroring for LVM. +# - Implement container management for btrfs. # - If you click to add a mountpoint while editing a device the lightbox # screenshot is taken prior to the ui update so the background shows the old # size and free space while you're deciding on a size for the new device. @@ -92,6 +96,7 @@ NOTEBOOK_LUKS_PAGE = 2 NOTEBOOK_UNEDITABLE_PAGE = 3
new_install_name = N_("New %s %s Installation") % (productName, productVersion) +new_vg_text = N_("Create a new volume group ...") unrecoverable_error_msg = N_("Storage configuration reset due to unrecoverable " "error. Click for details.") device_configuration_error_msg = N_("Device reconfiguration failed. Click for " @@ -340,6 +345,69 @@ class DisksDialog(GUIObject): def run(self): return self.window.run()
+class VolumeGroupDialog(GUIObject):
- builderObjects = ["vg_dialog", "disk_store", "vg_disk_view"]
- mainWidgetName = "vg_dialog"
- uiFile = "spokes/custom.glade"
- def __init__(self, *args, **kwargs):
self._disks = kwargs.pop("disks")
free = kwargs.pop("free")
self.name = kwargs.pop("name") or ""
I believe you want kwargs.pop("name", "") here. Or could kwargs contain "name": None?
It could get name=None.
self.selected = kwargs.pop("selected")[:]
GUIObject.__init__(self, *args, **kwargs)
self.builder.get_object("vg_name_entry").set_text(self.name)
self._store = self.builder.get_object("disk_store")
# populate the store
for disk in self._disks:
self._store.append([disk.description,
str(Size(spec="%dMB" % disk.size)),
str(free[disk.name][0]),
disk.serial,
disk.id])
treeview = self.builder.get_object("vg_disk_view")
model = treeview.get_model()
itr = model.get_iter_first()
selected_ids = [d.id for d in self.selected]
log.debug("selected: %s" % [d.name for d in self.selected])
log.debug("selected: %s" % selected_ids)
selection = treeview.get_selection()
while itr:
disk_id = model.get_value(itr, 4)
log.debug("store: %d" % disk_id)
if disk_id in selected_ids:
selection.select_iter(itr)
itr = model.iter_next(itr)
- def on_cancel_clicked(self, button):
self.window.destroy()
- def _get_disk_by_id(self, disk_id):
for disk in self._disks:
if disk.id == disk_id:
return disk
- def on_save_clicked(self, button):
treeview = self.builder.get_object("vg_disk_view")
model, paths = treeview.get_selection().get_selected_rows()
self.selected = []
for path in paths:
itr = model.get_iter(path)
disk_id = model.get_value(itr, 4)
We use model[itr][4] elsewhere.
I've used model.get_value a fair amount in custom.py -- especially in DisksDialog, which has an unfortunate amount of code in common with VolumeGroupDialog.
self.selected.append(self._get_disk_by_id(disk_id))
self.name = self.builder.get_object("vg_name_entry").get_text()
self.window.destroy()
- def run(self):
return self.window.run()
class CustomPartitioningSpoke(NormalSpoke, StorageChecker): builderObjects = ["customStorageWindow", "sizeAdjustment", "partitionStore", @@ -360,7 +428,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._fs_types = [] # list of supported fstypes self._unused_devices = None # None indicates uninitialized self._free_space = Size(bytes=0)
self._device_disks = []
self._device_container_name = None self._initialized = False
@@ -826,7 +896,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): prev_encrypted = device.encrypted log.debug("old encryption setting: %s" % prev_encrypted) encryption_checkbox = self.builder.get_object("encryptCheckbox")
encrypted = None encrypted = encryption_checkbox.get_active() changed_encryption = (prev_encrypted != encrypted)
@@ -893,6 +962,13 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._populate_right_side(selector) return
# XXX Now we have to do something squirrely with the encryption setting.
# We've done the checks for things that cannot be on an encrypted
# device. From here on out, it means "encrypt new devices we create",
# which we don't want to do in the event that encryption is already
# present below the surface of an existing device stack.
encrypted = encrypted and encryption_checkbox.get_sensitive()
with ui_storage_logger(): # create a new factory using the appropriate size and type factory = self.__storage.getDeviceFactory(device_type, size,
@@ -919,9 +995,22 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): old_disk_set = device.disks if hasattr(device, "req_disks") and not device.exists: old_disk_set = device.req_disks
changed_disk_set = (set(old_disk_set) != set(self._device_disks))
if changed_device_type or changed_raid_level:
changed_container = False
old_container_name = None
container = self.__storage.getContainer(factory)
if not changed_device_type and device_type == DEVICE_TYPE_LVM:
old_container = self.__storage.getContainer(factory,
device=device)
container = self.__storage.getContainer(factory,
container_name=self._device_container_name)
if self._device_container_name != old_container.name:
old_container_name = old_container.name
changed_container = True
if changed_device_type or changed_raid_level or changed_container: if changed_device_type: log.info("changing device type from %s to %s" % (current_device_type, device_type))
@@ -935,6 +1024,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): % ([d.name for d in device.disks], [d.name for d in self._device_disks]))
if changed_container:
log.info("changing container from %s to %s"
% (old_container.name, self._device_container_name))
# remove the current device self.clear_errors() root = self._current_selector._root
@@ -945,7 +1038,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
with ui_storage_logger(): disks = self._device_disks[:]
container = self.__storage.getContainer(factory) if container and changed_device_type: log.debug("overriding disk set with container's") disks = container.disks[:]
@@ -955,6 +1047,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): disks=disks, mountpoint=mountpoint, label=label, raid_level=raid_level, encrypted=encrypted,
container_name=self._device_container_name, selector=selector) except ErrorRecoveryFailure as e: self._error = e
@@ -979,6 +1072,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): label=old_label, raid_level=old_raid_level, encrypted=prev_encrypted,
container_name=old_container_name, selector=selector) except StorageError as e: # failed to recover.
@@ -1043,6 +1137,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): device=device, disks=self._device_disks[:], encrypted=encrypted,
container_name=self._device_container_name, raid_level=raid_level) except ErrorRecoveryFailure as e: self._error = e
@@ -1368,6 +1463,13 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
log.debug("updated device_disks to %s" % [d.name for d in self._device_disks])
if hasattr(use_dev, "vg"):
self._device_container_name = use_dev.vg.name
else:
self._device_container_name = None
log.debug("updated device_vg_name to %s" % self._device_container_name)
selectedDeviceLabel.set_text(selector.props.name) selectedDeviceDescLabel.set_text(self._description(selector.props.name))
@@ -1522,9 +1624,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): typeCombo.set_active(type_index_map[device_type])
# you can't change the type of an existing device
typeCombo.set_sensitive(not device.exists)
typeCombo.set_sensitive(not use_dev.exists) self._populate_raid(raid_level, device.size)
self._populate_lvm(device=use_dev) self.builder.get_object("optionsNotebook").set_sensitive(not device.exists)
###
@@ -1807,6 +1910,94 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
self._device_disks = disks
- def run_vg_editor(self, vg=None, name=None):
if vg:
vg_name = vg.name
elif name:
vg_name = name
dialog = VolumeGroupDialog(self.data,
name=vg_name,
disks=self._clearpartDevices,
free=self._currentFreeInfo,
selected=self._device_disks)
with enlightbox(self.window, dialog.window):
rc = dialog.run()
if rc == 0:
return
disks = dialog.selected
name = dialog.name
log.debug("new disks for %s: %s" % (name, [d.name for d in disks]))
if not disks:
self._error = "No disks selected. Not saving changes."
self.window.set_info(Gtk.MessageType.INFO,
self._error)
self.window.show_all()
return
log.debug("new VG name: %s" % name)
if name != vg_name and name in self.__storage.names:
self._error = _("Volume Group name %s is already in use. Not "
"saving changes.")
self.window.set_info(Gtk.MessageType.INFO,
self._error)
self.window.show_all()
return
self._device_disks = disks
self._device_container_name = name
- def on_modify_vg_clicked(self, button):
vg_name = self.builder.get_object("volumeGroupCombo").get_active_text()
vg = self.__storage.devicetree.getDeviceByName(vg_name)
# pass the name along with any found vg since we could be modifying a
# vg that hasn't been instantiated yet
self.run_vg_editor(vg=vg, name=vg_name)
log.debug("%s -> %s" % (vg_name, self._device_container_name))
if vg_name == self._device_container_name:
return
log.debug("renaming VG %s to %s" % (vg_name, self._device_container_name))
if vg:
self.__storage.devicetree.names.remove(vg.name)
self.__storage.devicetree.names.append(self._device_container_name)
vg.name = self._device_container_name
vg_combo = self.builder.get_object("volumeGroupCombo")
for idx, data in enumerate(vg_combo.get_model()):
# we're looking for the original vg name
if data[0] == vg_name:
vg_combo.remove(idx)
vg_combo.insert_text(idx, self._device_container_name)
vg_combo.set_active(idx)
break
- def on_vg_changed(self, combo):
vg_name = combo.get_active_text()
log.debug("new vg selection: %s" % vg_name)
if vg_name == new_vg_text:
# run the vg editor dialog with a default name and disk set
hostname = self.data.network.hostname
name = self.__storage.suggestContainerName(hostname=hostname)
self.run_vg_editor(name=name)
for idx, data in enumerate(combo.get_model()):
if data[0] == new_vg_text:
combo.insert_text(idx, self._device_container_name)
combo.set_active(idx)
break
else:
self._device_container_name = vg_name
vg = self.__storage.devicetree.getDeviceByName(self._device_container_name)
vg_exists = getattr(vg, "exists", False) # might not be in the tree
self.builder.get_object("modifyVGButton").set_sensitive(not vg_exists)
- def on_selector_clicked(self, selector): if not self._initialized: return
@@ -1971,6 +2162,55 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): hasattr(fmt, "label")) mountPointEntry.set_sensitive(fmt.mountable)
- def _populate_lvm(self, device=None):
#
# set up the volume group widgets for lvm or hide them for other types
#
This could be a docstring.
Good idea. Fixed locally.
device_type = self._get_current_device_type()
if device is None:
if self._current_selector is None:
return
device = self._current_selector._device
vg_combo = self.builder.get_object("volumeGroupCombo")
vg_button = self.builder.get_object("modifyVGButton")
vg_label = self.builder.get_object("volumeGroupLabel")
if device_type == DEVICE_TYPE_LVM:
# set up the vg widgets and then bail out
if self._device_container_name:
default_vg = self._device_container_name
else:
with ui_storage_logger():
factory = self.__storage.getDeviceFactory(DEVICE_TYPE_LVM,
0)
container = self.__storage.getContainer(factory)
default_vg = getattr(container, "name", None)
vg_combo.remove_all()
vgs = self.__storage.vgs
for vg in vgs:
vg_combo.append_text(vg.name)
if default_vg and vg.name == default_vg:
vg_combo.set_active(vgs.index(vg))
vg_combo.append_text(new_vg_text)
if default_vg is None:
vg_combo.set_active(len(vg_combo.get_model()))
#vg_combo.set_active(-1)
This could probably go away.
Also fixed locally.
vg_combo.show()
vg_button.show()
vg_label.show()
# make the combo and button insensitive for existing LVs
can_change_vg = (device is not None and not device.exists)
vg_combo.set_sensitive(can_change_vg)
else:
vg_label.hide()
vg_combo.hide()
vg_button.hide()
- def on_device_type_changed(self, combo): if not self._initialized: return
@@ -2011,8 +2251,12 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): elif new_type == DEVICE_TYPE_MD: raid_level = "raid0"
# lvm uses the RHS to set disk set. no foolish minds here.
self._configButton.set_sensitive(new_type != DEVICE_TYPE_LVM)
size = self.builder.get_object("sizeSpinner").get_value() self._populate_raid(raid_level, size)
self._populate_lvm() # begin btrfs magic fsCombo = self.builder.get_object("fileSystemTypeCombo")
Thanks for the explanations. ACK.
The code to decide when to lock the encryption checkbutton was too simple. It was possible to toggle it for existing LVs that had been marked for reformat previously. It was also possible to toggle it for new LVs in existing VGs whose PVs were encrypted. It should be active and insensitive whenever there is encryption on existing devices anywhere in the stack since we cannot change that. --- pyanaconda/ui/gui/spokes/custom.py | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 21501af..c52789c 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -1512,10 +1512,12 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
encryptCheckbox.set_active(device.encrypted) encryptCheckbox.set_sensitive(reformatCheckbox.get_active()) - if device.encrypted and device.exists: - # For existing devices, the encryption checkbutton should not be - # sensitive if the encryption is below the leaf layer. - encryptCheckbox.set_sensitive(use_dev.format.type == "luks") + ancestors = use_dev.ancestors + ancestors.remove(use_dev) + if any([a.format.type == "luks" and a.format.exists for a in ancestors]): + # The encryption checkbutton should not be sensitive if there is + # existing encryption below the leaf layer. + encryptCheckbox.set_sensitive(False)
## ## Set up the filesystem type combo.
VGs have a freeSpace attribute while btrfs vols do not. Choose the existing container with the most free space or the most total space. --- pyanaconda/storage/__init__.py | 17 +++++++++---- pyanaconda/ui/gui/spokes/custom.py | 47 +++++++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 13 deletions(-)
diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py index 07f2cce..32ca2cb 100644 --- a/pyanaconda/storage/__init__.py +++ b/pyanaconda/storage/__init__.py @@ -1992,15 +1992,15 @@ class Storage(object): self, size, [d.name for d in disks], raid_level)) return factory_class(self, size, disks, raid_level, encrypted)
- def getContainer(self, factory, device=None, container_name=None): + def getContainer(self, factory, device=None, name=None, existing=False): # XXX would it be useful to implement this as a series of fallbacks # instead of mutually exclusive branches? container = None - if container_name: - container = self.devicetree.getDeviceByName(container_name) + if name: + container = self.devicetree.getDeviceByName(name) if container and container not in factory.container_list: log.debug("specified container name %s is wrong type (%s)" - % (container_name, container.type)) + % (name, container.type)) container = None elif device: if hasattr(device, "vg"): @@ -2012,6 +2012,13 @@ class Storage(object): if containers: container = containers[0]
+ if container is None and existing: + containers = [c for c in factory.container_list if c.exists] + if containers: + containers.sort(key=lambda c: getattr(c, "freeSpace", c.size), + reverse=True) + container = containers[0] + return container
def __cleanUpMemberDevices(self, members, container=None): @@ -2108,7 +2115,7 @@ class Storage(object):
container = self.getContainer(factory, device=device, - container_name=container_name) + name=container_name)
# TODO: striping, mirroring, &c # TODO: non-partition members (pv-on-md) diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index c52789c..ee70c14 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -24,7 +24,6 @@ # - Add a way for users to specify the names of md, lv, subvol devices. # - We should either remove boot disk selection from the cart we show from here # or re-partition when it gets changed to make a best-effort at keeping up. -# - Add new LVs to existing VG if creating a new VG fails outright. # - Deleting an LV is not reflected in available space in the bottom left. # - this is only true for preexisting LVs # - Device descriptions, suggested sizes, etc. should be moved out into a support file. @@ -35,6 +34,7 @@ # - If you click to add a mountpoint while editing a device the lightbox # screenshot is taken prior to the ui update so the background shows the old # size and free space while you're deciding on a size for the new device. +# - DeviceTree.populate brings back deleted devices when unlocking LUKS.
import gettext _ = lambda x: gettext.ldgettext("anaconda", x) @@ -122,6 +122,11 @@ DEVICE_TEXT_PARTITION = N_("Standard Partition") DEVICE_TEXT_BTRFS = N_("BTRFS") DEVICE_TEXT_DISK = N_("Disk")
+device_text_map = {DEVICE_TYPE_LVM: DEVICE_TEXT_LVM, + DEVICE_TYPE_MD: DEVICE_TEXT_MD, + DEVICE_TYPE_PARTITION: DEVICE_TEXT_PARTITION, + DEVICE_TYPE_BTRFS: DEVICE_TEXT_BTRFS} + options_page_dict = {DEVICE_TYPE_LVM: 0, DEVICE_TYPE_MD: 1, DEVICE_TYPE_BTRFS: 2} @@ -1005,7 +1010,7 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): old_container = self.__storage.getContainer(factory, device=device) container = self.__storage.getContainer(factory, - container_name=self._device_container_name) + name=self._device_container_name) if self._device_container_name != old_container.name: old_container_name = old_container.name changed_container = True @@ -1713,12 +1718,13 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): encrypted = False
disks = self._clearpartDevices + size = float(size.convertTo(spec="mb")) self.clear_errors()
with ui_storage_logger(): try: self.__storage.newDevice(device_type, - size=float(size.convertTo(spec="mb")), + size=size, fstype=fstype, mountpoint=mountpoint, encrypted=encrypted, @@ -1732,11 +1738,36 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._reset_storage() except StorageError as e: log.error("newDevice failed: %s" % e) - self._error = e - self.window.set_info(Gtk.MessageType.ERROR, - _("Failed to add new device. Click for " - "details.")) - self.window.show_all() + log.debug("trying to find an existing container to use") + factory = self.__storage.getDeviceFactory(device_type, size) + container = self.__storage.getContainer(factory, existing=True) + log.debug("found container %s" % container) + if container: + try: + self.__storage.newDevice(device_type, + size=size, + fstype=fstype, + mountpoint=mountpoint, + encrypted=encrypted, + disks=disks, + container_name=container.name) + except StorageError as e2: + log.error("newDevice failed w/ old container: %s" % e2) + else: + type_str = device_text_map[device_type] + self.window.set_info(Gtk.MessageType.INFO, + _("Added new %s to existing " + "container %s.") + % (type_str, container.name)) + self.window.show_all() + e = None + + if e: + self._error = e + self.window.set_info(Gtk.MessageType.ERROR, + _("Failed to add new device. Click for " + "details.")) + self.window.show_all()
self._devices = self.__storage.devices self._do_refresh()
On Wed, 2012-11-21 at 13:44 -0600, David Lehman wrote:
Patches 2 and 4 add support for changing an LV's VG and for creating new LVs when the only free space is in a preexisting VG, respectively.
The other two are trivial.
Apart from comments in reply to PATCH 2/4 this looks good to me.
anaconda-patches@lists.fedorahosted.org