[blivet:rhel7/master 2/2] Lots of work on DeviceTree.hide (#1043763)
Anne Mulhern
amulhern at redhat.com
Mon Feb 24 23:07:06 UTC 2014
I just noticed that:
It looks like _removeDevice was deliberately _not_ called originally so that
DeviceTree.names would not be changed.
Is not changing DeviceTree.names still important when hiding a device?
If so, then it looks like _removeDevice is gonna need a save_names flag and there may be some additional consequences.
Perhaps that flag would be True if the device was just being hidden and not totally removed.
- mulhern
----- Original Message -----
> From: "mulhern" <amulhern at redhat.com>
> To: anaconda-patches at lists.fedorahosted.org
> Cc: "mulhern" <amulhern at redhat.com>
> Sent: Monday, February 24, 2014 5:31:41 PM
> Subject: [blivet:rhel7/master 2/2] Lots of work on DeviceTree.hide (#1043763)
>
> Resolves: rhbz#1043763
>
> The major changes are:
>
> 1) The lists of actions and devices to remove are calculated before any
> actions
> or devices are removed. Removing actions can have side-effects
> on their associated devices which can make subsequent calculations have
> errors.
> 2) The hide method is no longer itself recursive. The recursion interleaved
> the discovery of what devices and actions to remove with their removal,
> which was bad (see 1).
> 3) All logical volumes and physical volumes belonging to a volume group that
> is being hidden are also hidden.
> 4) When a device is removed, the internal _removeDevice function is used.
> Previously the device was just removed from the list of devices, and the
> pyparted information was therefore not updated correctly.
>
> Note: actions are removed after devices.
> If you remove the actions first, the devices are incomplete and you can not
> call _removeDevice on them without crashing with errors like "DeviceFormat
> object has no attribute 'x'". Actions are not removed using the
> cancelAction()
> method as that method has side-effects on the devicetree, specifically the
> devices included in the tree, which it is better to avoid.
>
> Signed-off-by: mulhern <amulhern at redhat.com>
> ---
> blivet/devicetree.py | 76
> ++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 53 insertions(+), 23 deletions(-)
>
> diff --git a/blivet/devicetree.py b/blivet/devicetree.py
> index 8a79f14..888670b 100644
> --- a/blivet/devicetree.py
> +++ b/blivet/devicetree.py
> @@ -1792,45 +1792,75 @@ class DeviceTree(object):
> devicelibs.lvm.lvm_cc_addFilterRejectRegexp(pv.name)
>
> def hide(self, device):
> - if device in self._hidden:
> - return
>
> - for d in self.getChildren(device):
> - self.hide(d)
> + def hide_device(device):
> + """Hide a single device.
>
> - log.info("hiding device %s %s (id %d)" % (device.type,
> - device.name,
> - device.id))
> + If the device does not exist, just remove it.
>
> - for action in reversed(self._actions):
> - if not action.device.dependsOn(device) and action.device !=
> device:
> - continue
> + self.names is not modified, even if a device is removed
> + """
> + log.info("hiding device %s %s (id %d)" % (device.type,
> + device.name,
> + device.id))
> + self._removeDevice(device)
> + if not device.exists:
> + return
>
> + self._hidden.append(device)
> + lvm.lvm_cc_addFilterRejectRegexp(device.name)
> +
> + if isinstance(device, DASDDevice):
> + self.dasd.removeDASD(device)
> +
> + def remove_action(action):
> + """Cancel a single action."""
> log.debug("cancelling action: %s" % action)
> try:
> action.cancel()
> except Exception:
> - log.warning("failed to cancel action while hiding %s: %s"
> - % (device.name, action))
> + log.warning("failed to cancel action %s" % action)
> finally:
> self._actions.remove(action)
>
> - # XXX modifications that do not require actions, like setting a
> - # mountpoint, will not be reversed here
> + def getDependents(device, seen=[]):
> + """Get all devices that depend on this device. Assumes that
> + the graph (where the nodes are devices and the edges are
> + dependency relationships) may contain cycles.
>
> - # we're intentionally not modifying self.names here
> - self._devices.remove(device)
> - for parent in device.parents:
> - parent.removeChild()
> + The device itself is included among its dependents.
> + """
> + seen = seen + [device]
>
> - if not device.exists:
> + dependents = self.getChildren(device)
> + if isinstance(device, LVMVolumeGroupDevice):
> + dependents += device.lvs + device.pvs
> +
> + for d in dependents:
> + if not d in seen:
> + seen = getDependents(d, seen)
> + return seen
> +
> + def getDependentActions(devices):
> + cancels = []
> + for device in devices:
> + cancels += [ a for a in reversed(self._actions) if
> a.device.dependsOn(device) or a.device == device ]
> + return list(set(cancels))
> +
> + if device in self._hidden:
> return
>
> - self._hidden.append(device)
> - lvm.lvm_cc_addFilterRejectRegexp(device.name)
> + dependents = getDependents(device)
> + cancels = getDependentActions(dependents)
> +
> + while dependents:
> + leaves = [ d for d in dependents if d.isleaf ]
> + for leaf in leaves:
> + hide_device(leaf)
> + dependents.remove(leaf)
>
> - if isinstance(device, DASDDevice):
> - self.dasd.removeDASD(device)
> + for action in cancels:
> + remove_action(action)
>
> def unhide(self, device):
> # the hidden list should be in leaves-first order
> --
> 1.8.3.1
>
>
More information about the anaconda-patches
mailing list