This is an alternative to #165. The approach is to deepcopy all format objects when copying, instead of to define an ```__copy__``` method that handles tasks specially.
The first two patches are the same as in #165.
It's tested, in that I tried an experiment whereby I made a deep copy of all formats objects in fstesting.py and ran all tests on the deepcopy, rather than the original format object, and they all passed. Using copy, instead of deepcopy, they all failed. Using copy, and overriding ```__copy__``` in FS, they all passed.
It should be noted that format objects are already deep copied, rather than shallow copied, in actionlist.
From: mulhern amulhern@redhat.com
It defines the initializer, which should not vary among filesystem tasks.
Include a convenience class for simple unimplemented filesystem tasks.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fstask.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 blivet/tasks/fstask.py
diff --git a/blivet/tasks/fstask.py b/blivet/tasks/fstask.py new file mode 100644 index 0000000..46dc3d6 --- /dev/null +++ b/blivet/tasks/fstask.py @@ -0,0 +1,47 @@ +# fstask.py +# Superclass for filesystem tasks. +# +# Copyright (C) 2015 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 +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): Anne Mulhern amulhern@redhat.com + +import abc + +from six import add_metaclass + +from . import task + +@add_metaclass(abc.ABCMeta) +class FSTask(task.Task): + """ An abstract class that encapsulates the fact that all FSTasks + have a single master object: the filesystem that they belong to. + """ + description = "parent of all filesystem tasks" + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + +class UnimplementedFSTask(FSTask, task.UnimplementedTask): + """ A convenience class for unimplemented filesystem tasks. + Useful in the usual case where an Unimplemented task has + no special methods that it is required to implement. + """ + pass
From: mulhern amulhern@redhat.com
Inherit from these classes wherever possible in filesystem task classes.
Get rid of all initializers in subclasses of FSTask.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsck.py | 20 ++++---------------- blivet/tasks/fsinfo.py | 20 ++++---------------- blivet/tasks/fsminsize.py | 20 ++++---------------- blivet/tasks/fsmkfs.py | 17 ++--------------- blivet/tasks/fsmount.py | 6 ++---- blivet/tasks/fsreadlabel.py | 20 ++++---------------- blivet/tasks/fsresize.py | 17 ++--------------- blivet/tasks/fssize.py | 29 +++++------------------------ blivet/tasks/fssync.py | 20 ++++---------------- blivet/tasks/fswritelabel.py | 20 ++++---------------- 10 files changed, 35 insertions(+), 154 deletions(-)
diff --git a/blivet/tasks/fsck.py b/blivet/tasks/fsck.py index 80d8661..ec97c96 100644 --- a/blivet/tasks/fsck.py +++ b/blivet/tasks/fsck.py @@ -27,12 +27,13 @@ from .. import util
from . import availability +from . import fstask from . import task
_UNKNOWN_RC_MSG = "Unknown return code: %d"
@add_metaclass(abc.ABCMeta) -class FSCK(task.BasicApplication): +class FSCK(task.BasicApplication, fstask.FSTask): """An abstract class that represents actions associated with checking consistency of a filesystem. """ @@ -41,13 +42,6 @@ class FSCK(task.BasicApplication): options = abc.abstractproperty( doc="Options for invoking the application.")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - # IMPLEMENTATION methods
@abc.abstractmethod @@ -147,11 +141,5 @@ class NTFSFSCK(FSCK): def _errorMessage(self, rc): return _UNKNOWN_RC_MSG % (rc,) if rc != 0 else None
-class UnimplementedFSCK(task.UnimplementedTask): - - def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs +class UnimplementedFSCK(fstask.UnimplementedFSTask): + pass diff --git a/blivet/tasks/fsinfo.py b/blivet/tasks/fsinfo.py index 13ed555..5962cb7 100644 --- a/blivet/tasks/fsinfo.py +++ b/blivet/tasks/fsinfo.py @@ -27,10 +27,11 @@ from .. import util
from . import availability +from . import fstask from . import task
@add_metaclass(abc.ABCMeta) -class FSInfo(task.BasicApplication): +class FSInfo(task.BasicApplication, fstask.FSTask): """ An abstract class that represents an information gathering app. """
description = "filesystem info" @@ -38,13 +39,6 @@ class FSInfo(task.BasicApplication): options = abc.abstractproperty( doc="Options for invoking the application.")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - @property def _infoCommand(self): """ Returns the command for reading filesystem information. @@ -96,11 +90,5 @@ class XFSInfo(FSInfo): ext = availability.XFSDB_APP options = ["-c", "sb 0", "-c", "p dblocks", "-c", "p blocksize"]
-class UnimplementedFSInfo(task.UnimplementedTask): - - def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs +class UnimplementedFSInfo(fstask.UnimplementedFSTask): + pass diff --git a/blivet/tasks/fsminsize.py b/blivet/tasks/fsminsize.py index 5044190..999240e 100644 --- a/blivet/tasks/fsminsize.py +++ b/blivet/tasks/fsminsize.py @@ -28,23 +28,17 @@ from ..size import Size
from . import availability +from . import fstask from . import task
@add_metaclass(abc.ABCMeta) -class FSMinSize(task.BasicApplication): +class FSMinSize(task.BasicApplication, fstask.FSTask): """ An abstract class that represents min size information extraction. """
description = "minimum filesystem size"
options = abc.abstractproperty(doc="Options for use with app.")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - def _resizeCommand(self): return [str(self.ext)] + self.options + [self.fs.device]
@@ -177,11 +171,5 @@ def doTask(self): raise FSError("Unable to discover minimum size of filesystem on %s" % self.fs.device) return minSize
-class UnimplementedFSMinSize(task.UnimplementedTask): - - def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs +class UnimplementedFSMinSize(fstask.UnimplementedFSTask): + pass diff --git a/blivet/tasks/fsmkfs.py b/blivet/tasks/fsmkfs.py index a2110a9..542e61d 100644 --- a/blivet/tasks/fsmkfs.py +++ b/blivet/tasks/fsmkfs.py @@ -27,10 +27,11 @@ from .. import util
from . import availability +from . import fstask from . import task
@add_metaclass(abc.ABCMeta) -class FSMkfsTask(task.Task): +class FSMkfsTask(fstask.FSTask):
canLabel = abc.abstractproperty(doc="whether this task labels")
@@ -44,13 +45,6 @@ class FSMkfs(task.BasicApplication, FSMkfsTask):
args = abc.abstractproperty(doc="options for creating filesystem")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - # IMPLEMENTATION methods
@property @@ -219,13 +213,6 @@ def args(self):
class UnimplementedFSMkfs(task.UnimplementedTask, FSMkfsTask):
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - @property def canLabel(self): return False diff --git a/blivet/tasks/fsmount.py b/blivet/tasks/fsmount.py index 552faed..9b674d5 100644 --- a/blivet/tasks/fsmount.py +++ b/blivet/tasks/fsmount.py @@ -27,9 +27,10 @@ from ..formats import fslib
from . import availability +from . import fstask from . import task
-class FSMount(task.BasicApplication): +class FSMount(task.BasicApplication, fstask.FSTask): """An abstract class that represents filesystem mounting actions. """ description = "mount a filesystem"
@@ -39,9 +40,6 @@ class FSMount(task.BasicApplication):
ext = availability.MOUNT_APP
- def __init__(self, an_fs): - self.fs = an_fs - # TASK methods
@property diff --git a/blivet/tasks/fsreadlabel.py b/blivet/tasks/fsreadlabel.py index 59e3e12..ec00f73 100644 --- a/blivet/tasks/fsreadlabel.py +++ b/blivet/tasks/fsreadlabel.py @@ -28,10 +28,11 @@ from .. import util
from . import availability +from . import fstask from . import task
@add_metaclass(abc.ABCMeta) -class FSReadLabel(task.BasicApplication): +class FSReadLabel(task.BasicApplication, fstask.FSTask): """ An abstract class that represents reading a filesystem's label. """ description = "read filesystem label"
@@ -40,13 +41,6 @@ class FSReadLabel(task.BasicApplication):
args = abc.abstractproperty(doc="arguments for reading a label.")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - # IMPLEMENTATION methods
@property @@ -123,11 +117,5 @@ class XFSReadLabel(FSReadLabel): def args(self): return ["-l", self.fs.device]
-class UnimplementedFSReadLabel(task.UnimplementedTask): - - def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs +class UnimplementedFSReadLabel(fstask.UnimplementedFSTask): + pass diff --git a/blivet/tasks/fsresize.py b/blivet/tasks/fsresize.py index ef12686..6fb5e0e 100644 --- a/blivet/tasks/fsresize.py +++ b/blivet/tasks/fsresize.py @@ -28,10 +28,11 @@ from ..import util
from . import availability +from . import fstask from . import task
@add_metaclass(abc.ABCMeta) -class FSResizeTask(task.Task): +class FSResizeTask(fstask.FSTask): """ The abstract properties that any resize task must have. """
unit = abc.abstractproperty(doc="Resize unit.") @@ -45,13 +46,6 @@ class FSResize(task.BasicApplication, FSResizeTask):
args = abc.abstractproperty(doc="Resize arguments.")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - # IMPLEMENTATION methods
@abc.abstractmethod @@ -135,13 +129,6 @@ def args(self):
class UnimplementedFSResize(task.UnimplementedTask, FSResizeTask):
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - @property def unit(self): raise NotImplementedError() diff --git a/blivet/tasks/fssize.py b/blivet/tasks/fssize.py index b28083e..aa82ac0 100644 --- a/blivet/tasks/fssize.py +++ b/blivet/tasks/fssize.py @@ -29,26 +29,20 @@ from .. import util
from . import availability +from . import fstask from . import task
_tags = ("count", "size") _Tags = namedtuple("_Tags", _tags)
@add_metaclass(abc.ABCMeta) -class FSSize(task.Task): +class FSSize(fstask.FSTask): """ An abstract class that represents size information extraction. """ description = "current filesystem size"
tags = abc.abstractproperty( doc="Strings used for extracting components of size.")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - # TASK methods
@property @@ -121,18 +115,11 @@ class ReiserFSSize(FSSize): class XFSSize(FSSize): tags = _Tags(size="blocksize =", count="dblocks =")
-class TmpFSSize(task.BasicApplication): +class TmpFSSize(task.BasicApplication, fstask.FSTask): description = "current filesystem size"
ext = availability.DF_APP
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - @property def _sizeCommand(self): return [str(self.ext), self.fs.systemMountpoint, "--output=size"] @@ -156,11 +143,5 @@ def doTask(self): return Size("%s KiB" % lines[1])
-class UnimplementedFSSize(task.UnimplementedTask): - - def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs +class UnimplementedFSSize(fstask.UnimplementedFSTask): + pass diff --git a/blivet/tasks/fssync.py b/blivet/tasks/fssync.py index e83aa12..e08288d 100644 --- a/blivet/tasks/fssync.py +++ b/blivet/tasks/fssync.py @@ -27,21 +27,15 @@ from .. import util
from . import availability +from . import fstask from . import task
@add_metaclass(abc.ABCMeta) -class FSSync(task.BasicApplication): +class FSSync(task.BasicApplication, fstask.FSTask): """ An abstract class that represents syncing a filesystem. """
description = "filesystem syncing"
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - @abc.abstractmethod def doTask(self): raise NotImplementedError() @@ -79,11 +73,5 @@ def doTask(self, root="/"): if error_msg: raise FSError(error_msg)
-class UnimplementedFSSync(task.UnimplementedTask): - - def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs +class UnimplementedFSSync(fstask.UnimplementedFSTask): + pass diff --git a/blivet/tasks/fswritelabel.py b/blivet/tasks/fswritelabel.py index af64d92..50eacdf 100644 --- a/blivet/tasks/fswritelabel.py +++ b/blivet/tasks/fswritelabel.py @@ -27,23 +27,17 @@ from ..errors import FSWriteLabelError
from . import availability +from . import fstask from . import task
@add_metaclass(abc.ABCMeta) -class FSWriteLabel(task.BasicApplication): +class FSWriteLabel(task.BasicApplication, fstask.FSTask): """ An abstract class that represents writing a label for a filesystem. """
description = "write filesystem label"
args = abc.abstractproperty(doc="arguments for writing a label")
- def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs - # IMPLEMENTATION methods
@property @@ -106,11 +100,5 @@ class XFSWriteLabel(FSWriteLabel): def args(self): return ["-L", self.fs.label if self.fs.label != "" else "--", self.fs.device]
-class UnimplementedFSWriteLabel(task.UnimplementedTask): - - def __init__(self, an_fs): - """ Initializer. - - :param FS an_fs: a filesystem object - """ - self.fs = an_fs +class UnimplementedFSWriteLabel(fstask.UnimplementedFSTask): + pass
From: mulhern amulhern@redhat.com
To avoid sharing of mutable subparts.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/blivet.py | 2 +- blivet/devices/btrfs.py | 2 +- blivet/devices/storage.py | 2 +- blivet/populator.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/blivet/blivet.py b/blivet/blivet.py index cc9607f..3cdf629 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -1121,7 +1121,7 @@ def resetDevice(self, device): self.devicetree.cancelAction(action)
# make sure any random overridden attributes are reset - device.format = copy.copy(device.originalFormat) + device.format = copy.deepcopy(device.originalFormat)
def resizeDevice(self, device, new_size): """ Schedule a resize of a device and its formatting, if any. diff --git a/blivet/devices/btrfs.py b/blivet/devices/btrfs.py index 867e271..56f0f37 100644 --- a/blivet/devices/btrfs.py +++ b/blivet/devices/btrfs.py @@ -204,7 +204,7 @@ def __init__(self, *args, **kwargs): device=self.path, subvolspec=self.vol_id, mountopts="subvolid=%d" % self.vol_id) - self.originalFormat = copy.copy(self.format) + self.originalFormat = copy.deepcopy(self.format)
self._defaultSubVolumeID = None
diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py index 525db63..911652c 100644 --- a/blivet/devices/storage.py +++ b/blivet/devices/storage.py @@ -124,7 +124,7 @@ def __init__(self, name, fmt=None, uuid=None, self.controllable = not flags.testing
self.format = fmt - self.originalFormat = copy.copy(self.format) + self.originalFormat = copy.deepcopy(self.format) self.fstabComment = "" self._targetSize = self._size
diff --git a/blivet/populator.py b/blivet/populator.py index 8927b47..323094f 100644 --- a/blivet/populator.py +++ b/blivet/populator.py @@ -756,7 +756,7 @@ def addUdevDevice(self, info, updateOrigFmt=False): # now handle the device's formatting self.handleUdevDeviceFormat(info, device) if device_added or updateOrigFmt: - device.originalFormat = copy.copy(device.format) + device.originalFormat = copy.deepcopy(device.format) device.deviceLinks = udev.device_get_symlinks(info)
def handleUdevDiskLabelFormat(self, info, device):
Looks good to me and I like it more than PR #165
Added label: ACK.
Looks good.
Closed.
anaconda-patches@lists.fedorahosted.org
-
dashea