[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