[PATCH 2/4] Add support for changing a new LV's VG.
Vratislav Podzimek
vpodzime at redhat.com
Tue Nov 27 12:30:18 UTC 2012
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")
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list