Patches 1 and 3 fix tracebacks that are pretty easy to hit.
Patch 2 adds handling for systems with incomplete lvm/md devices.
--- pyanaconda/storage/devicetree.py | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py index a082171..3743fd0 100644 --- a/pyanaconda/storage/devicetree.py +++ b/pyanaconda/storage/devicetree.py @@ -777,6 +777,9 @@ class DeviceTree(object): # something must be wrong -- if all of the slaves we in # the tree, this device should be as well if device is None: + if name is None: + name = udev_device_get_name(info) + log.error("failed to scan md array %s" % name) try: devicelibs.mdraid.mddeactivate("/dev/" + name)
TODO: Test handling of a complete and an incomplete array with the same name.
The strategy is to keep the vg/md in the devicetree, but to not include them in devicetree.devices or the results of getDeviceBy{Name,Path}. In the custom spoke we display a special options page for them with a brief explanation and the option to remove the device or leave it be.
When removing an incomplete vg we don't actually do anything to remove the vg since lvm gives us no way to specify one of several vgs with the same name. All we do is wipe the pvs that we have access to. --- pyanaconda/storage/devices.py | 13 +++++ pyanaconda/storage/devicetree.py | 44 ++++++++-------- pyanaconda/ui/gui/spokes/custom.glade | 88 +++++++++++++++++++++++++++++++++ pyanaconda/ui/gui/spokes/custom.py | 37 +++++++++++--- 4 files changed, 153 insertions(+), 29 deletions(-)
diff --git a/pyanaconda/storage/devices.py b/pyanaconda/storage/devices.py index 07387c4..127e688 100644 --- a/pyanaconda/storage/devices.py +++ b/pyanaconda/storage/devices.py @@ -2169,6 +2169,15 @@ class LVMVolumeGroupDevice(DMDevice): def _destroy(self): """ Destroy the device. """ log_method_call(self, self.name, status=self.status) + if not self.complete: + for pv in self.pvs: + # Remove the PVs from the ignore filter so we can wipe them. + lvm.lvm_cc_removeFilterRejectRegexp(pv.name) + + # Don't run vgremove or vgreduce since there may be another VG with + # the same name that we want to keep/use. + return + lvm.vgreduce(self.name, [], rm=True) lvm.vgdeactivate(self.name) lvm.vgremove(self.name) @@ -3002,6 +3011,10 @@ class MDRaidArrayDevice(StorageDevice): return rc
@property + def complete(self): + return (self.memberDevices <= len(self.parents)) or not self.exists + + @property def devices(self): """ Return a list of this array's member device instances. """ return self.parents diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py index 3743fd0..52bcacb 100644 --- a/pyanaconda/storage/devicetree.py +++ b/pyanaconda/storage/devicetree.py @@ -398,7 +398,7 @@ class DeviceTree(object): dev.volume._removeSubVolume(dev.name)
self._devices.remove(dev) - if dev.name in self.names: + if dev.name in self.names and getattr(dev, "complete", True): self.names.remove(dev.name) log.info("removed %s %s (id %d) from device tree" % (dev.type, dev.name, @@ -1318,16 +1318,16 @@ class DeviceTree(object): # lookup/create the VG and LVs try: vg_name = udev_device_get_vg_name(info) + vg_uuid = udev_device_get_vg_uuid(info) except KeyError: # no vg name means no vg -- we're done with this pv return
- vg_device = self.getDeviceByName(vg_name) + vg_device = self.getDeviceByUuid(vg_uuid) if vg_device: vg_device._addDevice(device) else: try: - vg_uuid = udev_device_get_vg_uuid(info) vg_size = udev_device_get_vg_size(info) vg_free = udev_device_get_vg_free(info) pe_size = udev_device_get_vg_extent_size(info) @@ -1434,7 +1434,8 @@ class DeviceTree(object): if md_name: array = self.getDeviceByName(md_name) if array and array.uuid != md_uuid: - md_name = None + log.error("found multiple devices with the name %s" + % md_name)
log.info("using name %s for md array containing member %s" % (md_name, device.name)) @@ -1687,25 +1688,15 @@ class DeviceTree(object): log.info("got format: %s" % device.format)
def _handleInconsistencies(self): - def leafInconsistencies(device): - devicelibs.lvm.lvm_cc_addFilterRejectRegexp(device.name) - devicelibs.lvm.blacklistVG(device.name) - for parent in device.parents: - if parent.type == "partition": - parent.format.inconsistentVG = True - parent.protected = True - else: - self.addIgnoredDisk(parent.name) - devicelibs.lvm.lvm_cc_addFilterRejectRegexp(parent.name) - - 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)) + for vg in [d for d in self.devices if d.type == "lvmvg"]: + if vg.complete: + continue
- self._removeDevice(md) + # Make sure lvm doesn't get confused by PVs that belong to + # incomplete VGs. We will remove the PVs from the blacklist when/if + # the time comes to remove the incomplete VG and its PVs. + for pv in vg.pvs: + devicelibs.lvm.lvm_cc_addFilterRejectRegexp(pv.name)
def hide(self, device): for d in self.getChildren(device): @@ -2050,6 +2041,9 @@ class DeviceTree(object):
found = None for device in self._devices: + if not getattr(device, "complete", True): + continue + if device.name == name: found = device break @@ -2071,6 +2065,9 @@ class DeviceTree(object): leaf = None other = None for device in self._devices: + if not getattr(device, "complete", True): + continue + if (device.path == path or ((device.type == "lvmlv" or device.type == "lvmvg") and device.path == path.replace("--","-"))): @@ -2107,6 +2104,9 @@ class DeviceTree(object): """ List of device instances """ devices = [] for device in self._devices: + if not getattr(device, "complete", True): + continue + if device.uuid and device.uuid in [d.uuid for d in devices] and \ not isinstance(device, NoDevice): raise DeviceTreeError("duplicate uuids in device tree") diff --git a/pyanaconda/ui/gui/spokes/custom.glade b/pyanaconda/ui/gui/spokes/custom.glade index 9bf53d5..c763cf1 100644 --- a/pyanaconda/ui/gui/spokes/custom.glade +++ b/pyanaconda/ui/gui/spokes/custom.glade @@ -1504,6 +1504,94 @@ you'll be able to view their details here.</property> <child type="tab"> <placeholder/> </child> + <child> + <object class="GtkBox" id="incompleteDeviceBox"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="orientation">vertical</property> + <property name="spacing">6</property> + <child> + <object class="GtkBox" id="box5"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="spacing">6</property> + <child> + <object class="GtkLabel" id="incompleteDeviceLabel"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="label" translatable="yes">Selected Device</property> + <attributes> + <attribute name="font-desc" value="Cantarell Bold 10"/> + <attribute name="weight" value="bold"/> + <attribute name="scale" value="1.2"/> + </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="incompleteDeviceDescriptionLabel"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="halign">end</property> + <property name="label" translatable="yes">Device description</property> + <attributes> + <attribute name="font-desc" value="Cantarell Italic 10"/> + <attribute name="style" value="italic"/> + </attributes> + </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">0</property> + </packing> + </child> + <child> + <object class="GtkSeparator" id="separator4"> + <property name="visible">True</property> + <property name="can_focus">False</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + <child> + <object class="GtkLabel" id="incompleteDeviceOptionsLabel"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="yalign">0.43000000715255737</property> + <property name="label" translatable="yes">This %s device is missing %d of %d %s. You can remove it or select a different device.</property> + <property name="wrap">True</property> + <attributes> + <attribute name="font-desc" value="Cantarell 11"/> + </attributes> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">2</property> + </packing> + </child> + </object> + <packing> + <property name="position">4</property> + </packing> + </child> + <child type="tab"> + <placeholder/> + </child> </object> <packing> <property name="left_attach">1</property> diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 78047a9..e7534b3 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -94,6 +94,7 @@ NOTEBOOK_LABEL_PAGE = 0 NOTEBOOK_DETAILS_PAGE = 1 NOTEBOOK_LUKS_PAGE = 2 NOTEBOOK_UNEDITABLE_PAGE = 3 +NOTEBOOK_INCOMPLETE_PAGE = 4
new_install_name = N_("New %s %s Installation") new_vg_text = N_("Create a new volume group ...") @@ -563,6 +564,10 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._unused_devices = [d for d in self.__storage.unusedDevices if d.disks and not d.partitioned and d.isleaf] + # add incomplete VGs and MDs + incomplete = [d for d in self.__storage.devicetree._devices + if not getattr(d, "complete", True)] + self._unused_devices.extend(incomplete)
return self._unused_devices
@@ -2122,22 +2127,40 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
self._current_selector.set_chosen(False)
+ no_edit = False if selector._device.format.type == "luks" and \ selector._device.format.exists: self._partitionsNotebook.set_current_page(NOTEBOOK_LUKS_PAGE) selectedDeviceLabel = self.builder.get_object("encryptedDeviceLabel") selectedDeviceDescLabel = self.builder.get_object("encryptedDeviceDescriptionLabel") - selectedDeviceLabel.set_text(selector.props.name) - selectedDeviceDescLabel.set_text(self._description(selector.props.name)) - selector.set_chosen(True) - self._current_selector = selector - self._configButton.set_sensitive(False) - self._removeButton.set_sensitive(True) - return + no_edit = True + elif not getattr(selector._device, "complete", True): + self._partitionsNotebook.set_current_page(NOTEBOOK_INCOMPLETE_PAGE) + selectedDeviceLabel = self.builder.get_object("incompleteDeviceLabel") + selectedDeviceDescLabel = self.builder.get_object("incompleteDeviceDescriptionLabel") + optionsLabel = self.builder.get_object("incompleteDeviceOptionsLabel") + + if selector._device.type == "mdarray": + total = selector._device.memberDevices + missing = total - len(selector._device.parents) + txt = _("This Software RAID array is missing %d of %d member " + "partitions. You can remove it or select a different " + "device.") % (missing, total) + else: + total = selector._device.pvCount + missing = total - len(selector._device.parents) + txt = _("This LVM Volume Group is missing %d of %d physical " + "volumes. You can remove it or select a different " + "device.") % (missing, total) + optionsLabel.set_text(txt) + no_edit = True elif getDeviceType(selector._device) is None: self._partitionsNotebook.set_current_page(NOTEBOOK_UNEDITABLE_PAGE) selectedDeviceLabel = self.builder.get_object("uneditableDeviceLabel") selectedDeviceDescLabel = self.builder.get_object("uneditableDeviceDescriptionLabel") + no_edit = True + + if no_edit: selectedDeviceLabel.set_text(selector._device.name) selectedDeviceDescLabel.set_text(self._description(selector._device.type)) selector.set_chosen(True)
On Mon, 2012-12-17 at 16:33 -0600, David Lehman wrote:
TODO: Test handling of a complete and an incomplete array with the same name.
I did this. It went surprisingly well. I'm removing this TODO from the commit in my local repo now.
@@ -2122,22 +2127,40 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
self._current_selector.set_chosen(False)
no_edit = False if selector._device.format.type == "luks" and \ selector._device.format.exists: self._partitionsNotebook.set_current_page(NOTEBOOK_LUKS_PAGE) selectedDeviceLabel = self.builder.get_object("encryptedDeviceLabel") selectedDeviceDescLabel = self.builder.get_object("encryptedDeviceDescriptionLabel")
selectedDeviceLabel.set_text(selector.props.name)selectedDeviceDescLabel.set_text(self._description(selector.props.name))selector.set_chosen(True)self._current_selector = selectorself._configButton.set_sensitive(False)self._removeButton.set_sensitive(True)return
no_edit = Trueelif not getattr(selector._device, "complete", True):self._partitionsNotebook.set_current_page(NOTEBOOK_INCOMPLETE_PAGE)selectedDeviceLabel = self.builder.get_object("incompleteDeviceLabel")selectedDeviceDescLabel = self.builder.get_object("incompleteDeviceDescriptionLabel")optionsLabel = self.builder.get_object("incompleteDeviceOptionsLabel")if selector._device.type == "mdarray":total = selector._device.memberDevicesmissing = total - len(selector._device.parents)txt = _("This Software RAID array is missing %d of %d member ""partitions. You can remove it or select a different ""device.") % (missing, total)else:total = selector._device.pvCountmissing = total - len(selector._device.parents)txt = _("This LVM Volume Group is missing %d of %d physical ""volumes. You can remove it or select a different ""device.") % (missing, total)
For these kinds of strings, it's easier to translate if you use the dict form of substitution. See distributionText in product.py for an example.
- Chris
--- pyanaconda/ui/gui/spokes/custom.py | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index e7534b3..7137f68 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -2095,6 +2095,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): def on_vg_changed(self, combo): vg_name = combo.get_active_text() log.debug("new vg selection: %s" % vg_name) + if vg_name is None: + return + if vg_name == new_vg_text: # run the vg editor dialog with a default name and disk set hostname = self.data.network.hostname
On Mon, Dec 17, 2012 at 04:33:37PM -0600, David Lehman wrote:
Patches 1 and 3 fix tracebacks that are pretty easy to hit.
Patch 2 adds handling for systems with incomplete lvm/md devices.
Ack to these.
1 & 3 look ok for f18 to me, even w/o blocker bugs. #2 is a pretty big change, do we have a bug for it we could nominate for blocker?
On Mon, 2012-12-17 at 16:45 -0800, Brian C. Lane wrote:
On Mon, Dec 17, 2012 at 04:33:37PM -0600, David Lehman wrote:
Patches 1 and 3 fix tracebacks that are pretty easy to hit.
Patch 2 adds handling for systems with incomplete lvm/md devices.
Ack to these.
1 & 3 look ok for f18 to me, even w/o blocker bugs. #2 is a pretty big change, do we have a bug for it we could nominate for blocker?
I just forgot to note it before sending the patch off: bug 876441
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
anaconda-patches@lists.fedorahosted.org
-
Brian C. Lane -
Chris Lumens -
David Lehman