[PATCH 6/9] Add action classes for container member set management.
David Lehman
dlehman at redhat.com
Fri Mar 21 15:14:02 UTC 2014
On Fri, 2014-03-21 at 08:51 +0100, Vratislav Podzimek wrote:
> On Thu, 2014-03-20 at 12:14 -0500, David Lehman wrote:
> > ---
> > blivet/deviceaction.py | 184 ++++++++++++++++++++++++++++++++++++++++++++++---
> > tests/action_test.py | 96 ++++++++++++++++++++++++++
> > 2 files changed, 271 insertions(+), 9 deletions(-)
> >
> > diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
> > index d9a6a87..569377a 100644
> > --- a/blivet/deviceaction.py
> > +++ b/blivet/deviceaction.py
> > @@ -1,8 +1,7 @@
> > # deviceaction.py
> > -# Device modification action classes for anaconda's storage configuration
> > -# module.
> > +# Device modification action classes.
> > #
> > -# Copyright (C) 2009 Red Hat, Inc.
> > +# Copyright (C) 2009-2014 Red Hat, Inc.
> > #
> > # This copyrighted material is made available to anyone wishing to use,
> > # modify, copy, or redistribute it subject to the terms and conditions of
> > @@ -49,19 +48,25 @@ ACTION_TYPE_NONE = 0
> > ACTION_TYPE_DESTROY = 1000
> > ACTION_TYPE_RESIZE = 500
> > ACTION_TYPE_CREATE = 100
> > +ACTION_TYPE_ADD = 50
> > +ACTION_TYPE_REMOVE = 10
> >
> > action_strings = {ACTION_TYPE_NONE: "None",
> > ACTION_TYPE_DESTROY: "Destroy",
> > ACTION_TYPE_RESIZE: "Resize",
> > - ACTION_TYPE_CREATE: "Create"}
> > + ACTION_TYPE_CREATE: "Create",
> > + ACTION_TYPE_ADD: "Add",
> > + ACTION_TYPE_REMOVE: "Remove"}
> If we change those dicts to:
> { key1: val1,
> key2: val2,
> ...
> keyN: valN,
> }
>
> we will be able to add more items without ruining the git history of the
> last item. But that's just a neatpick.
I always disliked that syntax, but I hadn't thought of that reason to
like/use it.
>
> >
> > ACTION_OBJECT_NONE = 0
> > ACTION_OBJECT_FORMAT = 1
> > ACTION_OBJECT_DEVICE = 2
> > +ACTION_OBJECT_CONTAINER = 3
> >
> > object_strings = {ACTION_OBJECT_NONE: "None",
> > ACTION_OBJECT_FORMAT: "Format",
> > - ACTION_OBJECT_DEVICE: "Device"}
> > + ACTION_OBJECT_DEVICE: "Device",
> > + ACTION_OBJECT_CONTAINER: "Container"}
> >
> > RESIZE_SHRINK = 88
> > RESIZE_GROW = 89
> > @@ -148,6 +153,7 @@ class DeviceAction(util.ObjectID):
> > if not isinstance(device, StorageDevice):
> > raise ValueError("arg 1 must be a StorageDevice instance")
> > self.device = device
> > + self.container = getattr(self.device, "container", None)
> >
> > def execute(self):
> > """ perform the action """
> > @@ -178,10 +184,22 @@ class DeviceAction(util.ObjectID):
> > return (self.type == ACTION_TYPE_RESIZE and self.dir == RESIZE_GROW)
> >
> > @property
> > + def isAdd(self):
> > + return self.type == ACTION_TYPE_ADD
> > +
> > + @property
> > + def isRemove(self):
> > + return self.type == ACTION_TYPE_REMOVE
> > +
> > + @property
> > def isDevice(self):
> > return self.obj == ACTION_OBJECT_DEVICE
> >
> > @property
> > + def isContainer(self):
> > + return self.obj == ACTION_OBJECT_CONTAINER
> > +
> > + @property
> > def isFormat(self):
> > return self.obj == ACTION_OBJECT_FORMAT
> >
> > @@ -223,7 +241,7 @@ class DeviceAction(util.ObjectID):
> > return _(self.typeDescStr)
> >
> > def __str__(self):
> > - s = "[%d] %s %s" % (self.id, self.typeString, self.objectString)
> > + s = "[%d] %s" % (self.id, self.typeDescStr)
> > if self.isResize:
> > s += " (%s)" % self.resizeString
> > if self.isFormat:
> > @@ -273,6 +291,7 @@ class ActionCreateDevice(DeviceAction):
> > - this action's device depends on the other action's device
> > - both actions are partition create actions on the same disk
> > and this partition has a higher number
> > + - the other action adds a member to this device's container
> > """
> > rc = False
> > if self.device.dependsOn(action.device):
> > @@ -292,6 +311,9 @@ class ActionCreateDevice(DeviceAction):
> > self.device.vg == action.device.vg and
> > action.device.singlePV and not self.device.singlePV):
> > rc = True
> > + elif (action.isAdd and action.container == self.container):
> > + rc = True
> > +
> > return rc
> >
> >
> > @@ -325,6 +347,7 @@ class ActionDestroyDevice(DeviceAction):
> > - the other action's device depends on this action's device
> > - both actions are partition create actions on the same disk
> > and this partition has a lower number
> > + - the other action removes this action's device from a container
> > """
> > rc = False
> > if action.device.dependsOn(self.device) and action.isDestroy:
> > @@ -342,6 +365,8 @@ class ActionDestroyDevice(DeviceAction):
> > action.device.id == self.device.id):
> > # device destruction comes after destruction of device's format
> > rc = True
> > + elif (action.isRemove and action.device == self.device):
> > + rc = True
> > return rc
> >
> > def obsoletes(self, action):
> > @@ -352,6 +377,10 @@ class ActionDestroyDevice(DeviceAction):
> >
> > - obsoletes all but ActionDestroyFormat actions w/ lower id on the
> > same device if device exists
> > +
> > + - obsoletes all actions that add a member to this action's
> > + (container) device
> > +
> > """
> > rc = False
> > if action.device.id == self.device.id:
> > @@ -361,6 +390,10 @@ class ActionDestroyDevice(DeviceAction):
> > self.device.exists and \
> > not (action.isDestroy and action.isFormat):
> > rc = True
> > + elif action.isAdd and (action.device == self.device):
> > + rc = True
> > + elif action.isAdd and (action.container == self.device):
> > + rc = True
> >
> > return rc
> >
> > @@ -413,6 +446,8 @@ class ActionResizeDevice(DeviceAction):
> > this action's device depends on
> > - the other action shrinks a device (or format it contains)
> > that depends on this action's device
> > + - the other action removes this action's device from a container
> > + - the other action adds a member to this device's container
> > """
> > retval = False
> > if action.isResize:
> > @@ -424,6 +459,10 @@ class ActionResizeDevice(DeviceAction):
> > retval = True
> > elif action.isShrink and action.device.dependsOn(self.device):
> > retval = True
> > + elif (action.isRemove and action.device == self.device):
> > + rc = True
> > + elif (action.isAdd and action.container == self.container):
> > + rc = True
> These two 'rc's should be 'retval's.
Fixed throughout.
>
> >
> > return retval
> >
> > @@ -488,12 +527,14 @@ class ActionCreateFormat(DeviceAction):
> > Format create action can require another action if:
> >
> > - this action's device depends on the other action's device
> > - and the other action is not a device destroy action
> > + and the other action is not a device destroy action or a
> > + container action
> > - the other action is a create or resize of this action's
> > device
> > """
> > return ((self.device.dependsOn(action.device) and
> > - not (action.isDestroy and action.isDevice)) or
> > + not ((action.isDestroy and action.isDevice) or
> > + action.isContainer)) or
> > (action.isDevice and (action.isCreate or action.isResize) and
> > self.device.id == action.device.id))
> >
> > @@ -545,8 +586,15 @@ class ActionDestroyFormat(DeviceAction):
> >
> > - the other action's device depends on this action's device
> > and the other action is a destroy action
> > + - the other action removes this action's device from a container
> > """
> > - return action.device.dependsOn(self.device) and action.isDestroy
> > + retval = False
> > + if action.device.dependsOn(self.device) and action.isDestroy:
> > + retval = True
> > + elif (action.isRemove and action.device == self.device):
> > + rc = True
> Same here.
>
> > +
> > + return retval
> >
> > def obsoletes(self, action):
> > """ Return True if this action obsoletes action.
> > @@ -611,6 +659,7 @@ class ActionResizeFormat(DeviceAction):
> > that depends on this action's device
> > - the other action grows a device (or format) that this
> > action's device depends on
> > + - the other action removes this action's device from a container
> > """
> > retval = False
> > if action.isResize:
> > @@ -622,5 +671,122 @@ class ActionResizeFormat(DeviceAction):
> > retval = True
> > elif action.isGrow and self.device.dependsOn(action.device):
> > retval = True
> > + elif (action.isRemove and action.device == self.device):
> > + rc = True
> And here again.
>
> > +
> > + return retval
> > +
> > +
> > +class ActionAddMember(DeviceAction):
> > + """ An action representing addition of a member device to a container. """
> > + type = ACTION_TYPE_ADD
> > + obj = ACTION_OBJECT_CONTAINER
> > + typeDescStr = N_("add container member")
> > +
> > + def __init__(self, container, device):
> > + super(ActionAddMember, self).__init__(device)
> > + self.container = container
> > + container._addMember(device)
> > +
> > + def cancel(self):
> > + self.container._removeMember(self.device)
> > +
> > + def execute(self):
> > + self.container.add(self.device)
> > +
> > + def requires(self, action):
> > + """
> > + requires
> > + - create/resize the same device
> > +
> > + required by
> > + - any create/grow action on a device in the same container
> > + """
> > + retval = False
> > + if ((action.isCreate or action.isResize) and
> > + action.device == self.device):
> > + retval = True
> > +
> > + return retval
> This could be narrowed down to:
> return ((action.isCreate or action.isResize) and
> action.device == self.device)
Done.
>
> > +
> > + def obsoletes(self, action):
> > + """
> > + obsoletes
> > + - remove same member from same container
> > + - add same member to same container w/ higher id
> > +
> > + obsoleted by
> > + - destroy the container
> > + - destroy the device
> > + - remove same member from same container
> > + """
> > + retval = False
> > + if (action.isRemove and
> > + action.device == self.device and
> > + action.container == self.container):
> > + retval = True
> > + elif (action.isAdd and
> > + action.device == self.device and
> > + action.container == self.container and
> > + action.id > self.id):
> > + retval = True
> > +
> > + return retval
> This could again be a single return line or at least those retval
> assignments (with the first one dropped) should be replaced with return
> statements with 'return False' at the end.
I like having a single return statement in these types of places. It
makes debugging much easier if the need arises. Is there a significant
performance degradation in doing it this way?
>
> > +
> > +
> > +class ActionRemoveMember(DeviceAction):
> > + """ An action representing removal of a member device from a container. """
> > + type = ACTION_TYPE_REMOVE
> > + obj = ACTION_OBJECT_CONTAINER
> > + typeDescStr = N_("remove container member")
> > +
> > + def __init__(self, container, device):
> > + super(ActionRemoveMember, self).__init__(device)
> > + self.container = container
> > + container._removeMember(device)
> > +
> > + def cancel(self):
> > + self.container._addMember(self.device)
> > +
> > + def execute(self):
> > + self.container.remove(self.device)
> > +
> > + def requires(self, action):
> > + """
> > + requires
> > + - any destroy/shrink action on a device in the same container
> > + - any add action on this container
> > +
> > + required by
> > + - any destroy/resize action on the device
> > + """
> > + retval = False
> > + if ((action.isShrink or action.isDestroy) and
> > + action.device.container == self.container):
> > + retval = True
> > + elif action.isAdd and action.container == self.container:
> > + retval = True
> > +
> > + return retval
> > +
> > + def obsoletes(self, action):
> > + """
> > + obsoletes
> > + - add same member to same container
> > + - remove same member from same container w/ higher id
> > +
> > + obsoleted by
> > + - add same member to same container
> > + """
> > + retval = False
> > + if (action.isAdd and
> > + action.device == self.device and
> > + action.container == self.container):
> > + retval = True
> > + elif (action.isRemove and
> > + action.device == self.device and
> > + action.container == self.container and
> > + action.id > self.id):
> > + retval = True
> >
> > return retval
> Same in those two methods.
>
More information about the anaconda-patches
mailing list