Patch 1 is in the spirit of quietly building a sane device tree instead of asking about everything that looks broken or out of place.
Patch 2 reworks partition clearing and disk (re)initialization to follow a more consistent/clear/coherent pattern. See the commit message for details and note how simple Storage.clearPartitions becomes as a result.
Patch 3 changes the disk selection from the equivalent of choosing clearpart disks to actually defining the set of disks anaconda sees. De-selecting a disk means it is not available in custom partitioning. Selecting a disk means it is fair game for clearpart/autopart.
Patch 4 catches the storage spoke up with the software spoke in terms of sanity checking. Both spokes can run a sanity check and notify the user if a failure occurs, but neither has a way to let the user see the actual set of problems. Baby steps.
--- pyanaconda/storage/devicetree.py | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py index 5d50986..907c315 100644 --- a/pyanaconda/storage/devicetree.py +++ b/pyanaconda/storage/devicetree.py @@ -1678,6 +1678,15 @@ class DeviceTree(object): for leaf in filter(lambda leaf: leaf.type == "lvmvg" and not leaf.complete, self.leaves): leafInconsistencies(leaf)
+ for md in [d for d in self.leaves if d.type == "mdarray" and len(d.parents) < d.memberDevices]: + log.debug("removing incomplete/degraded md array %s" % md.name) + try: + md.teardown() + except StorageError as e: + log.error("failed to deactivate %s: %s" % (md.name, e)) + + self._removeDevice(md) + def _recursiveRemove(self, device): for d in self.getChildren(device): self._recursiveRemove(d)
CLEARPART_TYPE_ALL means clear existing partitions and create a new disklabel.
CLEARPART_TYPE_LINUX means clear linux-native partitions (or linux- native whole-disk formatting) and create a new disklabel if the removals result in an empty disk.
CLEARPART_TYPE_LIST means clear the specified devices (and devices built on top of them) and create a new disklabel if the removals result in an empty disk.
Zerombr becomes an alias for 'clearpart --initlabel'.
To initialize all disks:
clearpart --all
To initialize uninitalized disks but otherwise clear no partitions:
clearpart --none --initlabel
This adds a flag to the storage discovery config class that determines whether or not clearPartitions will remove not-yet-created devices and disklabels. The default is to not clear non-existent devices/formats.
After the first trip to the storage spoke, we should not remove any layout the user set up in previous visits unless they choose autopart again, in which case we start from scratch each time, or de-select disks. Since the disk set can change each time through we have to be able to initialize uninitialized disks even on the way to the custom ui. --- pyanaconda/iw/filter_gui.py | 2 +- pyanaconda/kickstart.py | 2 +- pyanaconda/storage/__init__.py | 140 +++++++++++++++---------------- pyanaconda/storage/devices.py | 9 ++ pyanaconda/storage/devicetree.py | 1 - pyanaconda/storage/formats/disklabel.py | 10 ++ pyanaconda/storage/partitioning.py | 2 +- pyanaconda/ui/gui/spokes/storage.py | 12 ++- 8 files changed, 100 insertions(+), 78 deletions(-)
diff --git a/pyanaconda/iw/filter_gui.py b/pyanaconda/iw/filter_gui.py index 40b9563..59da8fd 100644 --- a/pyanaconda/iw/filter_gui.py +++ b/pyanaconda/iw/filter_gui.py @@ -621,7 +621,7 @@ class FilterWindow(InstallWindow): zfcp.ZFCP().startup(anaconda.intf) dasd.DASD().startup(anaconda.intf, anaconda.storage.config.exclusiveDisks, - anaconda.storage.config.zeroMbr) + anaconda.storage.config.initializeDisks) disks = self._getFilterDisks()
mcw = MultipathConfigWriter() diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index a3a6c5d..42723e0 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -412,7 +412,7 @@ class ClearPart(commands.clearpart.F17_ClearPart): storage.config.clearPartDevices = self.devices
if self.initAll: - storage.config.reinitializeDisks = self.initAll + storage.config.initializeDisks = self.initAll
storage.clearPartitions()
diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py index db566b4..5f55067 100644 --- a/pyanaconda/storage/__init__.py +++ b/pyanaconda/storage/__init__.py @@ -77,16 +77,7 @@ def storageInitialize(storage, ksdata, protected):
# Before we set up the storage system, we need to know which disks to # ignore, etc. Luckily that's all in the kickstart data. - storage.config.zeroMbr = ksdata.zerombr.zerombr - storage.config.ignoreDiskInteractive = ksdata.ignoredisk.interactive - storage.config.ignoredDisks = ksdata.ignoredisk.ignoredisk - storage.config.exclusiveDisks = ksdata.ignoredisk.onlyuse - - if ksdata.clearpart.type is not None: - storage.config.clearPartType = ksdata.clearpart.type - storage.config.clearPartDisks = ksdata.clearpart.drives - if ksdata.clearpart.initAll: - storage.config.reinitializeDisks = ksdata.clearpart.initAll + storage.config.update(ksdata)
lvm.lvm_vg_blacklist = []
@@ -317,12 +308,24 @@ class StorageDiscoveryConfig(object): self.clearPartType = None self.clearPartDisks = [] self.clearPartDevices = [] - self.reinitializeDisks = False - self.zeroMbr = None + self.initializeDisks = False self.protectedDevSpecs = [] self.diskImages = {} self.mpathFriendlyNames = True
+ # Whether clearPartitions removes scheduled/non-existent devices and + # disklabels depends on this flag. + self.clearNonExistent = False + + def update(self, ksdata): + self.ignoredDisks = ksdata.ignoredisk.ignoredisk[:] + self.exclusiveDisks = ksdata.ignoredisk.onlyuse[:] + self.clearPartType = ksdata.clearpart.type + self.clearPartDisks = ksdata.clearpart.drives[:] + self.clearPartDevices = ksdata.clearpart.devices[:] + self.initializeDisks = ksdata.clearpart.initAll + self.zeroMbr = ksdata.zerombr.zerombr + def writeKS(self, f): # clearpart if self.clearPartType is None or self.clearPartType == CLEARPART_TYPE_NONE: @@ -334,7 +337,7 @@ class StorageDiscoveryConfig(object):
if self.clearPartDisks: args += ["--drives=%s" % ",".join(self.clearPartDisks)] - if self.reinitializeDisks: + if self.initializeDisks: args += ["--initlabel"]
f.write("#clearpart %s\n" % " ".join(args)) @@ -483,13 +486,16 @@ class Storage(object): if device.format.type == "luks" and device.format.exists: self.__luksDevs[device.format.uuid] = device.format._LUKS__passphrase
+ if self.data: + self.config.update(self.data) + if not flags.imageInstall: self.iscsi.startup() self.fcoe.startup() self.zfcp.startup() self.dasd.startup(None, self.config.exclusiveDisks, - self.config.zeroMbr) + self.config.initializeDisks) clearPartType = self.config.clearPartType # save this before overriding it if self.data and self.data.upgrade.upgrade: self.config.clearPartType = CLEARPART_TYPE_NONE @@ -519,15 +525,6 @@ class Storage(object):
self.dumpState("initial")
- # if zerombr is set, go ahead and slap a disklabel on any disk that - # doesn't have (recognizeable) formatting - if self.config.zeroMbr: - for disk in self.disks: - if disk.format.type is None: - log.info("zerombr: initializing %s" % disk.name) - self.reinitializeDisk(disk) - - # we may have added candidate boot disks just above by initializing self.updateBootLoaderDiskList()
@property @@ -754,22 +751,37 @@ class Storage(object): clearPartDevices = kwargs.get("clearPartDevices", self.config.clearPartDevices)
- if clearPartType in [CLEARPART_TYPE_NONE, None]: + for disk in device.disks: + # this will not include disks with hidden formats like multipath + # and firmware raid member disks + if clearPartDisks and disk.name not in clearPartDisks: + return False + + if not self.config.clearNonExistent: + if not device.exists: + return False + + if device.isDisk: + if not device.format.exists: + return False + + for partition in self.devicetree.getChildren(device): + if not (partition.isMagic or self.shouldClear(partition)): + return False + + # the only devices we want to clear when clearPartType is + # CLEARPART_TYPE_NONE are uninitialized disks in clearPartDisks, and + # then only when we have been asked to initialize disks as needed + if clearPartType in [CLEARPART_TYPE_NONE, None] and \ + not (device.isDisk and device.format.type is None and + self.config.initializeDisks): return False
if isinstance(device, PartitionDevice): # Never clear the special first partition on a Mac disk label, as # that holds the partition table itself. # Something similar for the third partition on a Sun disklabel. - if device.disk.format.labelType == "mac" and \ - device.partedPartition.number == 1: - return False - elif device.disk.format.labelType == "sun" and \ - device.partedPartition.number == 3: - return False - - # If we got a list of disks to clear, make sure this one's on it - if clearPartDisks and device.disk.name not in clearPartDisks: + if device.isMagic: return False
# We don't want to fool with extended partitions, freespace, &c @@ -782,17 +794,27 @@ class Storage(object): not device.getFlag(parted.PARTITION_RAID) and \ not device.getFlag(parted.PARTITION_SWAP): return False - elif device.isDisk and not device.partitioned: - # If we got a list of disks to clear, make sure this one's on it - if clearPartDisks and device.name not in clearPartDisks: - return False + elif device.isDisk: + if device.partitioned and clearPartType != CLEARPART_TYPE_ALL: + # if clearPartType is not CLEARPART_TYPE_ALL but we'll still be + # removing every partition from the disk, return True since we + # will want to be able to create a new disklabel on this disk + for partition in self.devicetree.getChildren(device): + if not (partition.isMagic or self.shouldClear(partition)): + return False
# Never clear disks with hidden formats if device.format.hidden: return False
+ # When clearPartType is CLEARPART_TYPE_LINUX and a disk has non- + # linux whole-disk formatting, do not clear it. The exception is + # the case of an uninitialized disk when we've been asked to + # initialize disks as needed if clearPartType == CLEARPART_TYPE_LINUX and \ - not device.format.linuxNative: + not (self.config.initializeDisks and + device.format.type is None) and \ + not device.partitioned and not device.format.linuxNative: return False
# Don't clear devices holding install media. @@ -836,22 +858,16 @@ class Storage(object): - Needs some error handling
""" - if self.config.clearPartType in (None, CLEARPART_TYPE_NONE): - # not much to do - return - - # XXX FIXME: We can still clear partitions. What we can't do is create - # appropriately-typed disklabels. if not hasattr(self.platform, "diskLabelTypes"): raise StorageError("can't clear partitions without platform data")
- # we are only interested in partitions that physically exist - partitions = [p for p in self.partitions if p.exists] # Sort partitions by descending partition number to minimize confusing # things like multiple "destroy sda5" actions due to parted renumbering # partitions. This can still happen through the UI but it makes sense to # avoid it where possible. - partitions.sort(key=lambda p: p.partedPartition.number, reverse=True) + partitions = sorted(self.partitions, + key=lambda p: p.partedPartition.number, + reverse=True) for part in partitions: log.debug("clearpart: looking at %s" % part.name) if not self.shouldClear(part): @@ -863,35 +879,17 @@ class Storage(object): # now remove any empty extended partitions self.removeEmptyExtendedPartitions()
- # make sure that the the boot device has the correct disklabel type if - # we're going to completely clear it. - # also handles clearpart --initlabel - boot_disk = self.bootDisk - for disk in self.partitioned: - if not boot_disk and not self.config.reinitializeDisks: - break - - if not self.config.reinitializeDisks and \ - (boot_disk is not None and disk != boot_disk): - continue - + # ensure all disks have appropriate disklabels + for disk in self.disks: if not self.shouldClear(disk): continue
- # don't reinitialize the disklabel if the disk contains install media - # or pvs from inconsistent vgs - if filter(lambda p: p.dependsOn(disk), self.protectedDevices): - continue - - if not self.config.reinitializeDisks and \ - disk.format.labelType == self.platform.bestDiskLabelType(disk): - continue - - self.reinitializeDisk(disk) + log.debug("clearpart: initializing %s" % disk.name) + self.initializeDisk(disk)
self.updateBootLoaderDiskList()
- def reinitializeDisk(self, disk): + def initializeDisk(self, disk): """ (Re)initialize a disk by creating a disklabel on it.
The disk should not contain any partitions except perhaps for a @@ -912,7 +910,7 @@ class Storage(object): self.devicetree._removeDevice(part, moddisk=False)
if disk.partitioned and disk.format.partitions: - raise ValueError("cannot reinitialize a disk that has partitions") + raise ValueError("cannot initialize a disk that has partitions")
# remove existing formatting from the disk destroy_action = ActionDestroyFormat(disk) diff --git a/pyanaconda/storage/devices.py b/pyanaconda/storage/devices.py index 1e22138..6b979e8 100644 --- a/pyanaconda/storage/devices.py +++ b/pyanaconda/storage/devices.py @@ -1446,6 +1446,15 @@ class PartitionDevice(StorageDevice):
self.partedPartition.unsetFlag(flag)
+ @property + def isMagic(self): + if not self.disk: + return False + + number = getattr(self.partedPartition, "number", -1) + magic = self.disk.format.magicPartitionNumber + return (number == magic) + def probe(self): """ Probe for any missing information about this device.
diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py index 907c315..2c36f83 100644 --- a/pyanaconda/storage/devicetree.py +++ b/pyanaconda/storage/devicetree.py @@ -170,7 +170,6 @@ class DeviceTree(object): self.populated = False
self.exclusiveDisks = getattr(conf, "exclusiveDisks", []) - self.zeroMbr = getattr(conf, "zeroMbr", False) self.shouldClear = shouldClear or (lambda d: False) self.iscsi = iscsi self.dasd = dasd diff --git a/pyanaconda/storage/formats/disklabel.py b/pyanaconda/storage/formats/disklabel.py index 16def28..338a880 100644 --- a/pyanaconda/storage/formats/disklabel.py +++ b/pyanaconda/storage/formats/disklabel.py @@ -426,5 +426,15 @@ class DiskLabel(DeviceFormat):
return free
+ @property + def magicPartitionNumber(self): + """ Number of disklabel-type-specific special partition. """ + if self.labelType == "mac": + return 1 + elif self.labelType == "sun": + return 3 + else: + return 0 + register_device_format(DiskLabel)
diff --git a/pyanaconda/storage/partitioning.py b/pyanaconda/storage/partitioning.py index b77f499..74eb685 100644 --- a/pyanaconda/storage/partitioning.py +++ b/pyanaconda/storage/partitioning.py @@ -295,8 +295,8 @@ def doAutoPartition(storage, data): devs = []
if storage.doAutoPart: + # XXX this doesn't belong on newui scheduleShrinkActions(storage) - storage.clearPartitions()
disks = _getCandidateDisks(storage) devs = _scheduleImplicitPartitions(storage, disks) diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py index 8771132..4cf3f5e 100644 --- a/pyanaconda/ui/gui/spokes/storage.py +++ b/pyanaconda/ui/gui/spokes/storage.py @@ -265,10 +265,16 @@ class StorageSpoke(NormalSpoke):
self.data.bootloader.location = "mbr"
+ self.data.clearpart.initAll = True self.data.clearpart.type = self.clearPartType - - if self.autopart: - self.data.clearpart.execute(self.storage, self.data, self.instclass) + self.storage.config.update(self.data) + + # If autopart is selected we want to remove whatever has been + # created/scheduled to make room for autopart. + # If custom is selected, we want to leave alone any storage layout the + # user may have set up before now. + self.storage.config.clearNonExistent = self.data.autopart.autopart + self.data.clearpart.execute(self.storage, self.data, self.instclass)
# Pick the first disk to be the destination device for the bootloader. # This appears to be the minimum amount of configuration required to
This adds a way to hide devices from the devicetree without removing them. The devicetree is initially populated without filtering because we need to collect information about disks in the system as well as free space in disks and filesystems. Disk selection in the storage spoke establishes the set of disks to be used during install. We don't want to re-populate the tree every time a disk is selected or deselected. Instead, we just hide/unhide the disk and all devices it contains. --- pyanaconda/storage/devicelibs/lvm.py | 15 +++++++++++- pyanaconda/storage/devicetree.py | 43 ++++++++++++++++++++++++++++++--- pyanaconda/ui/gui/spokes/storage.py | 31 +++++++++++++++++++----- 3 files changed, 77 insertions(+), 12 deletions(-)
diff --git a/pyanaconda/storage/devicelibs/lvm.py b/pyanaconda/storage/devicelibs/lvm.py index 36fd980..136168b 100644 --- a/pyanaconda/storage/devicelibs/lvm.py +++ b/pyanaconda/storage/devicelibs/lvm.py @@ -83,7 +83,20 @@ def lvm_cc_addFilterRejectRegexp(regexp): log.debug("lvm filter: adding %s to the reject list" % regexp) config_args_data["filterRejects"].append(regexp)
- # compoes config once more. + # compose config once more. + _composeConfig() + +def lvm_cc_removeFilterRejectRegexp(regexp): + """ Remove a regular expression from the --config string.""" + global config_args_data + log.debug("lvm filter: removing %s from the reject list" % regexp) + try: + config_args_data["filterRejects"].remove(regexp) + except ValueError: + log.debug("%s wasn't in the reject list" % regexp) + return + + # compose config once more. _composeConfig()
def lvm_cc_resetFilter(): diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py index 2c36f83..ae69be2 100644 --- a/pyanaconda/storage/devicetree.py +++ b/pyanaconda/storage/devicetree.py @@ -166,6 +166,8 @@ class DeviceTree(object): # a list of all device names we encounter self.names = []
+ self._hidden = [] + # indicates whether or not the tree has been fully populated self.populated = False
@@ -1686,12 +1688,45 @@ class DeviceTree(object):
self._removeDevice(md)
- def _recursiveRemove(self, device): + def hide(self, device): for d in self.getChildren(device): - self._recursiveRemove(d) + self.hide(d) + + log.info("hiding device %s %s (id %d)" % (device.type, + device.name, + device.id))
- device.teardown() - self._removeDevice(device) + for action in reversed(self._actions): + if not action.device.dependsOn(device) and action.device != device: + continue + + log.debug("cancelling action: %s" % action) + try: + action.cancel() + except Exception: + log.warning("failed to cancel action while hiding %s: %s" + % (device.name, action)) + finally: + self._actions.remove(action) + + # XXX modifications that do not require actions, like setting a + # mountpoint, will not be reversed here + + # we're intentionally not modifying self.names here + self._devices.remove(device) + self._hidden.append(device) + lvm.lvm_cc_addFilterRejectRegexp(device.name) + + def unhide(self, device): + # the hidden list should be in leaves-first order + for hidden in reversed(self._hidden): + if hidden == device or hidden.dependsOn(device): + log.info("unhiding device %s %s (id %d)" % (hidden.type, + hidden.name, + hidden.id)) + self._hidden.remove(hidden) + self._devices.append(hidden) + lvm.lvm_cc_removeFilterRejectRegexp(device.name)
def _setupLvs(self): ret = False diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py index 4cf3f5e..0e3ecec 100644 --- a/pyanaconda/ui/gui/spokes/storage.py +++ b/pyanaconda/ui/gui/spokes/storage.py @@ -240,7 +240,12 @@ class StorageSpoke(NormalSpoke): def __init__(self, *args, **kwargs): NormalSpoke.__init__(self, *args, **kwargs) self._ready = False - self.selected_disks = self.data.clearpart.drives[:] + self.selected_disks = self.data.ignoredisk.onlyuse[:] + + # This list gets set up once in initialize and should not be modified + # except perhaps to add advanced devices. It will remain the full list + # of disks that can be included in the install. + self.disks = []
if not flags.automatedInstall: # default to using autopart for interactive installs @@ -252,6 +257,7 @@ class StorageSpoke(NormalSpoke): self.clearPartType = CLEARPART_TYPE_LINUX
def apply(self): + self.data.ignoredisk.onlyuse = self.selected_disks[:] self.data.clearpart.drives = self.selected_disks[:] self.data.autopart.autopart = self.autopart
@@ -263,6 +269,14 @@ class StorageSpoke(NormalSpoke): else: self.clearPartType = CLEARPART_TYPE_NONE
+ for disk in self.disks: + if disk.name not in self.selected_disks and \ + disk in self.storage.devices: + self.storage.devicetree.hide(disk) + elif disk.name in self.selected_disks and \ + disk not in self.storage.devices: + self.storage.devicetree.unhide(disk) + self.data.bootloader.location = "mbr"
self.data.clearpart.initAll = True @@ -301,10 +315,10 @@ class StorageSpoke(NormalSpoke): def status(self): """ A short string describing the current status of storage setup. """ msg = _("No disks selected") - if self.data.clearpart.drives: + if self.data.ignoredisk.onlyuse: msg = P_(("%d disk selected"), ("%d disks selected"), - len(self.data.clearpart.drives)) % len(self.data.clearpart.drives) + len(self.data.ignoredisk.onlyuse)) % len(self.data.ignoredisk.onlyuse)
if self.data.autopart.autopart: msg = _("Automatic partitioning selected") @@ -330,7 +344,7 @@ class StorageSpoke(NormalSpoke):
def refresh(self): # synchronize our local data store with the global ksdata - self.selected_disks = self.data.clearpart.drives[:] + self.selected_disks = self.data.ignoredisk.onlyuse[:] self.autopart = self.data.autopart.autopart
# update the selections in the ui @@ -367,7 +381,7 @@ class StorageSpoke(NormalSpoke): if storageThread: storageThread.join()
- print self.data.clearpart.drives + print self.data.ignoredisk.onlyuse
self.disks = getDisks(self.storage.devicetree)
@@ -408,7 +422,9 @@ class StorageSpoke(NormalSpoke): capacity = 0 free = Size(bytes=0)
- free_space = self.storage.getFreeSpace(clearPartType=self.clearPartType) + # pass in our disk list so hidden disks' free space is available + free_space = self.storage.getFreeSpace(disks=self.disks, + clearPartType=self.clearPartType) selected = [d for d in self.disks if d.name in self.selected_disks]
for disk in selected: @@ -448,7 +464,8 @@ class StorageSpoke(NormalSpoke): # signal handlers def on_summary_clicked(self, button): # show the selected disks dialog - free_space = self.storage.getFreeSpace() + # pass in our disk list so hidden disks' free space is available + free_space = self.storage.getFreeSpace(disks=self.disks) dialog = SelectedDisksDialog(self.data,) dialog.refresh([d for d in self.disks if d.name in self.selected_disks], free_space)
--- pyanaconda/ui/gui/spokes/custom.py | 5 ++- pyanaconda/ui/gui/spokes/storage.py | 39 +++++++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 4214952..59620d5 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -39,6 +39,7 @@ from pyanaconda.storage import Root
from pyanaconda.ui.gui import UIObject from pyanaconda.ui.gui.spokes import NormalSpoke +from pyanaconda.ui.gui.spokes.storage import StorageChecker from pyanaconda.ui.gui.spokes.lib.cart import SelectedDisksDialog from pyanaconda.ui.gui.spokes.lib.accordion import * from pyanaconda.ui.gui.utils import enlightbox, setViewportBackground @@ -92,7 +93,7 @@ class ConfirmDeleteDialog(UIObject): def run(self): return self.window.run()
-class CustomPartitioningSpoke(NormalSpoke): +class CustomPartitioningSpoke(NormalSpoke, StorageChecker): builderObjects = ["customStorageWindow", "sizeAdjustment", "partitionStore", "addImage", "removeImage", "settingsImage"] @@ -110,7 +111,7 @@ class CustomPartitioningSpoke(NormalSpoke): self._when_create_text = ""
def apply(self): - pass + StorageChecker.run(self)
@property def indirect(self): diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py index 0e3ecec..cdafae1 100644 --- a/pyanaconda/ui/gui/spokes/storage.py +++ b/pyanaconda/ui/gui/spokes/storage.py @@ -226,7 +226,26 @@ class InstallOptions3Dialog(InstallOptions1Dialog): "version of <b>%s</b>, or quit the installer.") % (productName, productName) self.builder.get_object("options3_label2").set_markup(label_text)
-class StorageSpoke(NormalSpoke): +class StorageChecker(object): + errors = [] + warnings = [] + _mainSpokeClass = "StorageSpoke" + + def run(self): + from pyanaconda.threads import threadMgr + from pyanaconda.threads import AnacondaThread + + communication.send_not_ready(self._mainSpokeClass) + threadMgr.add(AnacondaThread(name="AnaCheckStorageThread", + target=self.checkStorage)) + + def checkStorage(self): + communication.send_message(self._mainSpokeClass, + _("Checking storage configuration...")) + (self.errors, self.warnings) = self.storage.sanityCheck() + communication.send_ready(self._mainSpokeClass) + +class StorageSpoke(NormalSpoke, StorageChecker): builderObjects = ["storageWindow"] mainWidgetName = "storageWindow" uiFile = "spokes/storage.ui" @@ -301,15 +320,21 @@ class StorageSpoke(NormalSpoke): # this won't do anything if autopart is not selected self.data.autopart.execute(self.storage, self.data, self.instclass)
+ StorageChecker.run(self) + @property def completed(self): - return self.status != _("No disks selected") + from pyanaconda.threads import threadMgr + return (threadMgr.get("AnaCheckStorageThread") is None and + self.storage.rootDevice is not None and + not self.errors)
@property def ready(self): # By default, the storage spoke is not ready. We have to wait until # storageInitialize is done. - return self._ready + from pyanaconda.threads import threadMgr + return self._ready and not threadMgr.get("AnaCheckStorageThread")
@property def status(self): @@ -320,10 +345,12 @@ class StorageSpoke(NormalSpoke): ("%d disks selected"), len(self.data.ignoredisk.onlyuse)) % len(self.data.ignoredisk.onlyuse)
- if self.data.autopart.autopart: + if self.errors: + msg = _("Error checking storage configuration") + elif self.data.autopart.autopart: msg = _("Automatic partitioning selected") - - # if we had a storage instance we could check for a defined root + else: + msg = _("Custom partitioning selected")
return msg
@@ -320,10 +345,12 @@ class StorageSpoke(NormalSpoke): ("%d disks selected"), len(self.data.ignoredisk.onlyuse)) % len(self.data.ignoredisk.onlyuse)
if self.data.autopart.autopart:
if self.errors:msg = _("Error checking storage configuration")elif self.data.autopart.autopart: msg = _("Automatic partitioning selected")
# if we had a storage instance we could check for a defined root
else:msg = _("Custom partitioning selected") return msg
I notice you don't do anything with self.warnings in this chunk, which seems like a reasonable place to do something with it.
- Chris
On Tue, 2012-07-10 at 12:52 -0400, Chris Lumens wrote:
@@ -320,10 +345,12 @@ class StorageSpoke(NormalSpoke): ("%d disks selected"), len(self.data.ignoredisk.onlyuse)) % len(self.data.ignoredisk.onlyuse)
if self.data.autopart.autopart:
if self.errors:msg = _("Error checking storage configuration")elif self.data.autopart.autopart: msg = _("Automatic partitioning selected")
# if we had a storage instance we could check for a defined root
else:msg = _("Custom partitioning selected") return msgI notice you don't do anything with self.warnings in this chunk, which seems like a reasonable place to do something with it.
Yeah, for two reasons (neither very good). First, I had been throwing out the warnings, but decided at the last minute to start saving them for whenever we have a way to show them to the user. Second, I am struggling to find good phrasing for the corresponding status message. "Error checking storage configuration" seems perfectly clear to me, yet "Warning checking storage configuration" does not. Ideas?
Dave
- Chris
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/anaconda-patches
Yeah, for two reasons (neither very good). First, I had been throwing out the warnings, but decided at the last minute to start saving them for whenever we have a way to show them to the user. Second, I am struggling to find good phrasing for the corresponding status message. "Error checking storage configuration" seems perfectly clear to me, yet "Warning checking storage configuration" does not. Ideas?
What if both warnings and errors generate the same message - something along the lines of "Storage needs investigation". Or perhaps two different but similar messages - "Storage requires modification", "Storage may require modification". I'm not very good at the wording here. For errors, we make the spoke not ready, while warnings we leave that alone.
When you enter the storage spoke again, we display a status bar at the bottom of the screen and if you click it, you get a dialog with the various errors and warnings?
It might also be more user-friendly to flag individual filesystems so you get the error/warning message right where you can do something about it.
- Chris
pyanaconda/ui/gui/spokes/custom.py | 5 ++- pyanaconda/ui/gui/spokes/storage.py | 39 +++++++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 8 deletions(-)
I wonder if the existing check method stuff (see 561185b0a8c2e7aaf4a9a8b551bab394116430a8) could be useful for this. I added it a very long time ago before I really knew how a lot of the thread stuff was going to work, so it's entirely possible it's not useful in its existing format.
- Chris
On Tue, Jul 10, 2012 at 11:38:36AM -0500, David Lehman wrote:
pyanaconda/ui/gui/spokes/custom.py | 5 ++- pyanaconda/ui/gui/spokes/storage.py | 39 +++++++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 8 deletions(-)
The imports inside the methods bug me.
Other than that these all look good to me.
anaconda-patches@lists.fedorahosted.org
-
Brian C. Lane -
Chris Lumens -
David Lehman