The purpose of this patch is to make blivet able to operate in a context in which it is aware of what external dependencies are available. What this means is a couple of things:
* If a DeviceAction that requires an external resource is constructed and that resource is unavailable then the constructor will raise an exception. Currently these exceptions are ValueErrors, which are the same kind of exception raised when the device or some other aspect of the environment has a problem.
* All methods on formats like formattable, supported, are environment aware. Previously they usually returned True if the functionality was implemented in blivet...now they only return True if the functionality is implemented _and_ also the external tool required is available. So, where anaconda uses methods to determine whether to display a particular filesystem type in a combo, those should all return False if the operations they check can not be run.
* Ultimately, if by some chance a method is called requiring a task with an unavailable external dependency, an exception will be raised indicating that the problem is the unavailability of the external application. This is already implemented in the filesystem format tasks and will be implemented in the libblockdev plugins. There are two external apps in devices, multipath and kpartx for which perhaps it should be implemented as well.
Availability of resources has its own separate simple packages. There are only two kinds of external resources so far, libblockdev plugins and executables. libblockdev plugin availability is checked by loadedness, which is always going to be correct on an installed system. executable are available if their names are in $PATH.
There are three different categories of things to track availability of, in order of difficulty are: * non-filesystem formats * filesystem formats * devices
Non-filesystem formats are easy, because if they depend on anything external they depend on exactly one libblockdev plugin. SwapSpace is an example and all others are handled similarly. All methods like formattable, supportable, typically just check whether the plugin is available. There's just not much to look at for this.
Filesystem formats are bit harder. Each filesystem has a lot of tasks to do, create, mount, fsck, info getting, size calculating, etc, and these can be handled by different applications that get installed by different packages. To handle this, these tasks are abstracted out into subclasses of Task and each is set up with an unavailable and an unimplemented method. It is the availability of these tasks that is checked for the various filesystem format properties. Abstracting these tasks into separate tasks forms a significant part of these patches but these probably don't require a lot of scrutiny either. There are no additional tests for these tasks, as they are like friend classes of the filesystem formats and don't really make sense being tested outside the formats classes. They all pass a bunch of regression tests on filesystem formats.
Devices is of course the hardest. Besides device classes just generally being more intricate there are the following problems: * Calculations must be done recursively on ancestor devices. * Dependencies seem to accumulate as you travel down the class hierarchy. * Abstracting into tasks like for filesystem formats is not going to happen, but devices aren't really like non-filesystems formats in having just one simple dependency, either.
Under these circumstances it is impossible to be correct about all dependencies so I have aimed for being conservative, i.e., sometimes raising an exception when constructing a DeviceAction that actually could go through by some special circumstances. There are only two mechanisms: * an ability to find unavailable external resources for a device and all its ancestors * an ability to find unavailable external resources required for setting up formats for the formats on all the ancestors of a device I've chosen these because it seems, from study of the code, that for almost every DeviceAction a setup task must be performed that requires doing things to the device and to the formats on that device.
Generally there will be some small check on the specifics of the action as well; for example, the ActionDestroyFormat class has a check whether the format to be destroyed is actually destroyable. When creating a format, I check whether the format is formattable, DeviceFormat's are not formattable so an exception is raised in that case.
Currently, the search for unavailable external resources for a device and all its ancestors has only two modes, Default and Destroy. I chose to add the Destroy mode, because a PartitionDevice has a dependency on the dm plugin but only in the _postDestroy method and AFAICT, it would be a little harsh to require the dm plugin for every combination of DeviceAction, PartitionDevice. The mode mechanism is ready to be extended as necessary to refine the search to only relevant dependencies in case some other situation comes up where the DeviceAction constructor would raise an exception even though the action could be succesfully executed (and it is considered worthwhile to distinguish).
For checking whether formats are setupable I wrote two functions that mirror the structure of Device.setup and Device.setupParents, so that these function do not check the setupability of formats that setup does not actually try to setup.
In summary, it's probably OK to skip the code about formats, although you might want to look at the Tasks abstractions, and to focus on how I deal with devices, and how I insert calls into DeviceAction classes. That's the tricky part.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/task.py | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 blivet/tasks/task.py
diff --git a/blivet/tasks/task.py b/blivet/tasks/task.py new file mode 100644 index 0000000..d63fb1c --- /dev/null +++ b/blivet/tasks/task.py @@ -0,0 +1,63 @@ +# task.py +# Abstract class for 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 + +@add_metaclass(abc.ABCMeta) +class Task(object): + """ An abstract class that represents some task. """ + + @classmethod + @abc.abstractmethod + def available(cls): + """ A class-level method indicating whether tasks of this class + are available. + """ + raise NotImplementedError() + + description = abc.abstractproperty(doc="Brief description for this task.") + + # Note that unavailable is able to obtain more precise information than + # available, since it is able to access the filesystem object. + unavailable = abc.abstractproperty( + doc="Reason if the necessary external tools are unavailable.") + + unready = abc.abstractproperty( + doc="Reason if the external resource is not ready for this action.") + + unable = abc.abstractproperty( + doc="Reason if the object is not in a correct state for this task.") + + @property + def impossible(self): + """ Returns a reason if the task can not succeed, otherwise False. + + :returns: reason or False + :rtype: str or bool + """ + return self.unavailable or self.unable or self.unready + + @abc.abstractmethod + def doTask(self, *args, **kwargs): + """ Do the task for this class. """ + raise NotImplementedError()
In reply to line 41 of blivet/tasks/task.py:
The reference to *"the filesystem object"* is really cryptic here. Which filesystem object? Why's that?
In reply to line 43 of blivet/tasks/task.py:
Having both ``unavailable`` and ``available`` and not one being the opposite/negation of the other could lead to many issues and misunderstandings.
In reply to line 58 of blivet/tasks/task.py:
Maybe I'm just too positive person, but all these *un...* and *im...* properties are harder to think about for me than their "positive" counterparts. Can't these be ``available``, ``ready``, ``possible`` and something else for ``unable`` because that's confusing together with ``(un)ready``?
In reply to line 49 of blivet/tasks/task.py:
The doc for ``unable`` says what I'd expect to be ``unready`` and vice versa.
In reply to line 41 of blivet/tasks/task.py:
Yes, it should say something more like:
since it is generally able to access an object, unlike available() which can only access the object's class.
In reply to line 43 of blivet/tasks/task.py:
That's quite true, but unavoidable if we have both, since unavailable needs that precision. dlehman requested the class-level method available()...but it is currently unused. If he's amenable, it can be removed.
In reply to line 58 of blivet/tasks/task.py:
Those names would make a lot less sense with the return types that they actually have and they would be much harder to compose.
In reply to line 49 of blivet/tasks/task.py:
That would seem backwards to me. Neither word is super, but they were the best I could come up with.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/availability.py | 121 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 blivet/tasks/availability.py
diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py new file mode 100644 index 0000000..150ff2d --- /dev/null +++ b/blivet/tasks/availability.py @@ -0,0 +1,121 @@ +# availability.py +# Class for tracking availability of an application. +# +# Copyright (C) 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 +# 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 util + +class Application(object): + """ An application. """ + + def __init__(self, method, name): + """ Initializes an instance of an application. + + :param method: A method object + :type method: Method + :param str name: the name of the application + """ + self._method = method + self.name = name + + def __str__(self): + return self.name + + # Results of these methods may change at runtime, depending on system + # state. + + @property + def available(self): + """ Whether the resource is available. + + :returns: True if the resource is available, otherwise False + :rtype: bool + """ + return self._method.available(self) + + @property + def path(self): + """ Absolute path of directory containing the executable. + + :returns: Absolute path of directory containing executable. + :rtype: str or NoneType + + Returns None if available is False. + """ + return self._method.path(self) + + +@add_metaclass(abc.ABCMeta) +class Method(object): + """ A collection of methods for a particular application task. """ + + @abc.abstractmethod + def available(self, application): + """ Returns True if the application is available. + + :param application: any application + :type application: applications.application.Application + + :returns: True if the application is available + :rtype: bool + """ + raise NotImplementedError() + + @abc.abstractmethod + def path(self, application): + """ Returns the absolute path of the application. + + :param application: any application + :type application: applications.application.Application + + :returns: the absolute path of the application + :rtype: str or NoneType + + Returns None when available is False + """ + raise NotImplementedError() + +class Path(Method): + """ Methods for when application is found in PATH. """ + + def available(self, application): + """ Returns True if the name of the application is in the path. + + :param application: any application + :type application: applications.application.Application + + :returns: True if the name of the application is in the path + :rtype: bool + """ + return bool(self.path(application)) + + def path(self, application): + """ Obtain absolute path of executable. + + :param application: The application to locate. + :type application: applications.application.Application + + :returns: path of executable + :rtype: str + """ + return util.find_program_in_path(application.name)
In reply to line 4 of blivet/tasks/availability.py:
2015 or 2014-2015 would be better here
In reply to line 35 of blivet/tasks/availability.py:
I think this should be: ``` :type method: :class:`Method` ```
In reply to line 77 of blivet/tasks/availability.py:
Is this "path" to the ``Application`` class correct? Or is it the class defined above? In both cases ``:class:`` should be used here, AFAICT
In reply to line 96 of blivet/tasks/availability.py:
Are the two methods above expected to do something more than ``return application.path`` (or ``available``)? If yes, I'd really like to see it explained in the docstrings.
Maybe I'll get it while reading the follow-up patches, but I don't understand why we need 3 classes and methods for something like this. What are ``Path`` and ``Method`` good for? Are they going to have some attributes and hold some information other than a reference to an instance of ``Application``? If not, this should be reduced to two or three functions.
In reply to line 77 of blivet/tasks/availability.py:
docstrings are fixed now.
In reply to line 96 of blivet/tasks/availability.py:
This method got killed in a later commit, but rebasing the commits would have been a nightmare...
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fslabeling.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/blivet/formats/fslabeling.py b/blivet/formats/fslabeling.py index ef5b4ae..0abeaf7 100644 --- a/blivet/formats/fslabeling.py +++ b/blivet/formats/fslabeling.py @@ -61,8 +61,8 @@ def labelingArgs(self, label):
class Ext2FSLabeling(FSLabeling):
- default_label = property(lambda s: "") - label_app = property(lambda s: fslabel.E2Label) + default_label = "" + label_app = fslabel.E2Label
def labelFormatOK(self, label): return len(label) < 17 @@ -72,8 +72,8 @@ def labelingArgs(self, label):
class FATFSLabeling(FSLabeling):
- default_label = property(lambda s: "NO NAME") - label_app = property(lambda s: fslabel.DosFsLabel) + default_label = "NO NAME" + label_app = fslabel.DosFsLabel
def labelFormatOK(self, label): return len(label) < 12 @@ -83,8 +83,8 @@ def labelingArgs(self, label):
class JFSLabeling(FSLabeling):
- default_label = property(lambda s: "") - label_app = property(lambda s: fslabel.JFSTune) + default_label = "" + label_app = fslabel.JFSTune
def labelFormatOK(self, label): return len(label) < 17 @@ -94,8 +94,8 @@ def labelingArgs(self, label):
class ReiserFSLabeling(FSLabeling):
- default_label = property(lambda s: "") - label_app = property(lambda s: fslabel.ReiserFSTune) + default_label = "" + label_app = fslabel.ReiserFSTune
def labelFormatOK(self, label): return len(label) < 17 @@ -105,8 +105,8 @@ def labelingArgs(self, label):
class XFSLabeling(FSLabeling):
- default_label = property(lambda s: "") - label_app = property(lambda s: fslabel.XFSAdmin) + default_label = "" + label_app = fslabel.XFSAdmin
def labelFormatOK(self, label): return ' ' not in label and len(label) < 13 @@ -116,8 +116,8 @@ def labelingArgs(self, label):
class HFSLabeling(FSLabeling):
- default_label = property(lambda s: "Untitled") - label_app = property(lambda s: None) + default_label = "Untitled" + label_app = None
def labelFormatOK(self, label): return ':' not in label and len(label) < 28 and len(label) > 0 @@ -127,8 +127,8 @@ def labelingArgs(self, label):
class HFSPlusLabeling(FSLabeling):
- default_label = property(lambda s: "Untitled") - label_app = property(lambda s: None) + default_label = "Untitled" + label_app = None
def labelFormatOK(self, label): return ':' not in label and 0 < len(label) < 129 @@ -138,8 +138,8 @@ def labelingArgs(self, label):
class NTFSLabeling(FSLabeling):
- default_label = property(lambda s: "") - label_app = property(lambda s: fslabel.NTFSLabel) + default_label = "" + label_app = fslabel.NTFSLabel
def labelFormatOK(self, label): return len(label) < 129
I'd be really grateful for some explanation of why we can and should do this in the commit message.
I've referenced commit 3171acf404c31b541c4c57bb6aaeaaa45dcbd0fa where I first realized this was the right thing to do in this commit. There is a fuller explanation there.
From: mulhern amulhern@redhat.com
Tasks are things that require running some kind of application to get data, but don't write to data structures that represent storage.
Each task should encapsulate all its actions, as well as knowledge of whether the underlying resources that it requires are available.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 blivet/tasks/__init__.py
diff --git a/blivet/tasks/__init__.py b/blivet/tasks/__init__.py new file mode 100644 index 0000000..e69de29
From: mulhern amulhern@redhat.com
Create a separate tasks test directory and shuffle some tests into that directory. --- blivet/formats/fs.py | 2 +- blivet/formats/fslabel.py | 205 ------------------------------------ blivet/formats/fslabeling.py | 148 -------------------------- blivet/tasks/fslabel.py | 205 ++++++++++++++++++++++++++++++++++++ blivet/tasks/fslabeling.py | 148 ++++++++++++++++++++++++++ setup.py | 2 +- tests/formats_test/fslabeling.py | 160 ---------------------------- tests/formats_test/labeling_test.py | 142 ------------------------- tests/tasks_test/__init__.py | 0 tests/tasks_test/fslabeling.py | 160 ++++++++++++++++++++++++++++ tests/tasks_test/labeling_test.py | 142 +++++++++++++++++++++++++ 11 files changed, 657 insertions(+), 657 deletions(-) delete mode 100644 blivet/formats/fslabel.py delete mode 100644 blivet/formats/fslabeling.py create mode 100644 blivet/tasks/fslabel.py create mode 100644 blivet/tasks/fslabeling.py delete mode 100644 tests/formats_test/fslabeling.py delete mode 100755 tests/formats_test/labeling_test.py create mode 100644 tests/tasks_test/__init__.py create mode 100644 tests/tasks_test/fslabeling.py create mode 100755 tests/tasks_test/labeling_test.py
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 101c4c3..65acf6f 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -26,7 +26,7 @@ import os import tempfile
-from . import fslabeling +from ..tasks import fslabeling from ..errors import FormatCreateError, FSError, FSResizeError from . import DeviceFormat, register_device_format from .. import util diff --git a/blivet/formats/fslabel.py b/blivet/formats/fslabel.py deleted file mode 100644 index 81c18e6..0000000 --- a/blivet/formats/fslabel.py +++ /dev/null @@ -1,205 +0,0 @@ -# fslabel.py -# Filesystem labeling classes for anaconda's storage configuration module. -# -# Copyright (C) 2013 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 -import re - -from six import add_metaclass - -from .. import errors - -@add_metaclass(abc.ABCMeta) -class FSLabelApp(object): - """An abstract class that represents actions associated with a - filesystem's labeling application. - """ - - name = abc.abstractproperty( - doc="The name of the filesystem labeling application.") - - reads = abc.abstractproperty( - doc="Whether this application can read a label as well as write one.") - - _label_regex = abc.abstractproperty( - doc="Matches the string output by the label reading application.") - - @abc.abstractmethod - def _writeLabelArgs(self, fs): - """Returns a list of the arguments for writing a label. - - :param FS fs: a filesystem object - - :return: the arguments - :rtype: list of str - - It can be assumed in this function that fs.label is a str. - """ - raise NotImplementedError - - def setLabelCommand(self, fs): - """Get the command to label the filesystem. - - :param FS fs: a filesystem object - :return: the command - :rtype: list of str - - Raises an exception if fs.label is None. - """ - if fs.label is None: - raise errors.FSError("makes no sense to write a label when accepting default label") - return [self.name] + self._writeLabelArgs(fs) - - @abc.abstractmethod - def _readLabelArgs(self, fs): - """Returns a list of arguments for reading a label. - - :param FS fs: a filesystem object - :return: the arguments - :rtype: list of str - """ - raise NotImplementedError - - def readLabelCommand(self, fs): - """Get the command to read the filesystem label. - - :param FS fs: a filesystem object - :return: the command - :rtype: list of str - - Raises an FSError if this application can not read the label. - """ - if not self.reads: - raise errors.FSError("Application %s can not read the filesystem label." % self.name) - return [self.name] + self._readLabelArgs(fs) - - def extractLabel(self, labelstr): - """Extract the label from an output string. - - :param str labelstr: the string containing the label information - - :return: the label - :rtype: str - - Raises an FSError if the label can not be extracted. - """ - if not self.reads or self._label_regex is None: - raise errors.FSError("Unknown format for application %s" % self.name) - match = re.match(self._label_regex, labelstr) - if match is None: - raise errors.FSError("Unknown format for application %s" % self.name) - return match.group('label') - - -class E2Label(FSLabelApp): - """Application used by ext2 and its descendants.""" - - name = "e2label" - reads = True - - _label_regex = r'(?P<label>.*)' - - def _writeLabelArgs(self, fs): - return [fs.device, fs.label] - - def _readLabelArgs(self, fs): - return [fs.device] - -E2Label = E2Label() - -class DosFsLabel(FSLabelApp): - """Application used by FATFS.""" - - name = "dosfslabel" - reads = True - - _label_regex = r'(?P<label>.*)' - - def _writeLabelArgs(self, fs): - return [fs.device, fs.label] - - def _readLabelArgs(self, fs): - return [fs.device] - -DosFsLabel = DosFsLabel() - -class JFSTune(FSLabelApp): - """Application used by JFS.""" - - name = "jfs_tune" - reads = False - - _label_regex = property(lambda s: None) - - def _writeLabelArgs(self, fs): - return ["-L", fs.label, fs.device] - - def _readLabelArgs(self, fs): - raise NotImplementedError - -JFSTune = JFSTune() - -class ReiserFSTune(FSLabelApp): - """Application used by ReiserFS.""" - - name = "reiserfstune" - reads = False - - _label_regex = None - - def _writeLabelArgs(self, fs): - return ["-l", fs.label, fs.device] - - def _readLabelArgs(self, fs): - raise NotImplementedError - -ReiserFSTune = ReiserFSTune() - -class XFSAdmin(FSLabelApp): - """Application used by XFS.""" - - name = "xfs_admin" - reads = True - - _label_regex = r'label = "(?P<label>.*)"' - - def _writeLabelArgs(self, fs): - return ["-L", fs.label if fs.label != "" else "--", fs.device] - - def _readLabelArgs(self, fs): - return ["-l", fs.device] - -XFSAdmin = XFSAdmin() - -class NTFSLabel(FSLabelApp): - """Application used by NTFS.""" - - name = "ntfslabel" - reads = True - - _label_regex = r'(?P<label>.*)' - - def _writeLabelArgs(self, fs): - return [fs.device, fs.label] - - def _readLabelArgs(self, fs): - return [fs.device] - -NTFSLabel = NTFSLabel() diff --git a/blivet/formats/fslabeling.py b/blivet/formats/fslabeling.py deleted file mode 100644 index 0abeaf7..0000000 --- a/blivet/formats/fslabeling.py +++ /dev/null @@ -1,148 +0,0 @@ -# fslabeling.py -# Filesystem labeling classes for anaconda's storage configuration module. -# -# Copyright (C) 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 -# 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 fslabel - -@add_metaclass(abc.ABCMeta) -class FSLabeling(object): - """An abstract class that represents filesystem labeling actions. - """ - - default_label = abc.abstractproperty( - doc="Default label set on this filesystem at creation.") - - label_app = abc.abstractproperty( - doc="Post creation filesystem labeling application.") - - @abc.abstractmethod - def labelFormatOK(self, label): - """Returns True if this label is correctly formatted for this - filesystem, otherwise False. - - :param str label: the label for this filesystem - :rtype: bool - """ - raise NotImplementedError - - @abc.abstractmethod - def labelingArgs(self, label): - """Returns the arguments for writing the label during filesystem - creation. These arguments are intended to be passed to the - appropriate mkfs application. - - :param str label: the label to use - :return: the arguments - :rtype: list of str - """ - raise NotImplementedError - - -class Ext2FSLabeling(FSLabeling): - - default_label = "" - label_app = fslabel.E2Label - - def labelFormatOK(self, label): - return len(label) < 17 - - def labelingArgs(self, label): - return ["-L", label] - -class FATFSLabeling(FSLabeling): - - default_label = "NO NAME" - label_app = fslabel.DosFsLabel - - def labelFormatOK(self, label): - return len(label) < 12 - - def labelingArgs(self, label): - return ["-n", label] - -class JFSLabeling(FSLabeling): - - default_label = "" - label_app = fslabel.JFSTune - - def labelFormatOK(self, label): - return len(label) < 17 - - def labelingArgs(self, label): - return ["-L", label] - -class ReiserFSLabeling(FSLabeling): - - default_label = "" - label_app = fslabel.ReiserFSTune - - def labelFormatOK(self, label): - return len(label) < 17 - - def labelingArgs(self, label): - return ["-l", label] - -class XFSLabeling(FSLabeling): - - default_label = "" - label_app = fslabel.XFSAdmin - - def labelFormatOK(self, label): - return ' ' not in label and len(label) < 13 - - def labelingArgs(self, label): - return ["-L", label] - -class HFSLabeling(FSLabeling): - - default_label = "Untitled" - label_app = None - - def labelFormatOK(self, label): - return ':' not in label and len(label) < 28 and len(label) > 0 - - def labelingArgs(self, label): - return ["-l", label] - -class HFSPlusLabeling(FSLabeling): - - default_label = "Untitled" - label_app = None - - def labelFormatOK(self, label): - return ':' not in label and 0 < len(label) < 129 - - def labelingArgs(self, label): - return ["-v", label] - -class NTFSLabeling(FSLabeling): - - default_label = "" - label_app = fslabel.NTFSLabel - - def labelFormatOK(self, label): - return len(label) < 129 - - def labelingArgs(self, label): - return ["-L", label] diff --git a/blivet/tasks/fslabel.py b/blivet/tasks/fslabel.py new file mode 100644 index 0000000..81c18e6 --- /dev/null +++ b/blivet/tasks/fslabel.py @@ -0,0 +1,205 @@ +# fslabel.py +# Filesystem labeling classes for anaconda's storage configuration module. +# +# Copyright (C) 2013 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 +import re + +from six import add_metaclass + +from .. import errors + +@add_metaclass(abc.ABCMeta) +class FSLabelApp(object): + """An abstract class that represents actions associated with a + filesystem's labeling application. + """ + + name = abc.abstractproperty( + doc="The name of the filesystem labeling application.") + + reads = abc.abstractproperty( + doc="Whether this application can read a label as well as write one.") + + _label_regex = abc.abstractproperty( + doc="Matches the string output by the label reading application.") + + @abc.abstractmethod + def _writeLabelArgs(self, fs): + """Returns a list of the arguments for writing a label. + + :param FS fs: a filesystem object + + :return: the arguments + :rtype: list of str + + It can be assumed in this function that fs.label is a str. + """ + raise NotImplementedError + + def setLabelCommand(self, fs): + """Get the command to label the filesystem. + + :param FS fs: a filesystem object + :return: the command + :rtype: list of str + + Raises an exception if fs.label is None. + """ + if fs.label is None: + raise errors.FSError("makes no sense to write a label when accepting default label") + return [self.name] + self._writeLabelArgs(fs) + + @abc.abstractmethod + def _readLabelArgs(self, fs): + """Returns a list of arguments for reading a label. + + :param FS fs: a filesystem object + :return: the arguments + :rtype: list of str + """ + raise NotImplementedError + + def readLabelCommand(self, fs): + """Get the command to read the filesystem label. + + :param FS fs: a filesystem object + :return: the command + :rtype: list of str + + Raises an FSError if this application can not read the label. + """ + if not self.reads: + raise errors.FSError("Application %s can not read the filesystem label." % self.name) + return [self.name] + self._readLabelArgs(fs) + + def extractLabel(self, labelstr): + """Extract the label from an output string. + + :param str labelstr: the string containing the label information + + :return: the label + :rtype: str + + Raises an FSError if the label can not be extracted. + """ + if not self.reads or self._label_regex is None: + raise errors.FSError("Unknown format for application %s" % self.name) + match = re.match(self._label_regex, labelstr) + if match is None: + raise errors.FSError("Unknown format for application %s" % self.name) + return match.group('label') + + +class E2Label(FSLabelApp): + """Application used by ext2 and its descendants.""" + + name = "e2label" + reads = True + + _label_regex = r'(?P<label>.*)' + + def _writeLabelArgs(self, fs): + return [fs.device, fs.label] + + def _readLabelArgs(self, fs): + return [fs.device] + +E2Label = E2Label() + +class DosFsLabel(FSLabelApp): + """Application used by FATFS.""" + + name = "dosfslabel" + reads = True + + _label_regex = r'(?P<label>.*)' + + def _writeLabelArgs(self, fs): + return [fs.device, fs.label] + + def _readLabelArgs(self, fs): + return [fs.device] + +DosFsLabel = DosFsLabel() + +class JFSTune(FSLabelApp): + """Application used by JFS.""" + + name = "jfs_tune" + reads = False + + _label_regex = property(lambda s: None) + + def _writeLabelArgs(self, fs): + return ["-L", fs.label, fs.device] + + def _readLabelArgs(self, fs): + raise NotImplementedError + +JFSTune = JFSTune() + +class ReiserFSTune(FSLabelApp): + """Application used by ReiserFS.""" + + name = "reiserfstune" + reads = False + + _label_regex = None + + def _writeLabelArgs(self, fs): + return ["-l", fs.label, fs.device] + + def _readLabelArgs(self, fs): + raise NotImplementedError + +ReiserFSTune = ReiserFSTune() + +class XFSAdmin(FSLabelApp): + """Application used by XFS.""" + + name = "xfs_admin" + reads = True + + _label_regex = r'label = "(?P<label>.*)"' + + def _writeLabelArgs(self, fs): + return ["-L", fs.label if fs.label != "" else "--", fs.device] + + def _readLabelArgs(self, fs): + return ["-l", fs.device] + +XFSAdmin = XFSAdmin() + +class NTFSLabel(FSLabelApp): + """Application used by NTFS.""" + + name = "ntfslabel" + reads = True + + _label_regex = r'(?P<label>.*)' + + def _writeLabelArgs(self, fs): + return [fs.device, fs.label] + + def _readLabelArgs(self, fs): + return [fs.device] + +NTFSLabel = NTFSLabel() diff --git a/blivet/tasks/fslabeling.py b/blivet/tasks/fslabeling.py new file mode 100644 index 0000000..0abeaf7 --- /dev/null +++ b/blivet/tasks/fslabeling.py @@ -0,0 +1,148 @@ +# fslabeling.py +# Filesystem labeling classes for anaconda's storage configuration module. +# +# Copyright (C) 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 +# 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 fslabel + +@add_metaclass(abc.ABCMeta) +class FSLabeling(object): + """An abstract class that represents filesystem labeling actions. + """ + + default_label = abc.abstractproperty( + doc="Default label set on this filesystem at creation.") + + label_app = abc.abstractproperty( + doc="Post creation filesystem labeling application.") + + @abc.abstractmethod + def labelFormatOK(self, label): + """Returns True if this label is correctly formatted for this + filesystem, otherwise False. + + :param str label: the label for this filesystem + :rtype: bool + """ + raise NotImplementedError + + @abc.abstractmethod + def labelingArgs(self, label): + """Returns the arguments for writing the label during filesystem + creation. These arguments are intended to be passed to the + appropriate mkfs application. + + :param str label: the label to use + :return: the arguments + :rtype: list of str + """ + raise NotImplementedError + + +class Ext2FSLabeling(FSLabeling): + + default_label = "" + label_app = fslabel.E2Label + + def labelFormatOK(self, label): + return len(label) < 17 + + def labelingArgs(self, label): + return ["-L", label] + +class FATFSLabeling(FSLabeling): + + default_label = "NO NAME" + label_app = fslabel.DosFsLabel + + def labelFormatOK(self, label): + return len(label) < 12 + + def labelingArgs(self, label): + return ["-n", label] + +class JFSLabeling(FSLabeling): + + default_label = "" + label_app = fslabel.JFSTune + + def labelFormatOK(self, label): + return len(label) < 17 + + def labelingArgs(self, label): + return ["-L", label] + +class ReiserFSLabeling(FSLabeling): + + default_label = "" + label_app = fslabel.ReiserFSTune + + def labelFormatOK(self, label): + return len(label) < 17 + + def labelingArgs(self, label): + return ["-l", label] + +class XFSLabeling(FSLabeling): + + default_label = "" + label_app = fslabel.XFSAdmin + + def labelFormatOK(self, label): + return ' ' not in label and len(label) < 13 + + def labelingArgs(self, label): + return ["-L", label] + +class HFSLabeling(FSLabeling): + + default_label = "Untitled" + label_app = None + + def labelFormatOK(self, label): + return ':' not in label and len(label) < 28 and len(label) > 0 + + def labelingArgs(self, label): + return ["-l", label] + +class HFSPlusLabeling(FSLabeling): + + default_label = "Untitled" + label_app = None + + def labelFormatOK(self, label): + return ':' not in label and 0 < len(label) < 129 + + def labelingArgs(self, label): + return ["-v", label] + +class NTFSLabeling(FSLabeling): + + default_label = "" + label_app = fslabel.NTFSLabel + + def labelFormatOK(self, label): + return len(label) < 129 + + def labelingArgs(self, label): + return ["-L", label] diff --git a/setup.py b/setup.py index 4f9dd2c..bffcfc4 100644 --- a/setup.py +++ b/setup.py @@ -36,4 +36,4 @@ def add_member_order_option(files): author='David Lehman', author_email='dlehman@redhat.com', url='http://fedoraproject.org/wiki/blivet', data_files=data_files, - packages=['blivet', 'blivet.devices', 'blivet.devicelibs', 'blivet.formats']) + packages=['blivet', 'blivet.devices', 'blivet.devicelibs', 'blivet.formats', 'blivet.tasks']) diff --git a/tests/formats_test/fslabeling.py b/tests/formats_test/fslabeling.py deleted file mode 100644 index 9c6dabb..0000000 --- a/tests/formats_test/fslabeling.py +++ /dev/null @@ -1,160 +0,0 @@ -#!/usr/bin/python - -import abc -from six import add_metaclass - -from tests import loopbackedtestcase -from blivet.errors import FSError -from blivet.size import Size - -@add_metaclass(abc.ABCMeta) -class LabelingAsRoot(loopbackedtestcase.LoopBackedTestCase): - """Tests various aspects of labeling a filesystem where there - is no easy way to read the filesystem's label once it has been - set and where the filesystem can not be relabeled. - """ - - - _fs_class = abc.abstractproperty( - doc="The class of the filesystem being tested on.") - - _invalid_label = abc.abstractproperty( - doc="A label which is invalid for this filesystem.") - - def __init__(self, methodName='runTest'): - super(LabelingAsRoot, self).__init__(methodName=methodName, deviceSpec=[Size("100 MiB")]) - - def setUp(self): - an_fs = self._fs_class() - if not an_fs.utilsAvailable: - self.skipTest("utilities unavailable for filesystem %s" % an_fs.name) - super(LabelingAsRoot, self).setUp() - - def testLabeling(self): - """A sequence of tests of filesystem labeling. - - * create the filesystem when passing an invalid label - * raise an exception when reading the filesystem - * raise an exception when relabeling the filesystem - """ - an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) - self.assertIsNone(an_fs.create()) - - with self.assertRaisesRegexp(FSError, "no application to read label"): - an_fs.readLabel() - - an_fs.label = "an fs" - with self.assertRaisesRegexp(FSError, "no application to set label for filesystem"): - an_fs.writeLabel() - - def testCreating(self): - """Create the filesystem when passing a valid label """ - an_fs = self._fs_class(device=self.loopDevices[0], label="start") - self.assertIsNone(an_fs.create()) - - def testCreatingNone(self): - """Create the filesystem when passing None - (indicates filesystem default) - """ - an_fs = self._fs_class(device=self.loopDevices[0], label=None) - self.assertIsNone(an_fs.create()) - - def testCreatingEmpty(self): - """Create the filesystem when passing the empty label.""" - an_fs = self._fs_class(device=self.loopDevices[0], label="") - self.assertIsNone(an_fs.create()) - -class LabelingWithRelabeling(LabelingAsRoot): - """Tests labeling where it is possible to relabel. - """ - - def testLabeling(self): - """A sequence of tests of filesystem labeling. - - * create the filesystem when passing an invalid label - * raise an exception when reading the filesystem - * relabel the filesystem with a valid label - * relabel the filesystem with an empty label - * raise an exception when relabeling when None is specified - * raise an exception when relabeling with an invalid label - """ - an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) - self.assertIsNone(an_fs.create()) - - with self.assertRaisesRegexp(FSError, "no application to read label"): - an_fs.readLabel() - - an_fs.label = "an fs" - self.assertIsNone(an_fs.writeLabel()) - - an_fs.label = "" - self.assertIsNone(an_fs.writeLabel()) - - an_fs.label = None - with self.assertRaisesRegexp(FSError, "default label"): - an_fs.writeLabel() - - an_fs.label = self._invalid_label - with self.assertRaisesRegexp(FSError, "bad label format"): - an_fs.writeLabel() - -class CompleteLabelingAsRoot(LabelingAsRoot): - """Tests where it is possible to read the label and to relabel - an existing filesystem. - """ - - def testLabeling(self): - """A sequence of tests of filesystem labeling. - - * create the filesystem when passing an invalid label - and verify that the filesystem has the default label - * relabel the filesystem with a valid label - and verify that the filesystem has that label - * relabel the filesystem with an empty label - and verify that the filesystem has that label - * raise an exception when relabeling when None is specified - * raise an exception when relabeling with an invalid label - """ - an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) - self.assertIsNone(an_fs.create()) - self.assertEqual(an_fs.readLabel(), an_fs._labelfs.default_label) - - an_fs.label = "an_fs" - self.assertIsNone(an_fs.writeLabel()) - self.assertEqual(an_fs.readLabel(), an_fs.label) - - an_fs.label = "" - self.assertIsNone(an_fs.writeLabel()) - self.assertEqual(an_fs.readLabel(), an_fs.label) - - an_fs.label = None - with self.assertRaisesRegexp(FSError, "default label"): - an_fs.writeLabel() - - an_fs.label = "n" * 129 - with self.assertRaisesRegexp(FSError, "bad label format"): - an_fs.writeLabel() - - def testCreating(self): - """Create the filesystem when passing a valid label. - Verify that the filesystem has that label. - """ - an_fs = self._fs_class(device=self.loopDevices[0], label="start") - self.assertIsNone(an_fs.create()) - self.assertEqual(an_fs.readLabel(), "start") - - def testCreatingNone(self): - """Create a filesystem with the label None. - Verify that the filesystem has the default label. - """ - an_fs = self._fs_class(device=self.loopDevices[0], label=None) - self.assertIsNone(an_fs.create()) - self.assertEqual(an_fs.readLabel(), an_fs._labelfs.default_label) - - def testCreatingEmpty(self): - """Create a filesystem with an empty label. - Verify that the filesystem has the empty label. - """ - an_fs = self._fs_class(device=self.loopDevices[0], label="") - self.assertIsNone(an_fs.create()) - self.assertEqual(an_fs.readLabel(), "") diff --git a/tests/formats_test/labeling_test.py b/tests/formats_test/labeling_test.py deleted file mode 100755 index c1945d3..0000000 --- a/tests/formats_test/labeling_test.py +++ /dev/null @@ -1,142 +0,0 @@ -#!/usr/bin/python -import unittest - -from tests import loopbackedtestcase -from blivet.formats import device_formats -import blivet.formats.fs as fs -import blivet.formats.swap as swap - -from . import fslabeling - -class InitializationTestCase(unittest.TestCase): - """Test FS object initialization.""" - - def testLabels(self): - """Initialize some filesystems with valid and invalid labels.""" - - # Ext2FS has a maximum length of 16 - self.assertFalse(fs.Ext2FS().labelFormatOK("root___filesystem")) - self.assertTrue(fs.Ext2FS().labelFormatOK("root__filesystem")) - - # FATFS has a maximum length of 11 - self.assertFalse(fs.FATFS().labelFormatOK("rtfilesystem")) - self.assertTrue(fs.FATFS().labelFormatOK("rfilesystem")) - - # JFS has a maximum length of 16 - self.assertFalse(fs.JFS().labelFormatOK("root___filesystem")) - self.assertTrue(fs.JFS().labelFormatOK("root__filesystem")) - - # ReiserFS has a maximum length of 16 - self.assertFalse(fs.ReiserFS().labelFormatOK("root___filesystem")) - self.assertTrue(fs.ReiserFS().labelFormatOK("root__filesystem")) - - #XFS has a maximum length 12 and does not allow spaces - self.assertFalse(fs.XFS().labelFormatOK("root_filesyst")) - self.assertFalse(fs.XFS().labelFormatOK("root file")) - self.assertTrue(fs.XFS().labelFormatOK("root_filesys")) - - #HFS has a maximum length of 27, minimum length of 1, and does not allow colons - self.assertFalse(fs.HFS().labelFormatOK("n" * 28)) - self.assertFalse(fs.HFS().labelFormatOK("root:file")) - self.assertFalse(fs.HFS().labelFormatOK("")) - self.assertTrue(fs.HFS().labelFormatOK("n" * 27)) - - #HFSPlus has a maximum length of 128, minimum length of 1, and does not allow colons - self.assertFalse(fs.HFSPlus().labelFormatOK("n" * 129)) - self.assertFalse(fs.HFSPlus().labelFormatOK("root:file")) - self.assertFalse(fs.HFSPlus().labelFormatOK("")) - self.assertTrue(fs.HFSPlus().labelFormatOK("n" * 128)) - - # NTFS has a maximum length of 128 - self.assertFalse(fs.NTFS().labelFormatOK("n" * 129)) - self.assertTrue(fs.NTFS().labelFormatOK("n" * 128)) - - # all devices are permitted to be passed a label argument of None - # some will ignore it completely - for _k, v in device_formats.items(): - self.assertIsNotNone(v(label=None)) - -class MethodsTestCase(unittest.TestCase): - """Test some methods that do not require actual images.""" - - def setUp(self): - self.fs = {} - for k, v in device_formats.items(): - if issubclass(v, fs.FS) and v().labeling(): - self.fs[k] = v(device="/dev", label="myfs") - - - def testGetLabelArgs(self): - self.longMessage = True - - # ReiserFS uses a -l flag - reiserfs = self.fs["reiserfs"] - self.assertEqual(reiserfs._labelfs.label_app.setLabelCommand(reiserfs), - ["reiserfstune", "-l", "myfs", "/dev"], msg="reiserfs") - - # JFS, XFS use a -L flag - lflag_classes = [fs.JFS, fs.XFS] - for name, klass in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in lflag_classes)]: - self.assertEqual(klass._labelfs.label_app.setLabelCommand(v), [klass._labelfs.label_app.name, "-L", "myfs", "/dev"], msg=name) - - # Ext2FS and descendants and FATFS do not use a flag - noflag_classes = [fs.Ext2FS, fs.FATFS] - for name, klass in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in noflag_classes)]: - self.assertEqual(klass._labelfs.label_app.setLabelCommand(klass), [klass._labelfs.label_app.name, "/dev", "myfs"], msg=name) - - # all of the remaining are non-labeling so will accept any label - label = "Houston, we have a problem!" - for name, klass in device_formats.items(): - if issubclass(klass, fs.FS) and not klass().labeling() and not issubclass(klass, fs.NFS): - self.assertEqual(klass(device="/dev", label=label).label, label, msg=name) - -class XFSTestCase(fslabeling.CompleteLabelingAsRoot): - _fs_class = fs.XFS - _invalid_label = "root filesystem" - -class FATFSTestCase(fslabeling.CompleteLabelingAsRoot): - _fs_class = fs.FATFS - _invalid_label = "root___filesystem" - -class Ext2FSTestCase(fslabeling.CompleteLabelingAsRoot): - _fs_class = fs.Ext2FS - _invalid_label = "root___filesystem" - -class JFSTestCase(fslabeling.LabelingWithRelabeling): - _fs_class = fs.JFS - _invalid_label = "root___filesystem" - -class ReiserFSTestCase(fslabeling.LabelingWithRelabeling): - _fs_class = fs.ReiserFS - _invalid_label = "root___filesystem" - -class HFSTestCase(fslabeling.LabelingAsRoot): - _fs_class = fs.HFS - _invalid_label = "n" * 28 - -class HFSPlusTestCase(fslabeling.LabelingAsRoot): - _fs_class = fs.HFSPlus - _invalid_label = "n" * 129 - -@unittest.skip("Unable to create NTFS filesystem.") -class NTFSTestCase(fslabeling.CompleteLabelingAsRoot): - _fs_class = fs.NTFS - _invalid_label = "n" * 129 - -class LabelingSwapSpaceTestCase(loopbackedtestcase.LoopBackedTestCase): - - def testLabeling(self): - swp = swap.SwapSpace(device=self.loopDevices[0]) - swp.label = "mkswap is really pretty permissive about labels" - self.assertIsNone(swp.create()) - - def testCreatingSwapSpaceNone(self): - swp = swap.SwapSpace(device=self.loopDevices[0], label=None) - self.assertIsNone(swp.create()) - - def testCreatingSwapSpaceEmpty(self): - swp = swap.SwapSpace(device=self.loopDevices[0], label="") - self.assertIsNone(swp.create()) - -if __name__ == "__main__": - unittest.main() diff --git a/tests/tasks_test/__init__.py b/tests/tasks_test/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/tasks_test/fslabeling.py b/tests/tasks_test/fslabeling.py new file mode 100644 index 0000000..9c6dabb --- /dev/null +++ b/tests/tasks_test/fslabeling.py @@ -0,0 +1,160 @@ +#!/usr/bin/python + +import abc +from six import add_metaclass + +from tests import loopbackedtestcase +from blivet.errors import FSError +from blivet.size import Size + +@add_metaclass(abc.ABCMeta) +class LabelingAsRoot(loopbackedtestcase.LoopBackedTestCase): + """Tests various aspects of labeling a filesystem where there + is no easy way to read the filesystem's label once it has been + set and where the filesystem can not be relabeled. + """ + + + _fs_class = abc.abstractproperty( + doc="The class of the filesystem being tested on.") + + _invalid_label = abc.abstractproperty( + doc="A label which is invalid for this filesystem.") + + def __init__(self, methodName='runTest'): + super(LabelingAsRoot, self).__init__(methodName=methodName, deviceSpec=[Size("100 MiB")]) + + def setUp(self): + an_fs = self._fs_class() + if not an_fs.utilsAvailable: + self.skipTest("utilities unavailable for filesystem %s" % an_fs.name) + super(LabelingAsRoot, self).setUp() + + def testLabeling(self): + """A sequence of tests of filesystem labeling. + + * create the filesystem when passing an invalid label + * raise an exception when reading the filesystem + * raise an exception when relabeling the filesystem + """ + an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) + self.assertIsNone(an_fs.create()) + + with self.assertRaisesRegexp(FSError, "no application to read label"): + an_fs.readLabel() + + an_fs.label = "an fs" + with self.assertRaisesRegexp(FSError, "no application to set label for filesystem"): + an_fs.writeLabel() + + def testCreating(self): + """Create the filesystem when passing a valid label """ + an_fs = self._fs_class(device=self.loopDevices[0], label="start") + self.assertIsNone(an_fs.create()) + + def testCreatingNone(self): + """Create the filesystem when passing None + (indicates filesystem default) + """ + an_fs = self._fs_class(device=self.loopDevices[0], label=None) + self.assertIsNone(an_fs.create()) + + def testCreatingEmpty(self): + """Create the filesystem when passing the empty label.""" + an_fs = self._fs_class(device=self.loopDevices[0], label="") + self.assertIsNone(an_fs.create()) + +class LabelingWithRelabeling(LabelingAsRoot): + """Tests labeling where it is possible to relabel. + """ + + def testLabeling(self): + """A sequence of tests of filesystem labeling. + + * create the filesystem when passing an invalid label + * raise an exception when reading the filesystem + * relabel the filesystem with a valid label + * relabel the filesystem with an empty label + * raise an exception when relabeling when None is specified + * raise an exception when relabeling with an invalid label + """ + an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) + self.assertIsNone(an_fs.create()) + + with self.assertRaisesRegexp(FSError, "no application to read label"): + an_fs.readLabel() + + an_fs.label = "an fs" + self.assertIsNone(an_fs.writeLabel()) + + an_fs.label = "" + self.assertIsNone(an_fs.writeLabel()) + + an_fs.label = None + with self.assertRaisesRegexp(FSError, "default label"): + an_fs.writeLabel() + + an_fs.label = self._invalid_label + with self.assertRaisesRegexp(FSError, "bad label format"): + an_fs.writeLabel() + +class CompleteLabelingAsRoot(LabelingAsRoot): + """Tests where it is possible to read the label and to relabel + an existing filesystem. + """ + + def testLabeling(self): + """A sequence of tests of filesystem labeling. + + * create the filesystem when passing an invalid label + and verify that the filesystem has the default label + * relabel the filesystem with a valid label + and verify that the filesystem has that label + * relabel the filesystem with an empty label + and verify that the filesystem has that label + * raise an exception when relabeling when None is specified + * raise an exception when relabeling with an invalid label + """ + an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) + self.assertIsNone(an_fs.create()) + self.assertEqual(an_fs.readLabel(), an_fs._labelfs.default_label) + + an_fs.label = "an_fs" + self.assertIsNone(an_fs.writeLabel()) + self.assertEqual(an_fs.readLabel(), an_fs.label) + + an_fs.label = "" + self.assertIsNone(an_fs.writeLabel()) + self.assertEqual(an_fs.readLabel(), an_fs.label) + + an_fs.label = None + with self.assertRaisesRegexp(FSError, "default label"): + an_fs.writeLabel() + + an_fs.label = "n" * 129 + with self.assertRaisesRegexp(FSError, "bad label format"): + an_fs.writeLabel() + + def testCreating(self): + """Create the filesystem when passing a valid label. + Verify that the filesystem has that label. + """ + an_fs = self._fs_class(device=self.loopDevices[0], label="start") + self.assertIsNone(an_fs.create()) + self.assertEqual(an_fs.readLabel(), "start") + + def testCreatingNone(self): + """Create a filesystem with the label None. + Verify that the filesystem has the default label. + """ + an_fs = self._fs_class(device=self.loopDevices[0], label=None) + self.assertIsNone(an_fs.create()) + self.assertEqual(an_fs.readLabel(), an_fs._labelfs.default_label) + + def testCreatingEmpty(self): + """Create a filesystem with an empty label. + Verify that the filesystem has the empty label. + """ + an_fs = self._fs_class(device=self.loopDevices[0], label="") + self.assertIsNone(an_fs.create()) + self.assertEqual(an_fs.readLabel(), "") diff --git a/tests/tasks_test/labeling_test.py b/tests/tasks_test/labeling_test.py new file mode 100755 index 0000000..c1945d3 --- /dev/null +++ b/tests/tasks_test/labeling_test.py @@ -0,0 +1,142 @@ +#!/usr/bin/python +import unittest + +from tests import loopbackedtestcase +from blivet.formats import device_formats +import blivet.formats.fs as fs +import blivet.formats.swap as swap + +from . import fslabeling + +class InitializationTestCase(unittest.TestCase): + """Test FS object initialization.""" + + def testLabels(self): + """Initialize some filesystems with valid and invalid labels.""" + + # Ext2FS has a maximum length of 16 + self.assertFalse(fs.Ext2FS().labelFormatOK("root___filesystem")) + self.assertTrue(fs.Ext2FS().labelFormatOK("root__filesystem")) + + # FATFS has a maximum length of 11 + self.assertFalse(fs.FATFS().labelFormatOK("rtfilesystem")) + self.assertTrue(fs.FATFS().labelFormatOK("rfilesystem")) + + # JFS has a maximum length of 16 + self.assertFalse(fs.JFS().labelFormatOK("root___filesystem")) + self.assertTrue(fs.JFS().labelFormatOK("root__filesystem")) + + # ReiserFS has a maximum length of 16 + self.assertFalse(fs.ReiserFS().labelFormatOK("root___filesystem")) + self.assertTrue(fs.ReiserFS().labelFormatOK("root__filesystem")) + + #XFS has a maximum length 12 and does not allow spaces + self.assertFalse(fs.XFS().labelFormatOK("root_filesyst")) + self.assertFalse(fs.XFS().labelFormatOK("root file")) + self.assertTrue(fs.XFS().labelFormatOK("root_filesys")) + + #HFS has a maximum length of 27, minimum length of 1, and does not allow colons + self.assertFalse(fs.HFS().labelFormatOK("n" * 28)) + self.assertFalse(fs.HFS().labelFormatOK("root:file")) + self.assertFalse(fs.HFS().labelFormatOK("")) + self.assertTrue(fs.HFS().labelFormatOK("n" * 27)) + + #HFSPlus has a maximum length of 128, minimum length of 1, and does not allow colons + self.assertFalse(fs.HFSPlus().labelFormatOK("n" * 129)) + self.assertFalse(fs.HFSPlus().labelFormatOK("root:file")) + self.assertFalse(fs.HFSPlus().labelFormatOK("")) + self.assertTrue(fs.HFSPlus().labelFormatOK("n" * 128)) + + # NTFS has a maximum length of 128 + self.assertFalse(fs.NTFS().labelFormatOK("n" * 129)) + self.assertTrue(fs.NTFS().labelFormatOK("n" * 128)) + + # all devices are permitted to be passed a label argument of None + # some will ignore it completely + for _k, v in device_formats.items(): + self.assertIsNotNone(v(label=None)) + +class MethodsTestCase(unittest.TestCase): + """Test some methods that do not require actual images.""" + + def setUp(self): + self.fs = {} + for k, v in device_formats.items(): + if issubclass(v, fs.FS) and v().labeling(): + self.fs[k] = v(device="/dev", label="myfs") + + + def testGetLabelArgs(self): + self.longMessage = True + + # ReiserFS uses a -l flag + reiserfs = self.fs["reiserfs"] + self.assertEqual(reiserfs._labelfs.label_app.setLabelCommand(reiserfs), + ["reiserfstune", "-l", "myfs", "/dev"], msg="reiserfs") + + # JFS, XFS use a -L flag + lflag_classes = [fs.JFS, fs.XFS] + for name, klass in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in lflag_classes)]: + self.assertEqual(klass._labelfs.label_app.setLabelCommand(v), [klass._labelfs.label_app.name, "-L", "myfs", "/dev"], msg=name) + + # Ext2FS and descendants and FATFS do not use a flag + noflag_classes = [fs.Ext2FS, fs.FATFS] + for name, klass in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in noflag_classes)]: + self.assertEqual(klass._labelfs.label_app.setLabelCommand(klass), [klass._labelfs.label_app.name, "/dev", "myfs"], msg=name) + + # all of the remaining are non-labeling so will accept any label + label = "Houston, we have a problem!" + for name, klass in device_formats.items(): + if issubclass(klass, fs.FS) and not klass().labeling() and not issubclass(klass, fs.NFS): + self.assertEqual(klass(device="/dev", label=label).label, label, msg=name) + +class XFSTestCase(fslabeling.CompleteLabelingAsRoot): + _fs_class = fs.XFS + _invalid_label = "root filesystem" + +class FATFSTestCase(fslabeling.CompleteLabelingAsRoot): + _fs_class = fs.FATFS + _invalid_label = "root___filesystem" + +class Ext2FSTestCase(fslabeling.CompleteLabelingAsRoot): + _fs_class = fs.Ext2FS + _invalid_label = "root___filesystem" + +class JFSTestCase(fslabeling.LabelingWithRelabeling): + _fs_class = fs.JFS + _invalid_label = "root___filesystem" + +class ReiserFSTestCase(fslabeling.LabelingWithRelabeling): + _fs_class = fs.ReiserFS + _invalid_label = "root___filesystem" + +class HFSTestCase(fslabeling.LabelingAsRoot): + _fs_class = fs.HFS + _invalid_label = "n" * 28 + +class HFSPlusTestCase(fslabeling.LabelingAsRoot): + _fs_class = fs.HFSPlus + _invalid_label = "n" * 129 + +@unittest.skip("Unable to create NTFS filesystem.") +class NTFSTestCase(fslabeling.CompleteLabelingAsRoot): + _fs_class = fs.NTFS + _invalid_label = "n" * 129 + +class LabelingSwapSpaceTestCase(loopbackedtestcase.LoopBackedTestCase): + + def testLabeling(self): + swp = swap.SwapSpace(device=self.loopDevices[0]) + swp.label = "mkswap is really pretty permissive about labels" + self.assertIsNone(swp.create()) + + def testCreatingSwapSpaceNone(self): + swp = swap.SwapSpace(device=self.loopDevices[0], label=None) + self.assertIsNone(swp.create()) + + def testCreatingSwapSpaceEmpty(self): + swp = swap.SwapSpace(device=self.loopDevices[0], label="") + self.assertIsNone(swp.create()) + +if __name__ == "__main__": + unittest.main()
Am I missing some commit somewhere or are things in *fslabel.py* and *fslabeling.py* not tasks (i.e. inherited from the ``Task`` class) yet at this point? When they become so or why they are refered to as 'tasks'?
From: mulhern amulhern@redhat.com
After a few more patches they will be required to access instance variables.
Does not affect downstream, which always uses instance when invoking, anyway.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/__init__.py | 8 +++---- blivet/formats/fs.py | 15 +++++------- blivet/formats/swap.py | 6 ++--- tests/formats_test/labeling_test.py | 46 ++++++++++++++++++------------------- 4 files changed, 34 insertions(+), 41 deletions(-)
diff --git a/blivet/formats/__init__.py b/blivet/formats/__init__.py index 4bd9105..9582d4a 100644 --- a/blivet/formats/__init__.py +++ b/blivet/formats/__init__.py @@ -221,13 +221,11 @@ def dict(self): "resizable": self.resizable} return d
- @classmethod - def labeling(cls): + def labeling(self): """Returns False by default since most formats are non-labeling.""" return False
- @classmethod - def labelFormatOK(cls, label): + def labelFormatOK(self, label): """Checks whether the format of the label is OK for whatever application is used by blivet to write a label for this format. If there is no application that blivet uses to write a label, @@ -239,7 +237,7 @@ def labelFormatOK(cls, label): :return: True if the format of the label is OK, otherwise False """ # pylint: disable=unused-argument - return cls.labeling() + return self.labeling()
def _setLabel(self, label): """Sets the label for this format. diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 4e3d39f..3d7cae4 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -151,25 +151,22 @@ def dict(self): "mountable": self.mountable}) return d
- @classmethod - def labeling(cls): + def labeling(self): """Returns True if this filesystem uses labels, otherwise False.
:rtype: bool """ - return cls._labelfs is not None + return self._labelfs is not None
- @classmethod - def relabels(cls): + def relabels(self): """Returns True if it is possible to relabel this filesystem after creation, otherwise False.
:rtype: bool """ - return cls._labelfs is not None and cls._labelfs.label_app is not None + return self._labelfs is not None and self._labelfs.label_app is not None
- @classmethod - def labelFormatOK(cls, label): + def labelFormatOK(self, label): """Return True if the label has an acceptable format for this filesystem. None, which represents accepting the default for this device, is always acceptable. @@ -177,7 +174,7 @@ def labelFormatOK(cls, label): :param label: A possible label :type label: str or None """ - return label is None or (cls._labelfs is not None and cls._labelfs.labelFormatOK(label)) + return label is None or (self._labelfs is not None and self._labelfs.labelFormatOK(label))
label = property(lambda s: s._getLabel(), lambda s,l: s._setLabel(l), doc="this filesystem's label") diff --git a/blivet/formats/swap.py b/blivet/formats/swap.py index e355505..ccd2797 100644 --- a/blivet/formats/swap.py +++ b/blivet/formats/swap.py @@ -81,13 +81,11 @@ def dict(self): d.update({"priority": self.priority, "label": self.label}) return d
- @classmethod - def labeling(cls): + def labeling(self): """Returns True as mkswap can write a label to the swap space.""" return True
- @classmethod - def labelFormatOK(cls, label): + def labelFormatOK(self, label): """Returns True since no known restrictions on the label.""" return True
diff --git a/tests/formats_test/labeling_test.py b/tests/formats_test/labeling_test.py index 25206b1..c1945d3 100755 --- a/tests/formats_test/labeling_test.py +++ b/tests/formats_test/labeling_test.py @@ -15,41 +15,41 @@ def testLabels(self): """Initialize some filesystems with valid and invalid labels."""
# Ext2FS has a maximum length of 16 - self.assertFalse(fs.Ext2FS.labelFormatOK("root___filesystem")) - self.assertTrue(fs.Ext2FS.labelFormatOK("root__filesystem")) + self.assertFalse(fs.Ext2FS().labelFormatOK("root___filesystem")) + self.assertTrue(fs.Ext2FS().labelFormatOK("root__filesystem"))
# FATFS has a maximum length of 11 - self.assertFalse(fs.FATFS.labelFormatOK("rtfilesystem")) - self.assertTrue(fs.FATFS.labelFormatOK("rfilesystem")) + self.assertFalse(fs.FATFS().labelFormatOK("rtfilesystem")) + self.assertTrue(fs.FATFS().labelFormatOK("rfilesystem"))
# JFS has a maximum length of 16 - self.assertFalse(fs.JFS.labelFormatOK("root___filesystem")) - self.assertTrue(fs.JFS.labelFormatOK("root__filesystem")) + self.assertFalse(fs.JFS().labelFormatOK("root___filesystem")) + self.assertTrue(fs.JFS().labelFormatOK("root__filesystem"))
# ReiserFS has a maximum length of 16 - self.assertFalse(fs.ReiserFS.labelFormatOK("root___filesystem")) - self.assertTrue(fs.ReiserFS.labelFormatOK("root__filesystem")) + self.assertFalse(fs.ReiserFS().labelFormatOK("root___filesystem")) + self.assertTrue(fs.ReiserFS().labelFormatOK("root__filesystem"))
#XFS has a maximum length 12 and does not allow spaces - self.assertFalse(fs.XFS.labelFormatOK("root_filesyst")) - self.assertFalse(fs.XFS.labelFormatOK("root file")) - self.assertTrue(fs.XFS.labelFormatOK("root_filesys")) + self.assertFalse(fs.XFS().labelFormatOK("root_filesyst")) + self.assertFalse(fs.XFS().labelFormatOK("root file")) + self.assertTrue(fs.XFS().labelFormatOK("root_filesys"))
#HFS has a maximum length of 27, minimum length of 1, and does not allow colons - self.assertFalse(fs.HFS.labelFormatOK("n" * 28)) - self.assertFalse(fs.HFS.labelFormatOK("root:file")) - self.assertFalse(fs.HFS.labelFormatOK("")) - self.assertTrue(fs.HFS.labelFormatOK("n" * 27)) + self.assertFalse(fs.HFS().labelFormatOK("n" * 28)) + self.assertFalse(fs.HFS().labelFormatOK("root:file")) + self.assertFalse(fs.HFS().labelFormatOK("")) + self.assertTrue(fs.HFS().labelFormatOK("n" * 27))
#HFSPlus has a maximum length of 128, minimum length of 1, and does not allow colons - self.assertFalse(fs.HFSPlus.labelFormatOK("n" * 129)) - self.assertFalse(fs.HFSPlus.labelFormatOK("root:file")) - self.assertFalse(fs.HFSPlus.labelFormatOK("")) - self.assertTrue(fs.HFSPlus.labelFormatOK("n" * 128)) + self.assertFalse(fs.HFSPlus().labelFormatOK("n" * 129)) + self.assertFalse(fs.HFSPlus().labelFormatOK("root:file")) + self.assertFalse(fs.HFSPlus().labelFormatOK("")) + self.assertTrue(fs.HFSPlus().labelFormatOK("n" * 128))
# NTFS has a maximum length of 128 - self.assertFalse(fs.NTFS.labelFormatOK("n" * 129)) - self.assertTrue(fs.NTFS.labelFormatOK("n" * 128)) + self.assertFalse(fs.NTFS().labelFormatOK("n" * 129)) + self.assertTrue(fs.NTFS().labelFormatOK("n" * 128))
# all devices are permitted to be passed a label argument of None # some will ignore it completely @@ -62,7 +62,7 @@ class MethodsTestCase(unittest.TestCase): def setUp(self): self.fs = {} for k, v in device_formats.items(): - if issubclass(v, fs.FS) and v.labeling(): + if issubclass(v, fs.FS) and v().labeling(): self.fs[k] = v(device="/dev", label="myfs")
@@ -87,7 +87,7 @@ def testGetLabelArgs(self): # all of the remaining are non-labeling so will accept any label label = "Houston, we have a problem!" for name, klass in device_formats.items(): - if issubclass(klass, fs.FS) and not klass.labeling() and not issubclass(klass, fs.NFS): + if issubclass(klass, fs.FS) and not klass().labeling() and not issubclass(klass, fs.NFS): self.assertEqual(klass(device="/dev", label=label).label, label, msg=name)
class XFSTestCase(fslabeling.CompleteLabelingAsRoot):
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fswritelabel.py | 149 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 blivet/tasks/fswritelabel.py
diff --git a/blivet/tasks/fswritelabel.py b/blivet/tasks/fswritelabel.py new file mode 100644 index 0000000..74c9653 --- /dev/null +++ b/blivet/tasks/fswritelabel.py @@ -0,0 +1,149 @@ +# fswritelabel.py +# Filesystem label writing classes. +# +# 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 +import os + +from six import add_metaclass + +from .. import util +from ..errors import FSWriteLabelError + +from . import availability +from . import task + +@add_metaclass(abc.ABCMeta) +class FSWriteLabel(task.Task): + """ An abstract class that represents writing a label for a filesystem. """ + + description = "write filesystem label" + + app_name = abc.abstractproperty( + doc="The name of the filesystem labeling application.") + + args = abc.abstractproperty(doc="arguments for writing a label") + + @classmethod + def _app(cls): + return availability.Application(availability.Path(), cls.app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + # TASK methods + + @classmethod + def available(cls): + return cls._app().available + + @property + def unavailable(self): + if not self._app().available: + return "application %s is not available" % self._app() + + return False + + @property + def unready(self): + if not self.fs.exists: + return "filesystem has not been created" + + if not os.path.exists(self.fs.device): + return "device (%s) does not exist" % self.fs.device.name + + return False + + @property + def unable(self): + if self.fs.label is None: + return "makes no sense to write a label when accepting default label" + + if not self.fs.labelFormatOK(self.fs.label): + return "bad label format for labelling application %s" % self._app() + + return False + + # IMPLEMENTATION methods + + def _setCommand(self): + """Get the command to label the filesystem. + + :return: the command + :rtype: list of str + + Requires that self.unavailable, self.unable, self.unready are False. + """ + return [str(self._app())] + self.args + + def doTask(self): + error_msg = self.impossible + if error_msg: + raise FSWriteLabelError(error_msg) + + rc = util.run_program(self._setCommand()) + if rc: + raise FSWriteLabelError("label failed") + +class DosFSWriteLabel(FSWriteLabel): + app_name = "dosfslabel" + + @property + def args(self): + return [self.fs.device, self.fs.label] + +class Ext2FSWriteLabel(FSWriteLabel): + app_name = "e2label" + + @property + def args(self): + return [self.fs.device, self.fs.label] + +class JFSWriteLabel(FSWriteLabel): + app_name = "jfs_tune" + + @property + def args(self): + return ["-L", self.fs.label, self.fs.device] + +class NTFSWriteLabel(FSWriteLabel): + app_name = "ntfslabel" + + @property + def args(self): + return [self.fs.device, self.fs.label] + +class ReiserFSWriteLabel(FSWriteLabel): + app_name = "reiserfstune" + + @property + def args(self): + return ["-l", self.fs.label, self.fs.device] + +class XFSWriteLabel(FSWriteLabel): + app_name = "xfs_admin" + + @property + def args(self): + return ["-L", self.fs.label if self.fs.label != "" else "--", self.fs.device]
In reply to line 87 of blivet/tasks/fswritelabel.py:
I really think ``available``, ``ready`` (instead of ``unable``) and perhaps ``supported`` (instead of ``unready``) returning (named)tuples (True, None) or (False, err_msg) would be much better readable and understandable.
In reply to line 98 of blivet/tasks/fswritelabel.py:
This should be a property.
In reply to line 103 of blivet/tasks/fswritelabel.py:
If I remember that correctly, ``impossible`` would only return the first error message if there are multiple problems (by using ``or`` on all of them). It would be nice if we got the full info in the exeception, not just the first error.
In reply to line 114 of blivet/tasks/fswritelabel.py:
Why is this ``@property`` and not just a simple assignment ``args = [self.fs.device, self.fs.label]``? And the same applies below.
In reply to line 98 of blivet/tasks/fswritelabel.py:
_setCommand() is a property now.
In reply to line 114 of blivet/tasks/fswritelabel.py:
There is no self.fs until the FSWriteLabel object is constructed. Evaluating those expressions when the class is loaded is way too early.
In reply to line 103 of blivet/tasks/fswritelabel.py:
I don't expect that computing all the error messages would ever prove to be an advantage. But if it turns out that I am wrong...that impossible() method is easily altered to do just that.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsreadlabel.py | 158 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 blivet/tasks/fsreadlabel.py
diff --git a/blivet/tasks/fsreadlabel.py b/blivet/tasks/fsreadlabel.py new file mode 100644 index 0000000..9a485a1 --- /dev/null +++ b/blivet/tasks/fsreadlabel.py @@ -0,0 +1,158 @@ +# fsreadlabel.py +# Filesystem label reading classes. +# +# 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 +import os +import re + +from six import add_metaclass + +from ..errors import FSReadLabelError +from .. import util + +from . import availability +from . import task + +@add_metaclass(abc.ABCMeta) +class FSReadLabel(task.Task): + """ An abstract class that represents reading a filesystem's label. """ + description = "read filesystem label" + + app_name = abc.abstractproperty( + doc="The name of the filesystem labeling application.") + + label_regex = abc.abstractproperty( + doc="Matches the string output by the reading application.") + + args = abc.abstractproperty(doc="arguments for reading a label.") + + @classmethod + def _app(cls): + return availability.Application(availability.Path(), cls.app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + # TASK methods + + @classmethod + def available(cls): + return cls._app().available + + @property + def unavailable(self): + if not self._app().available: + return "application %s is not available" % self._app() + + return False + + @property + def unready(self): + if not self.fs.exists: + return "filesystem has not been created" + + if not os.path.exists(self.fs.device): + return "device %s does not exist" % self.fs.device.name + + return False + + @property + def unable(self): + return False + + # IMPLEMENTATION methods + + def _readCommand(self): + """Get the command to read the filesystem label. + + :return: the command + :rtype: list of str + """ + return [str(self._app())] + self.args + + def _extractLabel(self, labelstr): + """Extract the label from an output string. + + :param str labelstr: the string containing the label information + + :return: the label + :rtype: str + + Raises an FSReadLabelError if the label can not be extracted. + """ + match = re.match(self.label_regex, labelstr) + if match is None: + raise FSReadLabelError("Unknown format for application %s" % self._app()) + return match.group('label') + + def doTask(self): + """ Get the label. + + :returns: the filesystem label + :rtype: str + """ + error_msg = self.impossible + if error_msg: + raise FSReadLabelError(error_msg) + + (rc, out) = util.run_program_and_capture_output(self._readCommand()) + if rc: + raise FSReadLabelError("read label failed") + + label = out.strip() + + return label if label == "" else self._extractLabel(label) + +class DosFSReadLabel(FSReadLabel): + app_name = "dosfslabel" + label_regex = r'(?P<label>.*)' + + @property + def args(self): + return [self.fs.device] + +class Ext2FSReadLabel(FSReadLabel): + app_name = "e2label" + label_regex = r'(?P<label>.*)' + + @property + def args(self): + return [self.fs.device] + +class NTFSReadLabel(FSReadLabel): + app_name = "ntfslabel" + label_regex = r'(?P<label>.*)' + + @property + def args(self): + return [self.fs.device] + +class XFSReadLabel(FSReadLabel): + app_name = "xfs_admin" + label_regex = r'label = "(?P<label>.*)"' + + @property + def args(self): + return ["-l", self.fs.device]
In reply to line 93 of blivet/tasks/fsreadlabel.py:
Again, this should be a property.
In reply to line 121 of blivet/tasks/fsreadlabel.py:
``rc != 0`` would be more explicit and clearer here, I think. I know ``bash`` is the evil, but it has ruined these checks for me forever.
In reply to line 93 of blivet/tasks/fsreadlabel.py:
it is now.
In reply to line 121 of blivet/tasks/fsreadlabel.py:
done.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsck.py | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 blivet/tasks/fsck.py
diff --git a/blivet/tasks/fsck.py b/blivet/tasks/fsck.py new file mode 100644 index 0000000..49aadae --- /dev/null +++ b/blivet/tasks/fsck.py @@ -0,0 +1,176 @@ +# fsck.py +# Filesystem check functionality. +# +# 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 +import os + +from six import add_metaclass + +from ..i18n import _, N_ + +from ..errors import FSError +from .. import util + +from . import availability +from . import task + +_UNKNOWN_RC_MSG = N_("Unknown return code: %d") + +@add_metaclass(abc.ABCMeta) +class FSCK(task.Task): + """An abstract class that represents actions associated with + checking consistency of a filesystem. + """ + description = "fsck" + + app_name = abc.abstractproperty( + doc="The name of the filesystem labeling application.") + + options = abc.abstractproperty( + doc="Options for invoking the application.") + + @classmethod + def _app(cls): + return availability.Application(availability.Path(), cls.app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + # TASK methods + + @classmethod + def available(cls): + return cls._app().available + + @property + def unavailable(self): + if not self._app().available: + return "application %s is not available" % self._app() + + return False + + @property + def unready(self): + if not self.fs.exists: + return "filesystem has not been created" + + if not os.path.exists(self.fs.device): + return "device %s does not exist" % self.fs.device.name + + return False + + @property + def unable(self): + return False + + # IMPLEMENTATION methods + + @abc.abstractmethod + def _errorMessage(self, rc): + """ Error message corresponding to rc. + + :param int rc: the fsck program return code + :returns: an error message corresponding to the code, or None + :rtype: str or NoneType + + A return value of None indicates no error. + """ + raise NotImplementedError() + + def _fsckCommand(self): + """The command to check the filesystem. + + :return: the command + :rtype: list of str + """ + return [str(self._app())] + self.options + [self.fs.device] + + def doTask(self): + """ Check the filesystem. + + :raises FSError: on failure + """ + error_msg = self.impossible + if error_msg: + raise FSError(error_msg) + + try: + rc = util.run_program(self._fsckCommand()) + except OSError as e: + raise FSError("filesystem check failed: %s" % e) + + error_msg = self._errorMessage(rc) + if error_msg is not None: + hdr = _("%(type)s filesystem check failure on %(device)s: ") % \ + {"type": self.fs.type, "device": self.fs.device} + + raise FSError(hdr + error_msg) + + +class DosFSCK(FSCK): + _fsckErrors = {1: N_("Recoverable errors have been detected or dosfsck has " + "discovered an internal inconsistency."), + 2: N_("Usage error.")} + + app_name = "dosfsck" + options = ["-n"] + + def _errorMessage(self, rc): + if rc < 1: + return None + try: + return _(self._fsckErrors[rc]) + except KeyError: + return _UNKNOWN_RC_MSG % rc + + +class Ext2FSCK(FSCK): + _fsckErrors = {4: N_("File system errors left uncorrected."), + 8: N_("Operational error."), + 16: N_("Usage or syntax error."), + 32: N_("e2fsck cancelled by user request."), + 128: N_("Shared library error.")} + + app_name = "e2fsck" + options = ["-f", "-p", "-C", "0"] + + def _errorMessage(self, rc): + msgs = (_(self._fsckErrors[c]) for c in self._fsckErrors.keys() if rc & c) + return "\n".join(msgs).strip() or None + +class HFSPlusFSCK(FSCK): + app_name = "fsck.hfsplus" + options = [] + + def _errorMessage(self, rc): + # pylint: disable=unused-argument + return None + +class NTFSFSCK(FSCK): + app_name = "ntfsresize" + options = ["-c"] + + def _errorMessage(self, rc): + return _UNKNOWN_RC_MSG % (rc,) if rc != 0 else None
In reply to line 108 of blivet/tasks/fsck.py:
Another candidate for property.
In reply to line 108 of blivet/tasks/fsck.py:
done.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsinfo.py | 127 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 blivet/tasks/fsinfo.py
diff --git a/blivet/tasks/fsinfo.py b/blivet/tasks/fsinfo.py new file mode 100644 index 0000000..2ae0cd7 --- /dev/null +++ b/blivet/tasks/fsinfo.py @@ -0,0 +1,127 @@ +# fsinfo.py +# Filesystem information gathering classes. +# +# 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 +import os + +from six import add_metaclass + +from ..errors import FSError +from .. import util + +from . import availability +from . import task + +@add_metaclass(abc.ABCMeta) +class FSInfo(task.Task): + """ An abstract class that represents an information gathering app. """ + + description = "filesystem info" + + app_name = abc.abstractproperty( + doc="The name of the filesystem information gathering application.") + + options = abc.abstractproperty( + doc="Options for invoking the application.") + + @classmethod + def _app(cls): + return availability.Application(availability.Path(), cls.app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + @classmethod + def available(cls): + return cls._app().available + + @property + def unavailable(self): + if not self._app().available: + return "application %s not available." % self._app() + + return False + + @property + def unready(self): + if not self.fs.exists: + return "filesystem has not been created" + + if not os.path.exists(self.fs.device): + return "device does not exist" + + return False + + @property + def unable(self): + return False + + def _infoCommand(self): + """ Returns the command for reading filesystem information. + + :returns: a list of appropriate options + :rtype: list of str + """ + return [str(self._app())] + self.options + [self.fs.device] + + def doTask(self): + """ Returns information from the command. + + :returns: a string representing the output of the command + :rtype: str + :raises FSError: if info cannot be obtained + """ + error_msg = self.impossible + if error_msg: + raise FSError(error_msg) + try: + (rc, out) = util.run_program_and_capture_output(self._infoCommand()) + if rc: + error_msg = "failed to gather fs info: %s" % rc + except OSError as e: + error_msg = "failed to gather fs info: %s" % e + if error_msg: + raise FSError(error_msg) + return out + +class Ext2FSInfo(FSInfo): + app_name = "dumpe2fs" + options = ["-h"] + +class JFSInfo(FSInfo): + app_name = "jfs_tune" + options = ["-l"] + +class NTFSInfo(FSInfo): + app_name = "ntfsinfo" + options = ["-m"] + +class ReiserFSInfo(FSInfo): + app_name = "debugreiserfs" + options = [] + +class XFSInfo(FSInfo): + app_name = "xfs_db" + options = ["-c", "sb 0", "-c", "p dblocks", "-c", "p blocksize"]
In reply to line 87 of blivet/tasks/fsinfo.py:
another property
In reply to line 87 of blivet/tasks/fsinfo.py:
done.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fssize.py | 183 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 blivet/tasks/fssize.py
diff --git a/blivet/tasks/fssize.py b/blivet/tasks/fssize.py new file mode 100644 index 0000000..930fd9d --- /dev/null +++ b/blivet/tasks/fssize.py @@ -0,0 +1,183 @@ +# fssize.py +# Filesystem size gathering classes. +# +# 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 collections import namedtuple + +from six import add_metaclass + +from ..errors import FSError +from ..size import Size +from .. import util + +from . import availability +from . import task + +_tags = ("count", "size") +_Tags = namedtuple("_Tags", _tags) + +@add_metaclass(abc.ABCMeta) +class FSSize(task.Task): + """ An abstract class that represents size information extraction. """ + description = "current filesystem size" + + tags = abc.abstractproperty( + doc="String 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 + + @classmethod + def available(cls): + return True + + @property + def unavailable(self): + return False + + @property + def unready(self): + return False + + @property + def unable(self): + if self.fs._current_info is None: + return "No filesystem info available to extract current size from." + return False + + # IMPLEMENTATION methods + + def doTask(self): + """ Returns the size of the filesystem. + + :returns: the size of the filesystem or None + :rtype: :class:`~.size.Size` or NoneType + :raises FSError: on failure + """ + error_msg = self.impossible + if error_msg: + raise FSError(error_msg) + + # Setup initial values + values = {} + for k in _tags: + values[k] = None + + # Attempt to set values from info + for line in (l.strip() for l in self.fs._current_info.splitlines()): + key = next((k for k in _tags if line.startswith(getattr(self.tags, k))), None) + if not key: + continue + + if values[key] is not None: + raise FSError("found two matches for key %s" % key) + + # Look for last numeric value in matching line + fields = line.split() + fields.reverse() + for field in fields: + try: + values[key] = int(field) + break + except ValueError: + continue + + # Raise an error if a value is missing + missing = next((k for k in _tags if values[k] is None), None) + if missing is not None: + raise FSError("Failed to parse info for %s." % missing) + + return values["count"] * Size(values["size"]) + +class Ext2FSSize(FSSize): + tags = _Tags(size="Block size:", count="Block count:") + +class JFSSize(FSSize): + tags = _Tags(size="Physical block size:", count="Aggregate size:") + +class NTFSSize(FSSize): + tags = _Tags(size="Cluster Size:", count="Volume Size in Clusters:") + +class ReiserFSSize(FSSize): + tags = _Tags(size="Blocksize:", count="Count of blocks on the device:") + +class XFSSize(FSSize): + tags = _Tags(size="blocksize =", count="dblocks =") + +class TmpFSSize(task.Task): + description = "current filesystem size" + + app_name = "df" + _app = availability.Application(availability.Path(), app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + @classmethod + def available(cls): + return cls._app.available + + @property + def unavailable(self): + if not self._app.available: + return "application %s is not available" % self._app + return False + + @property + def unready(self): + if not self.fs.status: + return "filesystem is not mounted" + return False + + @property + def unable(self): + return False + + def _sizeCommand(self): + return [str(self._app), self.fs.systemMountpoint, "--output=size"] + + def doTask(self): + error_msg = self.impossible + if error_msg: + raise FSError(error_msg) + + try: + (ret, out) = util.run_program_and_capture_output(self._sizeCommand()) + if ret: + raise FSError("Failed to execute command %s." % self._sizeCommand()) + except OSError: + raise FSError("Failed to execute command %s." % self._sizeCommand()) + + lines = out.split() + if len(lines) != 2 or lines[0] != "1K-blocks": + raise FSError("Failed to parse output of command %s." % self._sizeCommand()) + + return Size("%s KiB" % lines[1])
In reply to line 43 of blivet/tasks/fssize.py:
``tags`` is not supposed to be a string
In reply to line 110 of blivet/tasks/fssize.py:
I think you could use ``any`` here which could make the line better readable.
In reply to line 165 of blivet/tasks/fssize.py:
Another property being a method.
In reply to line 179 of blivet/tasks/fssize.py:
``out.splitlines()`` maybe?
In reply to line 110 of blivet/tasks/fssize.py:
I use the actual value of "missing" in the next line.
In reply to line 43 of blivet/tasks/fssize.py:
"String" is now "Strings".
In reply to line 165 of blivet/tasks/fssize.py:
done.
In reply to line 179 of blivet/tasks/fssize.py:
done.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsmkfs.py | 249 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 blivet/tasks/fsmkfs.py
diff --git a/blivet/tasks/fsmkfs.py b/blivet/tasks/fsmkfs.py new file mode 100644 index 0000000..d6dfe6f --- /dev/null +++ b/blivet/tasks/fsmkfs.py @@ -0,0 +1,249 @@ +# fsmkfs.py +# Filesystem formatting classes. +# +# 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 +import os + +from six import add_metaclass + +from ..errors import FSError, FSWriteLabelError +from .. import util + +from . import availability +from . import task + +@add_metaclass(abc.ABCMeta) +class FSMkfs(task.Task): + """An abstract class that represents filesystem creation actions. """ + description = "mkfs" + + app_name = abc.abstractproperty( + doc="Name of the filesystem creation application.") + + label_option = abc.abstractproperty( + doc="Option for setting a filesystem label.") + + args = abc.abstractproperty(doc="options for creating filesystem") + + @classmethod + def _app(cls): + return availability.Application(availability.Path(), cls.app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + # TASK methods + + @classmethod + def available(cls): + return cls._app().available + + @property + def unavailable(self): + if not self._app().available: + return "Filesystem formatting application %s is unavailable." % self._app() + return False + + @property + def unready(self): + if self.fs.exists: + return "filesystem already exists" + + if not os.path.exists(self.fs.device): + return "device does not exist" + + return False + + @property + def unable(self): + return False + + # IMPLEMENTATION methods + + @property + def labels(self): + """ Whether this task can label the filesystem. + + :returns: True if this task can label the filesystem + :rtype: bool + """ + return self.label_option is not None + + def _labelOptions(self, label=False): + """ Any labeling options that a particular filesystem may use. + + :param bool label: if True, label if possible, default is False + :returns: labeling options, may be an empty list + :rtype: list of str + """ + if label is False: + return [] + + # Do not know how to set label while formatting. + if self.label_option is None: + return [] + + # No label to set + if self.fs.label is None: + return [] + + if self.fs.labelFormatOK(self.fs.label): + return [self.label_option, self.fs.label] + else: + raise FSWriteLabelError("Choosing not to apply label (%s) during creation of filesystem %s. Label format is unacceptable for this filesystem." % (self.fs.label, self.fs.type)) + + def _formatOptions(self, options=None, label=False): + """Get a list of format options to be used when creating the + filesystem. + + :param options: any special options + :param bool label: if True, label if possible, default is False + :type options: list of str or None + """ + options = options or [] + + if not isinstance(options, list): + raise FSError("options parameter must be a list.") + + return options + self.args + self._labelOptions(label) + [self.fs.device] + + def _mkfsCommand(self, options, label): + """Return the command to make the filesystem. + + :param options: any special options + :type options: list of str or None + :returns: the mkfs command + :rtype: list of str + """ + return [str(self._app())] + self._formatOptions(options, label) + + def doTask(self, options=None, label=False): + """Create the format on the device and label if possible and desired. + + :param options: any special options, may be None + :type options: list of str or NoneType + :param bool label: whether to label while creating, default is False + """ + # pylint: disable=arguments-differ + error_msg = self.impossible + if error_msg: + raise FSError(error_msg) + + options = options or [] + try: + ret = util.run_program(self._mkfsCommand(options, label)) + except OSError as e: + raise FSError(e) + + if ret: + raise FSError("format failed: %s" % ret) + +class BTRFSMkfs(FSMkfs): + app_name = "mkfs.btrfs" + label_option = None + + @property + def args(self): + return [] + +class Ext2FSMkfs(FSMkfs): + app_name = "mke2fs" + label_option = "-L" + + _opts = [] + + @property + def args(self): + return self._opts + (["-T", self.fs.fsprofile] if self.fs.fsprofile else []) + +class Ext3FSMkfs(Ext2FSMkfs): + _opts = ["-t", "ext3"] + +class Ext4FSMkfs(Ext3FSMkfs): + _opts = ["-t", "ext4"] + +class FATFSMkfs(FSMkfs): + app_name = "mkdosfs" + label_option = "-n" + + @property + def args(self): + return [] + +class GFS2Mkfs(FSMkfs): + app_name = "mkfs.gfs2" + label_option = None + + @property + def args(self): + return ["-j", "1", "-p", "lock_nolock", "-O"] + +class HFSMkfs(FSMkfs): + app_name = "hformat" + label_option = "-l" + + @property + def args(self): + return [] + +class HFSPlusMkfs(FSMkfs): + app_name = "mkfs.hfsplus" + label_option = "-v" + + @property + def args(self): + return [] + +class JFSMkfs(FSMkfs): + app_name = "mkfs.jfs" + label_option = "-L" + + @property + def args(self): + return ["-q"] + +class NTFSMkfs(FSMkfs): + app_name = "mkntfs" + label_option = "-L" + + @property + def args(self): + return [] + +class ReiserFSMkfs(FSMkfs): + app_name = "mkreiserfs" + label_option = "-l" + + @property + def args(self): + return ["-f", "-f"] + +class XFSMkfs(FSMkfs): + app_name = "mkfs.xfs" + label_option = "-L" + + @property + def args(self): + return ["-f"]
In reply to line 77 of blivet/tasks/fsmkfs.py:
This does not seem to match the description of the ``unready`` property: ``` + unready = abc.abstractproperty( + doc="Reason if the external resource is not ready for this action.") ``` Maybe this should go to ``unable``?
In reply to line 86 of blivet/tasks/fsmkfs.py:
Could we please rename this to ``can_label``? ``labels`` looks/sounds to me like something that would return some list of labels.
In reply to line 102 of blivet/tasks/fsmkfs.py:
Why would anybody call this method with ``label=False`` to get label options? I'd guess that calling this function means that we want to label something. Or am I missing something about the ``label`` parameter?
In reply to line 123 of blivet/tasks/fsmkfs.py:
the two lines above should be swapped
In reply to line 130 of blivet/tasks/fsmkfs.py:
This could check the ``label`` parameter and avoid adding ``_labelOptions`` if it is ``False`` since this place is where such logic belongs to.
In reply to line 169 of blivet/tasks/fsmkfs.py:
The same question as in previous similar cases here -- why not simple ``args = []`` assignment?
In reply to line 77 of blivet/tasks/fsmkfs.py:
I'll try changing unready docstring to:
"Reason if the actual thing, e.g. device, is is not ready for this action."
In reply to line 86 of blivet/tasks/fsmkfs.py:
sure.
In reply to line 123 of blivet/tasks/fsmkfs.py:
done.
In reply to line 130 of blivet/tasks/fsmkfs.py:
done.
In reply to line 169 of blivet/tasks/fsmkfs.py:
same answer, need to bind later.
From: mulhern amulhern@redhat.com
Because labeling errors are sometimes not as severe as other errors, so we do want to distinguish.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/errors.py | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/blivet/errors.py b/blivet/errors.py index b05c0c7..4a93895 100644 --- a/blivet/errors.py +++ b/blivet/errors.py @@ -75,6 +75,12 @@ class MultipathMemberError(DeviceFormatError): class FSError(DeviceFormatError): pass
+class FSWriteLabelError(FSError): + pass + +class FSReadLabelError(FSError): + pass + class FSResizeError(FSError): def __init__(self, message, details): FSError.__init__(self, message)
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fssync.py | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 blivet/tasks/fssync.py
diff --git a/blivet/tasks/fssync.py b/blivet/tasks/fssync.py new file mode 100644 index 0000000..1fd64aa --- /dev/null +++ b/blivet/tasks/fssync.py @@ -0,0 +1,111 @@ +# fssync.py +# Filesystem syncing classes. +# +# Copyright (C) 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 +# 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 +import os + +from six import add_metaclass + +from ..errors import FSError +from .. import util + +from . import availability +from . import task + +@add_metaclass(abc.ABCMeta) +class FSSync(task.Task): + """ An abstract class that represents syncing a filesystem. """ + + description = "filesystem syncing" + + app_name = abc.abstractproperty(doc="The name of the syncing application.") + + @classmethod + def _app(cls): + return availability.Application(availability.Path(), cls.app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + @classmethod + def available(cls): + return cls._app().available + + @property + def unavailable(self): + if not self._app().available: + return "application %s not available" % self._app() + return None + + @property + def unready(self): + if not self.fs.status: + return "filesystem has not been mounted" + + if not os.path.exists(self.fs.device): + return "device does not exist" + + return False + + @property + def unable(self): + return False + + @abc.abstractmethod + def doTask(self): + raise NotImplementedError() + +class XFSSync(FSSync): + """ Info application for XFS. """ + + app_name = "xfs_freeze" + + def _freezeCommand(self): + return [str(self._app()), "-f", self.fs.systemMountpoint] + + def _unfreezeCommand(self): + return [str(self._app()), "-u", self.fs.systemMountpoint] + + def doTask(self, root="/"): + # pylint: disable=arguments-differ + error_msg = self.impossible + if error_msg: + raise FSError(error_msg) + + error_msg = None + try: + rc = util.run_program(self._freezeCommand(), root=root) + except OSError as e: + error_msg = "failed to sync filesytem: %s" % e + error_msg = error_msg or rc + + try: + rc = util.run_program(self._unfreezeCommand(), root=root) + except OSError as e: + error_msg = error_msg or "failed to sync filesystem: %s" % e + error_msg = error_msg or rc + + if error_msg: + raise FSError(error_msg)
From: mulhern amulhern@redhat.com
This particular step is low stress, as currently filesystem label reading is only done for testing.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 35 +++++++++++++++-------------------- tests/tasks_test/fslabeling.py | 6 +++--- 2 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 40fba02..3a6c179 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -27,7 +27,8 @@ import tempfile
from ..tasks import fslabeling -from ..errors import FormatCreateError, FSError, FSResizeError +from ..tasks import fsreadlabel +from ..errors import FormatCreateError, FSError, FSReadLabelError, FSResizeError from . import DeviceFormat, register_device_format from .. import util from .. import platform @@ -58,6 +59,7 @@ class FS(DeviceFormat): _fsck = "" # fs check utility _fsckErrors = {} # fs check command error codes & msgs _infofs = "" # fs info utility + _readlabelClass = None # read label _defaultFormatOptions = [] # default options passed to mkfs _defaultMountOptions = ["defaults"] # default options passed to mount _defaultCheckOptions = [] @@ -96,6 +98,10 @@ def getTaskObject(klass): raise TypeError("FS is an abstract class.")
DeviceFormat.__init__(self, **kwargs) + + # Create task objects + self._readlabel = getTaskObject(self._readlabelClass) + self.mountpoint = kwargs.get("mountpoint") self.mountopts = kwargs.get("mountopts") self.label = kwargs.get("label") @@ -695,27 +701,12 @@ def readLabel(self): :return: the filesystem's label :rtype: str
- Raises a FSError if the label can not be read. + Raises a FSReadLabelError if the label can not be read. """ - if not self.exists: - raise FSError("filesystem has not been created") - - if not os.path.exists(self.device): - raise FSError("device does not exist") - - if not self.relabels() or not self._labelfs.label_app.reads: - raise FSError("no application to read label for filesystem %s" % self.type) - - (rc, out) = util.run_program_and_capture_output(self._labelfs.label_app.readLabelCommand(self)) - if rc: - raise FSError("read label failed") - - label = out.strip() - - if label == "": - return "" + if self._readlabel is not None: + return self._readlabel.doTask() else: - return self._labelfs.label_app.extractLabel(label) + raise FSReadLabelError("label reading not implemented for filesystem %s" % self.type)
def writeLabel(self): """ Create a label for this filesystem. @@ -937,6 +928,7 @@ class Ext2FS(FS): _dump = True _check = True _infofs = "dumpe2fs" + _readlabelClass = fsreadlabel.Ext2FSReadLabel _defaultInfoOptions = ["-h"] _existingSizeFields = ["Block count:", "Block size:"] _resizefsUnit = MiB @@ -1073,6 +1065,7 @@ class FATFS(FS): _formattable = True _maxSize = Size("1 TiB") _packages = [ "dosfstools" ] + _readlabelClass = fsreadlabel.DosFSReadLabel _defaultMountOptions = ["umask=0077", "shortname=winnt"] _defaultCheckOptions = ["-n"] # FIXME this should be fat32 in some cases @@ -1247,6 +1240,7 @@ class XFS(FS): _supported = True _packages = ["xfsprogs"] _infofs = "xfs_db" + _readlabelClass = fsreadlabel.XFSReadLabel _defaultInfoOptions = ["-c", "sb 0", "-c", "p dblocks", "-c", "p blocksize"] _existingSizeFields = ["dblocks =", "blocksize ="] @@ -1352,6 +1346,7 @@ class NTFS(FS): _defaultCheckOptions = ["-c"] _packages = ["ntfsprogs"] _infofs = "ntfsinfo" + _readlabelClass = fsreadlabel.NTFSReadLabel _defaultInfoOptions = ["-m"] _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"] _resizefsUnit = B diff --git a/tests/tasks_test/fslabeling.py b/tests/tasks_test/fslabeling.py index 9c6dabb..6785133 100644 --- a/tests/tasks_test/fslabeling.py +++ b/tests/tasks_test/fslabeling.py @@ -4,7 +4,7 @@ from six import add_metaclass
from tests import loopbackedtestcase -from blivet.errors import FSError +from blivet.errors import FSError, FSReadLabelError from blivet.size import Size
@add_metaclass(abc.ABCMeta) @@ -40,7 +40,7 @@ def testLabeling(self): an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) self.assertIsNone(an_fs.create())
- with self.assertRaisesRegexp(FSError, "no application to read label"): + with self.assertRaises(FSReadLabelError): an_fs.readLabel()
an_fs.label = "an fs" @@ -81,7 +81,7 @@ def testLabeling(self): an_fs = self._fs_class(device=self.loopDevices[0], label=self._invalid_label) self.assertIsNone(an_fs.create())
- with self.assertRaisesRegexp(FSError, "no application to read label"): + with self.assertRaises(FSReadLabelError): an_fs.readLabel()
an_fs.label = "an fs"
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 3a6c179..917d08c 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -28,6 +28,7 @@
from ..tasks import fslabeling from ..tasks import fsreadlabel +from ..tasks import fssync from ..errors import FormatCreateError, FSError, FSReadLabelError, FSResizeError from . import DeviceFormat, register_device_format from .. import util @@ -60,6 +61,7 @@ class FS(DeviceFormat): _fsckErrors = {} # fs check command error codes & msgs _infofs = "" # fs info utility _readlabelClass = None # read label + _syncClass = None # sync the filesystem _defaultFormatOptions = [] # default options passed to mkfs _defaultMountOptions = ["defaults"] # default options passed to mount _defaultCheckOptions = [] @@ -101,6 +103,7 @@ def getTaskObject(klass):
# Create task objects self._readlabel = getTaskObject(self._readlabelClass) + self._sync = getTaskObject(self._syncClass)
self.mountpoint = kwargs.get("mountpoint") self.mountopts = kwargs.get("mountopts") @@ -891,8 +894,22 @@ def status(self): return False return self.systemMountpoint is not None
- def sync(self, root="/"): - pass + def sync(self, root='/'): + """ Ensure that data we've written is at least in the journal. + + This is a little odd because xfs_freeze will only be + available under the install root. + """ + if self._sync is None: + return + + if not self._mountpoint.startswith(root): + return + + try: + self._sync.doTask(root) + except FSError as e: + log.error(e)
def populateKSData(self, data): super(FS, self).populateKSData(data) @@ -1241,34 +1258,14 @@ class XFS(FS): _packages = ["xfsprogs"] _infofs = "xfs_db" _readlabelClass = fsreadlabel.XFSReadLabel + _syncClass = fssync.XFSSync _defaultInfoOptions = ["-c", "sb 0", "-c", "p dblocks", "-c", "p blocksize"] _existingSizeFields = ["dblocks =", "blocksize ="] partedSystem = fileSystemType["xfs"]
- def sync(self, root='/'): - """ Ensure that data we've written is at least in the journal. - - This is a little odd because xfs_freeze will only be - available under the install root. - """ - if not self.status or not self.systemMountpoint or \ - not self.systemMountpoint.startswith(root): - return - - try: - util.run_program(["xfs_freeze", "-f", self.systemMountpoint], root=root) - except OSError as e: - log.error("failed to run xfs_freeze: %s", e) - - try: - util.run_program(["xfs_freeze", "-u", self.systemMountpoint], root=root) - except OSError as e: - log.error("failed to run xfs_freeze: %s", e) - register_device_format(XFS)
- class HFS(FS): _type = "hfs" _mkfs = "hformat"
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 65acf6f..40fba02 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -87,6 +87,11 @@ def __init__(self, **kwargs): that you can specify the device at the last moment by specifying it via the 'device' kwarg to the :meth:`create` method. """ + + def getTaskObject(klass): + # pylint: disable=not-callable + return klass(self) if klass is not None else None + if self.__class__ is FS: raise TypeError("FS is an abstract class.")
From: mulhern amulhern@redhat.com
We don't translate the text of the other FSError objects that we raise, and it's kind of wrong to do translation in a task object.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsck.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/blivet/tasks/fsck.py b/blivet/tasks/fsck.py index 49aadae..2ada93c 100644 --- a/blivet/tasks/fsck.py +++ b/blivet/tasks/fsck.py @@ -24,15 +24,13 @@
from six import add_metaclass
-from ..i18n import _, N_ - from ..errors import FSError from .. import util
from . import availability from . import task
-_UNKNOWN_RC_MSG = N_("Unknown return code: %d") +_UNKNOWN_RC_MSG = "Unknown return code: %d"
@add_metaclass(abc.ABCMeta) class FSCK(task.Task): @@ -123,16 +121,16 @@ def doTask(self):
error_msg = self._errorMessage(rc) if error_msg is not None: - hdr = _("%(type)s filesystem check failure on %(device)s: ") % \ + hdr = "%(type)s filesystem check failure on %(device)s: " % \ {"type": self.fs.type, "device": self.fs.device}
raise FSError(hdr + error_msg)
class DosFSCK(FSCK): - _fsckErrors = {1: N_("Recoverable errors have been detected or dosfsck has " - "discovered an internal inconsistency."), - 2: N_("Usage error.")} + _fsckErrors = {1: "Recoverable errors have been detected or dosfsck has " + "discovered an internal inconsistency.", + 2: "Usage error."}
app_name = "dosfsck" options = ["-n"] @@ -141,24 +139,24 @@ def _errorMessage(self, rc): if rc < 1: return None try: - return _(self._fsckErrors[rc]) + return self._fsckErrors[rc] except KeyError: return _UNKNOWN_RC_MSG % rc
class Ext2FSCK(FSCK): - _fsckErrors = {4: N_("File system errors left uncorrected."), - 8: N_("Operational error."), - 16: N_("Usage or syntax error."), - 32: N_("e2fsck cancelled by user request."), - 128: N_("Shared library error.")} + _fsckErrors = {4: "File system errors left uncorrected.", + 8: "Operational error.", + 16: "Usage or syntax error.", + 32: "e2fsck cancelled by user request.", + 128: "Shared library error."}
app_name = "e2fsck" options = ["-f", "-p", "-C", "0"]
def _errorMessage(self, rc): - msgs = (_(self._fsckErrors[c]) for c in self._fsckErrors.keys() if rc & c) - return "\n".join(msgs).strip() or None + msgs = (self._fsckErrors[c] for c in self._fsckErrors.keys() if rc & c) + return "\n".join(msgs) or None
class HFSPlusFSCK(FSCK): app_name = "fsck.hfsplus"
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 101 ++++++--------------------------------------------- 1 file changed, 11 insertions(+), 90 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 917d08c..b3347f3 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -26,6 +26,7 @@ import os import tempfile
+from ..tasks import fsck from ..tasks import fslabeling from ..tasks import fsreadlabel from ..tasks import fssync @@ -39,7 +40,7 @@ from .. import arch from ..size import Size, ROUND_UP, ROUND_DOWN, unitStr from ..size import B, KiB, MiB, GiB, KB, MB, GB -from ..i18n import _, N_ +from ..i18n import N_ from .. import udev from ..mounts import mountsCache
@@ -57,14 +58,12 @@ class FS(DeviceFormat): _modules = [] # kernel modules required for support _resizefs = "" # resize utility _labelfs = None # labeling functionality - _fsck = "" # fs check utility - _fsckErrors = {} # fs check command error codes & msgs _infofs = "" # fs info utility + _fsckClass = None # fsck _readlabelClass = None # read label _syncClass = None # sync the filesystem _defaultFormatOptions = [] # default options passed to mkfs _defaultMountOptions = ["defaults"] # default options passed to mount - _defaultCheckOptions = [] _defaultInfoOptions = [] _existingSizeFields = [] _resizefsUnit = None @@ -102,6 +101,7 @@ def getTaskObject(klass): DeviceFormat.__init__(self, **kwargs)
# Create task objects + self._fsck = getTaskObject(self._fsckClass) self._readlabel = getTaskObject(self._readlabelClass) self._sync = getTaskObject(self._syncClass)
@@ -520,45 +520,16 @@ def doResize(self): self._size = self.targetSize self.notifyKernel()
- def _getCheckArgs(self): - argv = [] - argv.extend(self.defaultCheckOptions) - argv.append(self.device) - return argv - - def _fsckFailed(self, rc): - # pylint: disable=unused-argument - return False - - def _fsckErrorMessage(self, rc): - return _("Unknown return code: %d.") % (rc,) - def doCheck(self): """ Run a filesystem check.
:raises: FSError """ - if not self.exists: - raise FSError("filesystem has not been created") - - if not self.fsckProg: + if self._fsck is not None: + self._fsck.doTask() + else: return
- if not os.path.exists(self.device): - raise FSError("device does not exist") - - try: - ret = util.run_program([self.fsckProg] + self._getCheckArgs()) - except OSError as e: - raise FSError("filesystem check failed: %s" % e) - - if self._fsckFailed(ret): - hdr = _("%(type)s filesystem check failure on %(device)s: ") % \ - {"type": self.type, "device": self.device} - - msg = self._fsckErrorMessage(ret) - raise FSError(hdr + msg) - def loadModule(self): """Load whatever kernel module is required to support this filesystem.""" if not self._modules or self.mountType in kernel_filesystems: @@ -748,11 +719,6 @@ def mkfsProg(self): return self._mkfs
@property - def fsckProg(self): - """ Program used to check filesystems of this type. """ - return self._fsck - - @property def resizefsProg(self): """ Program used to resize filesystems of this type. """ return self._resizefs @@ -825,12 +791,6 @@ def defaultMountOptions(self): # return a copy to prevent modification return self._defaultMountOptions[:]
- @property - def defaultCheckOptions(self): - """ Default options passed to checker for this filesystem type. """ - # return a copy to prevent modification - return self._defaultCheckOptions[:] - def _getOptions(self): return self.mountopts or ",".join(self.defaultMountOptions)
@@ -929,22 +889,16 @@ class Ext2FS(FS): _modules = ["ext2"] _resizefs = "resize2fs" _labelfs = fslabeling.Ext2FSLabeling() - _fsck = "e2fsck" - _fsckErrors = {4: N_("File system errors left uncorrected."), - 8: N_("Operational error."), - 16: N_("Usage or syntax error."), - 32: N_("e2fsck cancelled by user request."), - 128: N_("Shared library error.")} _packages = ["e2fsprogs"] _formattable = True _supported = True _resizable = True _linuxNative = True _maxSize = Size("8 TiB") - _defaultCheckOptions = ["-f", "-p", "-C", "0"] _dump = True _check = True _infofs = "dumpe2fs" + _fsckClass = fsck.Ext2FSCK _readlabelClass = fsreadlabel.Ext2FSReadLabel _defaultInfoOptions = ["-h"] _existingSizeFields = ["Block count:", "Block size:"] @@ -952,21 +906,6 @@ class Ext2FS(FS): _fsProfileSpecifier = "-T" partedSystem = fileSystemType["ext2"]
- def _fsckFailed(self, rc): - for errorCode in self._fsckErrors.keys(): - if rc & errorCode: - return True - return False - - def _fsckErrorMessage(self, rc): - msg = '' - - for errorCode in self._fsckErrors.keys(): - if rc & errorCode: - msg += "\n" + _(self._fsckErrors[errorCode]) - - return msg.strip() - def _getMinSize(self, info=None): """ Set the minimum size for this filesystem in MiB.
@@ -1074,28 +1013,16 @@ class FATFS(FS): _mkfs = "mkdosfs" _modules = ["vfat"] _labelfs = fslabeling.FATFSLabeling() - _fsck = "dosfsck" - _fsckErrors = {1: N_("Recoverable errors have been detected or dosfsck has " - "discovered an internal inconsistency."), - 2: N_("Usage error.")} _supported = True _formattable = True _maxSize = Size("1 TiB") _packages = [ "dosfstools" ] + _fsckClass = fsck.DosFSCK _readlabelClass = fsreadlabel.DosFSReadLabel _defaultMountOptions = ["umask=0077", "shortname=winnt"] - _defaultCheckOptions = ["-n"] # FIXME this should be fat32 in some cases partedSystem = fileSystemType["fat16"]
- def _fsckFailed(self, rc): - if rc >= 1: - return True - return False - - def _fsckErrorMessage(self, rc): - return _(self._fsckErrors[rc]) - register_device_format(FATFS)
@@ -1297,7 +1224,6 @@ class HFSPlus(FS): _modules = ["hfsplus"] _udevTypes = ["hfsplus"] _mkfs = "mkfs.hfsplus" - _fsck = "fsck.hfsplus" _packages = ["hfsplus-tools"] _labelfs = fslabeling.HFSPlusLabeling() _formattable = True @@ -1306,6 +1232,7 @@ class HFSPlus(FS): _maxSize = Size("2 TiB") _check = True partedSystem = fileSystemType["hfs+"] + _fsckClass = fsck.HFSPlusFSCK
register_device_format(HFSPlus)
@@ -1335,25 +1262,19 @@ class NTFS(FS): _mkfs = "mkntfs" _resizefs = "ntfsresize" _labelfs = fslabeling.NTFSLabeling() - _fsck = "ntfsresize" _resizable = True _minSize = Size("1 MiB") _maxSize = Size("16 TiB") _defaultMountOptions = ["defaults", "ro"] - _defaultCheckOptions = ["-c"] _packages = ["ntfsprogs"] _infofs = "ntfsinfo" + _fsckClass = fsck.NTFSFSCK _readlabelClass = fsreadlabel.NTFSReadLabel _defaultInfoOptions = ["-m"] _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"] _resizefsUnit = B partedSystem = fileSystemType["ntfs"]
- def _fsckFailed(self, rc): - if rc != 0: - return True - return False - def _getMinSize(self, info=None): """ Set the minimum size for this filesystem.
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 47 +++++++++++++++++++++++------------------ tests/formats_test/fstesting.py | 2 +- 2 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index b3347f3..3c1a3b1 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -27,6 +27,7 @@ import tempfile
from ..tasks import fsck +from ..tasks import fsinfo from ..tasks import fslabeling from ..tasks import fsreadlabel from ..tasks import fssync @@ -58,13 +59,12 @@ class FS(DeviceFormat): _modules = [] # kernel modules required for support _resizefs = "" # resize utility _labelfs = None # labeling functionality - _infofs = "" # fs info utility _fsckClass = None # fsck + _infoClass = None _readlabelClass = None # read label _syncClass = None # sync the filesystem _defaultFormatOptions = [] # default options passed to mkfs _defaultMountOptions = ["defaults"] # default options passed to mount - _defaultInfoOptions = [] _existingSizeFields = [] _resizefsUnit = None _fsProfileSpecifier = None # mkfs option specifying fsprofile @@ -101,6 +101,7 @@ def getTaskObject(klass): DeviceFormat.__init__(self, **kwargs)
# Create task objects + self._info = getTaskObject(self._infoClass) self._fsck = getTaskObject(self._fsckClass) self._readlabel = getTaskObject(self._readlabelClass) self._sync = getTaskObject(self._syncClass) @@ -254,14 +255,21 @@ def _getMinSize(self, info=None): pass
def _getFSInfo(self): + """ Get filesystem information by use of external tools. + + :returns: the output of the tool + :rtype: str + + If for any reason the information is not obtained returns the + empty string. + """ buf = "" - if self.infofsProg and self.exists and \ - util.find_program_in_path(self.infofsProg): - argv = self._defaultInfoOptions + [ self.device ] + + if self._info is not None and not self._info.unavailable: try: - buf = util.capture_output([self.infofsProg] + argv) - except OSError as e: - log.error("failed to gather fs info: %s", e) + buf = self._info.doTask() + except FSError as e: + log.error(e)
return buf
@@ -736,8 +744,12 @@ def labelfsProg(self):
@property def infofsProg(self): - """ Program used to get information for this filesystem type. """ - return self._infofs + """ Program used to get information for this filesystem type. + + :returns: the name of the program used to get information + :rtype: str or NoneType + """ + return self._info.app_name if self._info else None
@property def utilsAvailable(self): @@ -897,10 +909,9 @@ class Ext2FS(FS): _maxSize = Size("8 TiB") _dump = True _check = True - _infofs = "dumpe2fs" _fsckClass = fsck.Ext2FSCK + _infoClass = fsinfo.Ext2FSInfo _readlabelClass = fsreadlabel.Ext2FSReadLabel - _defaultInfoOptions = ["-h"] _existingSizeFields = ["Block count:", "Block size:"] _resizefsUnit = MiB _fsProfileSpecifier = "-T" @@ -1133,8 +1144,7 @@ class JFS(FS): _linuxNative = True _dump = True _check = True - _infofs = "jfs_tune" - _defaultInfoOptions = ["-l"] + _infoClass = fsinfo.JFSInfo _existingSizeFields = ["Physical block size:", "Aggregate size:"] partedSystem = fileSystemType["jfs"]
@@ -1159,7 +1169,7 @@ class ReiserFS(FS): _dump = True _check = True _packages = ["reiserfs-utils"] - _infofs = "debugreiserfs" + _infoClass = fsinfo.ReiserFSInfo _existingSizeFields = ["Count of blocks on the device:", "Blocksize:"] partedSystem = fileSystemType["reiserfs"]
@@ -1183,11 +1193,9 @@ class XFS(FS): _linuxNative = True _supported = True _packages = ["xfsprogs"] - _infofs = "xfs_db" + _infoClass = fsinfo.XFSInfo _readlabelClass = fsreadlabel.XFSReadLabel _syncClass = fssync.XFSSync - _defaultInfoOptions = ["-c", "sb 0", "-c", "p dblocks", - "-c", "p blocksize"] _existingSizeFields = ["dblocks =", "blocksize ="] partedSystem = fileSystemType["xfs"]
@@ -1267,10 +1275,9 @@ class NTFS(FS): _maxSize = Size("16 TiB") _defaultMountOptions = ["defaults", "ro"] _packages = ["ntfsprogs"] - _infofs = "ntfsinfo" _fsckClass = fsck.NTFSFSCK + _infoClass = fsinfo.NTFSInfo _readlabelClass = fsreadlabel.NTFSReadLabel - _defaultInfoOptions = ["-m"] _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"] _resizefsUnit = B partedSystem = fileSystemType["ntfs"] diff --git a/tests/formats_test/fstesting.py b/tests/formats_test/fstesting.py index 9a5d92e..600726e 100644 --- a/tests/formats_test/fstesting.py +++ b/tests/formats_test/fstesting.py @@ -47,7 +47,7 @@ def _test_sizes(self, an_fs): else: expected_size = _size # If the size can be obtained it will not be 0 - if an_fs._infofs: + if an_fs._info: self.assertNotEqual(expected_size, Size(0)) self.assertTrue(expected_size <= self._DEVICE_SIZE) # Otherwise it will be 0, assuming the device was not initialized
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 39 +++++++++++++++------------------------ tests/tasks_test/fslabeling.py | 2 +- 2 files changed, 16 insertions(+), 25 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 3c1a3b1..3615e11 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -31,6 +31,7 @@ from ..tasks import fslabeling from ..tasks import fsreadlabel from ..tasks import fssync +from ..tasks import fswritelabel from ..errors import FormatCreateError, FSError, FSReadLabelError, FSResizeError from . import DeviceFormat, register_device_format from .. import util @@ -63,6 +64,7 @@ class FS(DeviceFormat): _infoClass = None _readlabelClass = None # read label _syncClass = None # sync the filesystem + _writelabelClass = None # write label after creation _defaultFormatOptions = [] # default options passed to mkfs _defaultMountOptions = ["defaults"] # default options passed to mount _existingSizeFields = [] @@ -105,6 +107,7 @@ def getTaskObject(klass): self._fsck = getTaskObject(self._fsckClass) self._readlabel = getTaskObject(self._readlabelClass) self._sync = getTaskObject(self._syncClass) + self._writelabel = getTaskObject(self._writelabelClass)
self.mountpoint = kwargs.get("mountpoint") self.mountopts = kwargs.get("mountopts") @@ -169,7 +172,7 @@ def relabels(self):
:rtype: bool """ - return self._labelfs is not None and self._labelfs.label_app is not None + return self._writelabel is not None and not self._writelabel.unavailable
def labelFormatOK(self, label): """Return True if the label has an acceptable format for this @@ -700,25 +703,10 @@ def writeLabel(self):
Raises a FSError if the label can not be set. """ - if self.label is None: - raise FSError("makes no sense to write a label when accepting default label") - - if not self.exists: - raise FSError("filesystem has not been created") - - if not self.relabels(): - raise FSError("no application to set label for filesystem %s" % self.type) - - if not self.labelFormatOK(self.label): - raise FSError("bad label format for labelling application %s" % self._labelfs.label_app.name) - - if not os.path.exists(self.device): - raise FSError("device does not exist") - - rc = util.run_program(self._labelfs.label_app.setLabelCommand(self)) - if rc: - raise FSError("label failed") - + if self._writelabel: + self._writelabel.doTask() + else: + raise FSError("label writing not implemented for filesystem %s" % self.type) self.notifyKernel()
@property @@ -737,10 +725,7 @@ def labelfsProg(self):
May be None if no such program exists. """ - if self._labelfs and self._labelfs.label_app: - return self._labelfs.label_app.name - else: - return None + return self._writelabel.app_name if self._writelabel else None
@property def infofsProg(self): @@ -912,6 +897,7 @@ class Ext2FS(FS): _fsckClass = fsck.Ext2FSCK _infoClass = fsinfo.Ext2FSInfo _readlabelClass = fsreadlabel.Ext2FSReadLabel + _writelabelClass = fswritelabel.Ext2FSWriteLabel _existingSizeFields = ["Block count:", "Block size:"] _resizefsUnit = MiB _fsProfileSpecifier = "-T" @@ -1030,6 +1016,7 @@ class FATFS(FS): _packages = [ "dosfstools" ] _fsckClass = fsck.DosFSCK _readlabelClass = fsreadlabel.DosFSReadLabel + _writelabelClass = fswritelabel.DosFSWriteLabel _defaultMountOptions = ["umask=0077", "shortname=winnt"] # FIXME this should be fat32 in some cases partedSystem = fileSystemType["fat16"] @@ -1145,6 +1132,7 @@ class JFS(FS): _dump = True _check = True _infoClass = fsinfo.JFSInfo + _writelabelClass = fswritelabel.JFSWriteLabel _existingSizeFields = ["Physical block size:", "Aggregate size:"] partedSystem = fileSystemType["jfs"]
@@ -1170,6 +1158,7 @@ class ReiserFS(FS): _check = True _packages = ["reiserfs-utils"] _infoClass = fsinfo.ReiserFSInfo + _writelabelClass = fswritelabel.ReiserFSWriteLabel _existingSizeFields = ["Count of blocks on the device:", "Blocksize:"] partedSystem = fileSystemType["reiserfs"]
@@ -1196,6 +1185,7 @@ class XFS(FS): _infoClass = fsinfo.XFSInfo _readlabelClass = fsreadlabel.XFSReadLabel _syncClass = fssync.XFSSync + _writelabelClass = fswritelabel.XFSWriteLabel _existingSizeFields = ["dblocks =", "blocksize ="] partedSystem = fileSystemType["xfs"]
@@ -1278,6 +1268,7 @@ class NTFS(FS): _fsckClass = fsck.NTFSFSCK _infoClass = fsinfo.NTFSInfo _readlabelClass = fsreadlabel.NTFSReadLabel + _writelabelClass = fswritelabel.NTFSWriteLabel _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"] _resizefsUnit = B partedSystem = fileSystemType["ntfs"] diff --git a/tests/tasks_test/fslabeling.py b/tests/tasks_test/fslabeling.py index 6785133..4c76265 100644 --- a/tests/tasks_test/fslabeling.py +++ b/tests/tasks_test/fslabeling.py @@ -44,7 +44,7 @@ def testLabeling(self): an_fs.readLabel()
an_fs.label = "an fs" - with self.assertRaisesRegexp(FSError, "no application to set label for filesystem"): + with self.assertRaises(FSError): an_fs.writeLabel()
def testCreating(self):
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 93 +++++++++++++--------------------------------------- 1 file changed, 23 insertions(+), 70 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 3615e11..2ac8885 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -29,10 +29,12 @@ from ..tasks import fsck from ..tasks import fsinfo from ..tasks import fslabeling +from ..tasks import fsmkfs from ..tasks import fsreadlabel from ..tasks import fssync from ..tasks import fswritelabel -from ..errors import FormatCreateError, FSError, FSReadLabelError, FSResizeError +from ..errors import FormatCreateError, FSError, FSReadLabelError +from ..errors import FSWriteLabelError, FSResizeError from . import DeviceFormat, register_device_format from .. import util from .. import platform @@ -56,20 +58,18 @@ class FS(DeviceFormat): _type = "Abstract Filesystem Class" # fs type name _mountType = None # like _type but for passing to mount _name = None - _mkfs = "" # mkfs utility _modules = [] # kernel modules required for support _resizefs = "" # resize utility _labelfs = None # labeling functionality _fsckClass = None # fsck _infoClass = None + _mkfsClass = None # mkfs _readlabelClass = None # read label _syncClass = None # sync the filesystem _writelabelClass = None # write label after creation - _defaultFormatOptions = [] # default options passed to mkfs _defaultMountOptions = ["defaults"] # default options passed to mount _existingSizeFields = [] _resizefsUnit = None - _fsProfileSpecifier = None # mkfs option specifying fsprofile
def __init__(self, **kwargs): """ @@ -105,6 +105,7 @@ def getTaskObject(klass): # Create task objects self._info = getTaskObject(self._infoClass) self._fsck = getTaskObject(self._fsckClass) + self._mkfs = getTaskObject(self._mkfsClass) self._readlabel = getTaskObject(self._readlabelClass) self._sync = getTaskObject(self._syncClass) self._writelabel = getTaskObject(self._writelabelClass) @@ -373,30 +374,6 @@ def free(self): """ return max(Size(0), self.currentSize - self.minSize)
- def _getFormatOptions(self, options=None, do_labeling=False): - """Get a list of format options to be used when creating the - filesystem. - - :param options: any special options - :type options: list of str or None - :param bool do_labeling: True if labeling during filesystem creation, - otherwise False - """ - argv = [] - if options and isinstance(options, list): - argv.extend(options) - argv.extend(self.defaultFormatOptions) - if self._fsProfileSpecifier and self.fsprofile: - argv.extend([self._fsProfileSpecifier, self.fsprofile]) - - if do_labeling and self.label is not None: - if self.labelFormatOK(self.label): - argv.extend(self._labelfs.labelingArgs(self.label)) - else: - log.warning("Choosing not to apply label (%s) during creation of filesystem %s. Label format is unacceptable for this filesystem.", self.label, self.type) - - argv.append(self.device) - return argv
def doFormat(self, options=None): """ Create the filesystem. @@ -408,29 +385,16 @@ def doFormat(self, options=None): log_method_call(self, type=self.mountType, device=self.device, mountpoint=self.mountpoint)
- if self.exists: - raise FormatCreateError("filesystem already exists", self.device) - - if not self.formattable: - return - - if not self.mkfsProg: + if not self._mkfs: return
- if not os.path.exists(self.device): - raise FormatCreateError("device does not exist", self.device) - - argv = self._getFormatOptions(options=options, - do_labeling=not self.relabels()) - try: - ret = util.run_program([self.mkfsProg] + argv) - except OSError as e: + self._mkfs.doTask(options=options, label=not self.relabels()) + except FSWriteLabelError as e: + log.warning("Choosing not to apply label (%s) during creation of filesystem %s. Label format is unacceptable for this filesystem.", self.label, self.type) + except FSError as e: raise FormatCreateError(e, self.device)
- if ret: - raise FormatCreateError("format failed: %s" % ret, self.device) - self.exists = True self.notifyKernel()
@@ -712,7 +676,7 @@ def writeLabel(self): @property def mkfsProg(self): """ Program used to create filesystems of this type. """ - return self._mkfs + return self._mkfs.app_name if self._mkfs else None
@property def resizefsProg(self): @@ -777,12 +741,6 @@ def resizable(self): return super(FS, self).resizable and self.utilsAvailable
@property - def defaultFormatOptions(self): - """ Default options passed to mkfs for this filesystem type. """ - # return a copy to prevent modification - return self._defaultFormatOptions[:] - - @property def defaultMountOptions(self): """ Default options passed to mount for this filesystem type. """ # return a copy to prevent modification @@ -882,7 +840,6 @@ def populateKSData(self, data): class Ext2FS(FS): """ ext2 filesystem. """ _type = "ext2" - _mkfs = "mke2fs" _modules = ["ext2"] _resizefs = "resize2fs" _labelfs = fslabeling.Ext2FSLabeling() @@ -896,11 +853,11 @@ class Ext2FS(FS): _check = True _fsckClass = fsck.Ext2FSCK _infoClass = fsinfo.Ext2FSInfo + _mkfsClass = fsmkfs.Ext2FSMkfs _readlabelClass = fsreadlabel.Ext2FSReadLabel _writelabelClass = fswritelabel.Ext2FSWriteLabel _existingSizeFields = ["Block count:", "Block size:"] _resizefsUnit = MiB - _fsProfileSpecifier = "-T" partedSystem = fileSystemType["ext2"]
def _getMinSize(self, info=None): @@ -981,9 +938,9 @@ def resizeArgs(self): class Ext3FS(Ext2FS): """ ext3 filesystem. """ _type = "ext3" - _defaultFormatOptions = ["-t", "ext3"] _modules = ["ext3"] partedSystem = fileSystemType["ext3"] + _mkfsClass = fsmkfs.Ext3FSMkfs
# It is possible for a user to specify an fsprofile that defines a blocksize # smaller than the default of 4096 bytes and therefore to make liars of us @@ -997,8 +954,8 @@ class Ext3FS(Ext2FS): class Ext4FS(Ext3FS): """ ext4 filesystem. """ _type = "ext4" - _defaultFormatOptions = ["-t", "ext4"] _modules = ["ext4"] + _mkfsClass = fsmkfs.Ext4FSMkfs partedSystem = fileSystemType["ext4"]
register_device_format(Ext4FS) @@ -1007,7 +964,6 @@ class Ext4FS(Ext3FS): class FATFS(FS): """ FAT filesystem. """ _type = "vfat" - _mkfs = "mkdosfs" _modules = ["vfat"] _labelfs = fslabeling.FATFSLabeling() _supported = True @@ -1015,6 +971,7 @@ class FATFS(FS): _maxSize = Size("1 TiB") _packages = [ "dosfstools" ] _fsckClass = fsck.DosFSCK + _mkfsClass = fsmkfs.FATFSMkfs _readlabelClass = fsreadlabel.DosFSReadLabel _writelabelClass = fswritelabel.DosFSWriteLabel _defaultMountOptions = ["umask=0077", "shortname=winnt"] @@ -1041,7 +998,6 @@ def supported(self): class BTRFS(FS): """ btrfs filesystem """ _type = "btrfs" - _mkfs = "mkfs.btrfs" _modules = ["btrfs"] _formattable = True _linuxNative = True @@ -1049,6 +1005,7 @@ class BTRFS(FS): _packages = ["btrfs-progs"] _minSize = Size("256 MiB") _maxSize = Size("16 EiB") + _mkfsClass = fsmkfs.BTRFSMkfs # FIXME parted needs to be taught about btrfs so that we can set the # partition table type correctly for btrfs partitions # partedSystem = fileSystemType["btrfs"] @@ -1099,14 +1056,13 @@ def setup(self, **kwargs): class GFS2(FS): """ gfs2 filesystem. """ _type = "gfs2" - _mkfs = "mkfs.gfs2" _modules = ["dlm", "gfs2"] _formattable = True - _defaultFormatOptions = ["-j", "1", "-p", "lock_nolock", "-O"] _linuxNative = True _dump = True _check = True _packages = ["gfs2-utils"] + _mkfsClass = fsmkfs.GFS2Mkfs # FIXME parted needs to be thaught about btrfs so that we can set the # partition table type correctly for btrfs partitions # partedSystem = fileSystemType["gfs2"] @@ -1122,16 +1078,15 @@ def supported(self): class JFS(FS): """ JFS filesystem """ _type = "jfs" - _mkfs = "mkfs.jfs" _modules = ["jfs"] _labelfs = fslabeling.JFSLabeling() - _defaultFormatOptions = ["-q"] _maxSize = Size("8 TiB") _formattable = True _linuxNative = True _dump = True _check = True _infoClass = fsinfo.JFSInfo + _mkfsClass = fsmkfs.JFSMkfs _writelabelClass = fswritelabel.JFSWriteLabel _existingSizeFields = ["Physical block size:", "Aggregate size:"] partedSystem = fileSystemType["jfs"] @@ -1147,10 +1102,8 @@ def supported(self): class ReiserFS(FS): """ reiserfs filesystem """ _type = "reiserfs" - _mkfs = "mkreiserfs" _labelfs = fslabeling.ReiserFSLabeling() _modules = ["reiserfs"] - _defaultFormatOptions = ["-f", "-f"] _maxSize = Size("16 TiB") _formattable = True _linuxNative = True @@ -1158,6 +1111,7 @@ class ReiserFS(FS): _check = True _packages = ["reiserfs-utils"] _infoClass = fsinfo.ReiserFSInfo + _mkfsClass = fsmkfs.ReiserFSMkfs _writelabelClass = fswritelabel.ReiserFSWriteLabel _existingSizeFields = ["Count of blocks on the device:", "Blocksize:"] partedSystem = fileSystemType["reiserfs"] @@ -1173,16 +1127,15 @@ def supported(self): class XFS(FS): """ XFS filesystem """ _type = "xfs" - _mkfs = "mkfs.xfs" _modules = ["xfs"] _labelfs = fslabeling.XFSLabeling() - _defaultFormatOptions = ["-f"] _maxSize = Size("16 EiB") _formattable = True _linuxNative = True _supported = True _packages = ["xfsprogs"] _infoClass = fsinfo.XFSInfo + _mkfsClass = fsmkfs.XFSMkfs _readlabelClass = fsreadlabel.XFSReadLabel _syncClass = fssync.XFSSync _writelabelClass = fswritelabel.XFSWriteLabel @@ -1193,10 +1146,10 @@ class XFS(FS):
class HFS(FS): _type = "hfs" - _mkfs = "hformat" _modules = ["hfs"] _labelfs = fslabeling.HFSLabeling() _formattable = True + _mkfsClass = fsmkfs.HFSMkfs partedSystem = fileSystemType["hfs"]
register_device_format(HFS) @@ -1221,7 +1174,6 @@ class HFSPlus(FS): _type = "hfs+" _modules = ["hfsplus"] _udevTypes = ["hfsplus"] - _mkfs = "mkfs.hfsplus" _packages = ["hfsplus-tools"] _labelfs = fslabeling.HFSPlusLabeling() _formattable = True @@ -1231,6 +1183,7 @@ class HFSPlus(FS): _check = True partedSystem = fileSystemType["hfs+"] _fsckClass = fsck.HFSPlusFSCK + _mkfsClass = fsmkfs.HFSPlusMkfs
register_device_format(HFSPlus)
@@ -1257,7 +1210,6 @@ def __init__(self, **kwargs): class NTFS(FS): """ ntfs filesystem. """ _type = "ntfs" - _mkfs = "mkntfs" _resizefs = "ntfsresize" _labelfs = fslabeling.NTFSLabeling() _resizable = True @@ -1267,6 +1219,7 @@ class NTFS(FS): _packages = ["ntfsprogs"] _fsckClass = fsck.NTFSFSCK _infoClass = fsinfo.NTFSInfo + _mkfsClass = fsmkfs.NTFSMkfs _readlabelClass = fsreadlabel.NTFSReadLabel _writelabelClass = fswritelabel.NTFSWriteLabel _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"]
From: mulhern amulhern@redhat.com
The only place these methods were called, _getFSInfo has just been called. Also, it makes dealing with these methods a bit more awkward in task framework.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index c5d39e9..03cce99 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -321,9 +321,6 @@ def _getExistingSize(self, info=None):
size = Size(0) if self.exists: - if info is None: - info = self._getFSInfo() - try: values = [] for line in info.splitlines(): @@ -871,10 +868,6 @@ def _getMinSize(self, info=None): blockSize = None
if self.exists and os.path.exists(self.device): - if info is None: - # get block size - info = self._getFSInfo() - for line in info.splitlines(): if line.startswith("Block size:"): blockSize = int(line.split(" ")[-1])
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 03cce99..7fc93ae 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -110,6 +110,8 @@ def getTaskObject(klass): self._sync = getTaskObject(self._syncClass) self._writelabel = getTaskObject(self._writelabelClass)
+ self._current_info = None # info obtained by _info task + self.mountpoint = kwargs.get("mountpoint") self.mountopts = kwargs.get("mountopts") self.label = kwargs.get("label") @@ -243,8 +245,8 @@ def updateSizeInfo(self): errors = False finally: # try to gather current size info anyway - info = self._getFSInfo() - self._size = self._getExistingSize(info=info) + self._current_info = self._getFSInfo() + self._size = self._getExistingSize(info=self._current_info) self._minSize = self._size # default to current size # We absolutely need a current size to enable resize. To shrink the # filesystem we need a real minimum size provided by the resize @@ -253,7 +255,7 @@ def updateSizeInfo(self): if errors or self._size == Size(0): self._resizable = False
- self._getMinSize(info=info) # force calculation of minimum size + self._getMinSize(info=self._current_info) # force calculation of minimum size
def _getMinSize(self, info=None): pass
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 124 ++++++++------------------------------------------- 1 file changed, 18 insertions(+), 106 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 7fc93ae..5b1b7d0 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -31,6 +31,7 @@ from ..tasks import fslabeling from ..tasks import fsmkfs from ..tasks import fsreadlabel +from ..tasks import fssize from ..tasks import fssync from ..tasks import fswritelabel from ..errors import FormatCreateError, FSError, FSReadLabelError @@ -65,10 +66,10 @@ class FS(DeviceFormat): _infoClass = None _mkfsClass = None # mkfs _readlabelClass = None # read label + _sizeinfoClass = None # get current size _syncClass = None # sync the filesystem _writelabelClass = None # write label after creation _defaultMountOptions = ["defaults"] # default options passed to mount - _existingSizeFields = [] _resizefsUnit = None
def __init__(self, **kwargs): @@ -107,6 +108,7 @@ def getTaskObject(klass): self._fsck = getTaskObject(self._fsckClass) self._mkfs = getTaskObject(self._mkfsClass) self._readlabel = getTaskObject(self._readlabelClass) + self._sizeinfo = getTaskObject(self._sizeinfoClass) self._sync = getTaskObject(self._syncClass) self._writelabel = getTaskObject(self._writelabelClass)
@@ -246,7 +248,7 @@ def updateSizeInfo(self): finally: # try to gather current size info anyway self._current_info = self._getFSInfo() - self._size = self._getExistingSize(info=self._current_info) + self._size = self._getExistingSize() self._minSize = self._size # default to current size # We absolutely need a current size to enable resize. To shrink the # filesystem we need a real minimum size provided by the resize @@ -279,85 +281,19 @@ def _getFSInfo(self):
return buf
- def _getExistingSize(self, info=None): + def _getExistingSize(self): """ Determine the size of this filesystem.
- Filesystem must exist. Each filesystem varies, but the general - procedure is to run the filesystem dump or info utility and read - the block size and number of blocks for the filesystem - and compute megabytes from that. - - The loop that reads the output from the infofsProg is meant - to be simple, but take in to account variations in output. - The general procedure: - 1) Capture output from infofsProg. - 2) Iterate over each line of the output: - a) Trim leading and trailing whitespace. - b) Break line into fields split on ' ' - c) If line begins with any of the strings in - _existingSizeFields, start at the end of - fields and take the first one that converts - to an int. Store this in the values list. - d) Repeat until the values list length equals - the _existingSizeFields length. - 3) If the length of the values list equals the length - of _existingSizeFields, compute the size of this - filesystem by multiplying all of the values together - to get bytes, then convert to megabytes. Return - this value. - 4) If we were unable to capture all fields, return 0. - - The caller should catch exceptions from this method. Any - exception raised indicates a need to change the fields we - are looking for, the command to run and arguments, or - something else. If you catch an exception from this method, - assume the filesystem cannot be resized. - - :keyword info: filesystem info buffer - :type info: str (output of :attr:`infofsProg`) - :returns: size of existing fs in MiB. - :rtype: float. + :returns: size of existing fs + :rtype: :class:`~.size.Size` """ - if not self._existingSizeFields: - return Size(0) - size = Size(0) - if self.exists: - try: - values = [] - for line in info.splitlines(): - found = False - - line = line.strip() - tmp = line.split() - tmp.reverse() - - for field in self._existingSizeFields: - if line.startswith(field): - for subfield in tmp: - try: - values.append(int(subfield)) - found = True - break - except ValueError: - continue - - if found: - break - - if len(values) == len(self._existingSizeFields): - break - - if len(values) != len(self._existingSizeFields): - return Size(0) - - size = 1 - for value in values: - size *= value
- size = Size(size) - except Exception: # pylint: disable=broad-except - log_exception_info(log.error, "failed to obtain size of filesystem on %s", [self.device]) + if self._sizeinfo: + try: + size = self._sizeinfo.doTask() + except FSError as e: + log.error("failed to obtain size of filesystem on %s: %s", self.device, e)
return size
@@ -854,8 +790,8 @@ class Ext2FS(FS): _infoClass = fsinfo.Ext2FSInfo _mkfsClass = fsmkfs.Ext2FSMkfs _readlabelClass = fsreadlabel.Ext2FSReadLabel + _sizeinfoClass = fssize.Ext2FSSize _writelabelClass = fswritelabel.Ext2FSWriteLabel - _existingSizeFields = ["Block count:", "Block size:"] _resizefsUnit = MiB partedSystem = fileSystemType["ext2"]
@@ -1082,8 +1018,8 @@ class JFS(FS): _check = True _infoClass = fsinfo.JFSInfo _mkfsClass = fsmkfs.JFSMkfs + _sizeinfoClass = fssize.JFSSize _writelabelClass = fswritelabel.JFSWriteLabel - _existingSizeFields = ["Physical block size:", "Aggregate size:"] partedSystem = fileSystemType["jfs"]
@property @@ -1107,8 +1043,8 @@ class ReiserFS(FS): _packages = ["reiserfs-utils"] _infoClass = fsinfo.ReiserFSInfo _mkfsClass = fsmkfs.ReiserFSMkfs + _sizeinfoClass = fssize.ReiserFSSize _writelabelClass = fswritelabel.ReiserFSWriteLabel - _existingSizeFields = ["Count of blocks on the device:", "Blocksize:"] partedSystem = fileSystemType["reiserfs"]
@property @@ -1132,9 +1068,9 @@ class XFS(FS): _infoClass = fsinfo.XFSInfo _mkfsClass = fsmkfs.XFSMkfs _readlabelClass = fsreadlabel.XFSReadLabel + _sizeinfoClass = fssize.XFSSize _syncClass = fssync.XFSSync _writelabelClass = fswritelabel.XFSWriteLabel - _existingSizeFields = ["dblocks =", "blocksize ="] partedSystem = fileSystemType["xfs"]
register_device_format(XFS) @@ -1216,8 +1152,8 @@ class NTFS(FS): _infoClass = fsinfo.NTFSInfo _mkfsClass = fsmkfs.NTFSMkfs _readlabelClass = fsreadlabel.NTFSReadLabel + _sizeinfoClass = fssize.NTFSSize _writelabelClass = fswritelabel.NTFSWriteLabel - _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"] _resizefsUnit = B partedSystem = fileSystemType["ntfs"]
@@ -1376,6 +1312,7 @@ class TmpFS(NoDevFS): # in the regard that the format is automatically created # once mounted _formattable = True + _sizeinfoClass = fssize.TmpFSSize
def __init__(self, **kwargs): NoDevFS.__init__(self, **kwargs) @@ -1408,31 +1345,6 @@ def destroy(self, *args, **kwargs): """ pass
- def _getExistingSize(self, info=None): - """ Get current size of tmpfs filesystem using df. - - :param NoneType info: a dummy parameter - :rtype: Size - :returns: the current size of the filesystem, 0 if not found. - """ - if not self.status: - return Size(0) - - df = ["df", self.systemMountpoint, "--output=size"] - try: - (ret, out) = util.run_program_and_capture_output(df) - except OSError: - return Size(0) - - if ret: - return Size(0) - - lines = out.split() - if len(lines) != 2 or lines[0] != "1K-blocks": - return Size(0) - - return Size("%s KiB" % lines[1]) - @property def mountable(self): return True
From: mulhern amulhern@redhat.com
This way, tasks can know what the kernel filesystems are if they need to. Move it up a directory, so tasks and formats can share.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/devicetree.py | 3 ++- blivet/formats/fs.py | 14 ++------------ blivet/fslib.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 blivet/fslib.py
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 5a7aa0c..3320c1d 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -31,6 +31,7 @@ from .deviceaction import ActionDestroyDevice, ActionDestroyFormat from .devices import BTRFSDevice, DASDDevice, NoDevice, PartitionDevice from . import formats +from .fslib import nodev_filesystems from .devicelibs import lvm from .devicelibs import edd from . import udev @@ -970,7 +971,7 @@ def handleNodevFilesystems(self): except ValueError: log.error("failed to parse /proc/mounts line: %s", line) continue - if fstype in formats.fs.nodev_filesystems: + if fstype in nodev_filesystems: if not flags.include_nodev: continue
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 3d7cae4..101c4c3 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -41,21 +41,11 @@ from .. import udev from ..mounts import mountsCache
+from ..fslib import kernel_filesystems, update_kernel_filesystems + import logging log = logging.getLogger("blivet")
-kernel_filesystems = [] -nodev_filesystems = [] - -def update_kernel_filesystems(): - for line in open("/proc/filesystems").readlines(): - fields = line.split() - kernel_filesystems.append(fields[-1]) - if fields[0] == "nodev": - nodev_filesystems.append(fields[-1]) - -update_kernel_filesystems() - class FS(DeviceFormat): """ Filesystem base class. """ _type = "Abstract Filesystem Class" # fs type name diff --git a/blivet/fslib.py b/blivet/fslib.py new file mode 100644 index 0000000..092439d --- /dev/null +++ b/blivet/fslib.py @@ -0,0 +1,34 @@ +# fslib.py +# Library to support filesystem classes. +# +# 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): Dave Lehman dlehman@redhat.com +# David Cantrell dcantrell@redhat.com +# Anne Mulhern amulhern@redhat.com + +kernel_filesystems = [] +nodev_filesystems = [] + +def update_kernel_filesystems(): + for line in open("/proc/filesystems").readlines(): + fields = line.split() + kernel_filesystems.append(fields[-1]) + if fields[0] == "nodev": + nodev_filesystems.append(fields[-1]) + +update_kernel_filesystems()
From: mulhern amulhern@redhat.com
Removed functionality now split out into other classes out of fslabeling classes.
Cleaned up FS so that it uses reading and writing where appropriate and _labelfs almost not at all.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/formats/fs.py | 2 +- blivet/tasks/fslabel.py | 205 -------------------------------------- blivet/tasks/fslabeling.py | 48 --------- tests/tasks_test/labeling_test.py | 6 +- 4 files changed, 4 insertions(+), 257 deletions(-) delete mode 100644 blivet/tasks/fslabel.py
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 2ac8885..c5d39e9 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -165,7 +165,7 @@ def labeling(self):
:rtype: bool """ - return self._labelfs is not None + return (self._mkfs is not None and self._mkfs.labels) or self._writelabel is not None
def relabels(self): """Returns True if it is possible to relabel this filesystem diff --git a/blivet/tasks/fslabel.py b/blivet/tasks/fslabel.py deleted file mode 100644 index 81c18e6..0000000 --- a/blivet/tasks/fslabel.py +++ /dev/null @@ -1,205 +0,0 @@ -# fslabel.py -# Filesystem labeling classes for anaconda's storage configuration module. -# -# Copyright (C) 2013 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 -import re - -from six import add_metaclass - -from .. import errors - -@add_metaclass(abc.ABCMeta) -class FSLabelApp(object): - """An abstract class that represents actions associated with a - filesystem's labeling application. - """ - - name = abc.abstractproperty( - doc="The name of the filesystem labeling application.") - - reads = abc.abstractproperty( - doc="Whether this application can read a label as well as write one.") - - _label_regex = abc.abstractproperty( - doc="Matches the string output by the label reading application.") - - @abc.abstractmethod - def _writeLabelArgs(self, fs): - """Returns a list of the arguments for writing a label. - - :param FS fs: a filesystem object - - :return: the arguments - :rtype: list of str - - It can be assumed in this function that fs.label is a str. - """ - raise NotImplementedError - - def setLabelCommand(self, fs): - """Get the command to label the filesystem. - - :param FS fs: a filesystem object - :return: the command - :rtype: list of str - - Raises an exception if fs.label is None. - """ - if fs.label is None: - raise errors.FSError("makes no sense to write a label when accepting default label") - return [self.name] + self._writeLabelArgs(fs) - - @abc.abstractmethod - def _readLabelArgs(self, fs): - """Returns a list of arguments for reading a label. - - :param FS fs: a filesystem object - :return: the arguments - :rtype: list of str - """ - raise NotImplementedError - - def readLabelCommand(self, fs): - """Get the command to read the filesystem label. - - :param FS fs: a filesystem object - :return: the command - :rtype: list of str - - Raises an FSError if this application can not read the label. - """ - if not self.reads: - raise errors.FSError("Application %s can not read the filesystem label." % self.name) - return [self.name] + self._readLabelArgs(fs) - - def extractLabel(self, labelstr): - """Extract the label from an output string. - - :param str labelstr: the string containing the label information - - :return: the label - :rtype: str - - Raises an FSError if the label can not be extracted. - """ - if not self.reads or self._label_regex is None: - raise errors.FSError("Unknown format for application %s" % self.name) - match = re.match(self._label_regex, labelstr) - if match is None: - raise errors.FSError("Unknown format for application %s" % self.name) - return match.group('label') - - -class E2Label(FSLabelApp): - """Application used by ext2 and its descendants.""" - - name = "e2label" - reads = True - - _label_regex = r'(?P<label>.*)' - - def _writeLabelArgs(self, fs): - return [fs.device, fs.label] - - def _readLabelArgs(self, fs): - return [fs.device] - -E2Label = E2Label() - -class DosFsLabel(FSLabelApp): - """Application used by FATFS.""" - - name = "dosfslabel" - reads = True - - _label_regex = r'(?P<label>.*)' - - def _writeLabelArgs(self, fs): - return [fs.device, fs.label] - - def _readLabelArgs(self, fs): - return [fs.device] - -DosFsLabel = DosFsLabel() - -class JFSTune(FSLabelApp): - """Application used by JFS.""" - - name = "jfs_tune" - reads = False - - _label_regex = property(lambda s: None) - - def _writeLabelArgs(self, fs): - return ["-L", fs.label, fs.device] - - def _readLabelArgs(self, fs): - raise NotImplementedError - -JFSTune = JFSTune() - -class ReiserFSTune(FSLabelApp): - """Application used by ReiserFS.""" - - name = "reiserfstune" - reads = False - - _label_regex = None - - def _writeLabelArgs(self, fs): - return ["-l", fs.label, fs.device] - - def _readLabelArgs(self, fs): - raise NotImplementedError - -ReiserFSTune = ReiserFSTune() - -class XFSAdmin(FSLabelApp): - """Application used by XFS.""" - - name = "xfs_admin" - reads = True - - _label_regex = r'label = "(?P<label>.*)"' - - def _writeLabelArgs(self, fs): - return ["-L", fs.label if fs.label != "" else "--", fs.device] - - def _readLabelArgs(self, fs): - return ["-l", fs.device] - -XFSAdmin = XFSAdmin() - -class NTFSLabel(FSLabelApp): - """Application used by NTFS.""" - - name = "ntfslabel" - reads = True - - _label_regex = r'(?P<label>.*)' - - def _writeLabelArgs(self, fs): - return [fs.device, fs.label] - - def _readLabelArgs(self, fs): - return [fs.device] - -NTFSLabel = NTFSLabel() diff --git a/blivet/tasks/fslabeling.py b/blivet/tasks/fslabeling.py index 0abeaf7..3d2faa3 100644 --- a/blivet/tasks/fslabeling.py +++ b/blivet/tasks/fslabeling.py @@ -23,8 +23,6 @@
from six import add_metaclass
-from . import fslabel - @add_metaclass(abc.ABCMeta) class FSLabeling(object): """An abstract class that represents filesystem labeling actions. @@ -33,9 +31,6 @@ class FSLabeling(object): default_label = abc.abstractproperty( doc="Default label set on this filesystem at creation.")
- label_app = abc.abstractproperty( - doc="Post creation filesystem labeling application.") - @abc.abstractmethod def labelFormatOK(self, label): """Returns True if this label is correctly formatted for this @@ -46,74 +41,41 @@ def labelFormatOK(self, label): """ raise NotImplementedError
- @abc.abstractmethod - def labelingArgs(self, label): - """Returns the arguments for writing the label during filesystem - creation. These arguments are intended to be passed to the - appropriate mkfs application. - - :param str label: the label to use - :return: the arguments - :rtype: list of str - """ - raise NotImplementedError - - class Ext2FSLabeling(FSLabeling):
default_label = "" - label_app = fslabel.E2Label
def labelFormatOK(self, label): return len(label) < 17
- def labelingArgs(self, label): - return ["-L", label] - class FATFSLabeling(FSLabeling):
default_label = "NO NAME" - label_app = fslabel.DosFsLabel
def labelFormatOK(self, label): return len(label) < 12
- def labelingArgs(self, label): - return ["-n", label] - class JFSLabeling(FSLabeling):
default_label = "" - label_app = fslabel.JFSTune
def labelFormatOK(self, label): return len(label) < 17
- def labelingArgs(self, label): - return ["-L", label] - class ReiserFSLabeling(FSLabeling):
default_label = "" - label_app = fslabel.ReiserFSTune
def labelFormatOK(self, label): return len(label) < 17
- def labelingArgs(self, label): - return ["-l", label] - class XFSLabeling(FSLabeling):
default_label = "" - label_app = fslabel.XFSAdmin
def labelFormatOK(self, label): return ' ' not in label and len(label) < 13
- def labelingArgs(self, label): - return ["-L", label] - class HFSLabeling(FSLabeling):
default_label = "Untitled" @@ -122,9 +84,6 @@ class HFSLabeling(FSLabeling): def labelFormatOK(self, label): return ':' not in label and len(label) < 28 and len(label) > 0
- def labelingArgs(self, label): - return ["-l", label] - class HFSPlusLabeling(FSLabeling):
default_label = "Untitled" @@ -133,16 +92,9 @@ class HFSPlusLabeling(FSLabeling): def labelFormatOK(self, label): return ':' not in label and 0 < len(label) < 129
- def labelingArgs(self, label): - return ["-v", label] - class NTFSLabeling(FSLabeling):
default_label = "" - label_app = fslabel.NTFSLabel
def labelFormatOK(self, label): return len(label) < 129 - - def labelingArgs(self, label): - return ["-L", label] diff --git a/tests/tasks_test/labeling_test.py b/tests/tasks_test/labeling_test.py index c1945d3..f3d489f 100755 --- a/tests/tasks_test/labeling_test.py +++ b/tests/tasks_test/labeling_test.py @@ -71,18 +71,18 @@ def testGetLabelArgs(self):
# ReiserFS uses a -l flag reiserfs = self.fs["reiserfs"] - self.assertEqual(reiserfs._labelfs.label_app.setLabelCommand(reiserfs), + self.assertEqual(reiserfs._writelabel._setCommand(), ["reiserfstune", "-l", "myfs", "/dev"], msg="reiserfs")
# JFS, XFS use a -L flag lflag_classes = [fs.JFS, fs.XFS] for name, klass in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in lflag_classes)]: - self.assertEqual(klass._labelfs.label_app.setLabelCommand(v), [klass._labelfs.label_app.name, "-L", "myfs", "/dev"], msg=name) + self.assertEqual(klass._writelabel._setCommand(), [klass._writelabel.app_name, "-L", "myfs", "/dev"], msg=name)
# Ext2FS and descendants and FATFS do not use a flag noflag_classes = [fs.Ext2FS, fs.FATFS] for name, klass in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in noflag_classes)]: - self.assertEqual(klass._labelfs.label_app.setLabelCommand(klass), [klass._labelfs.label_app.name, "/dev", "myfs"], msg=name) + self.assertEqual(klass._writelabel._setCommand(), [klass._writelabel.app_name, "/dev", "myfs"], msg=name)
# all of the remaining are non-labeling so will accept any label label = "Houston, we have a problem!"
From: mulhern amulhern@redhat.com
Previously, HFSPlusFSCK always considered fsck operation to have succeeded. Now it checkes for an error and behaves essentially like the other FSCK tasks.
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsck.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/blivet/tasks/fsck.py b/blivet/tasks/fsck.py index 2ada93c..cbf6d7c 100644 --- a/blivet/tasks/fsck.py +++ b/blivet/tasks/fsck.py @@ -159,12 +159,20 @@ def _errorMessage(self, rc): return "\n".join(msgs) or None
class HFSPlusFSCK(FSCK): + _fsckErrors = {3: "Quick check found a dirty filesystem; no repairs done.", + 4: "Root filesystem was dirty. System should be rebooted.", + 8: "Corrupt filesystem, repairs did not succeed.", + 47: "Major error found; no repairs attempted."} app_name = "fsck.hfsplus" options = []
def _errorMessage(self, rc): - # pylint: disable=unused-argument - return None + if rc < 1: + return None + try: + return self._fsckErrors[rc] + except KeyError: + return _UNKNOWN_RC_MSG % rc
class NTFSFSCK(FSCK): app_name = "ntfsresize"
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- tests/pylint/pointless-override.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/pylint/pointless-override.py b/tests/pylint/pointless-override.py index 6f7160c..e5f0fd8 100644 --- a/tests/pylint/pointless-override.py +++ b/tests/pylint/pointless-override.py @@ -162,7 +162,8 @@ def check_equal(cls, node, other): return len(node.elts) == len(other.elts) and \ all(cls.check_equal(n, o) for (n, o) in zip(node.elts, other.elts)) if isinstance(node, astroid.Dict): - return len(node.items) == len(other.items) + return len(node.items) == len(other.items) and \ + all(cls.check_equal(n, o) for (n, o) in zip(sorted(node.items), sorted(other.items))) if isinstance(node, astroid.CallFunc): if type(node.func) != type(other.func): return False @@ -180,7 +181,8 @@ def check_equal(cls, node, other):
return node.starargs == other.starargs and \ node.kwargs == other.kwargs and \ - all(cls.check_equal(n, o) for (n, o) in zip(node.args, other.args)) + node.args == other.args + return False
@staticmethod
From: mulhern amulhern@redhat.com
Signed-off-by: mulhern amulhern@redhat.com --- blivet/tasks/fsresize.py | 186 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 blivet/tasks/fsresize.py
diff --git a/blivet/tasks/fsresize.py b/blivet/tasks/fsresize.py new file mode 100644 index 0000000..3027dc1 --- /dev/null +++ b/blivet/tasks/fsresize.py @@ -0,0 +1,186 @@ +# fsresize.py +# Filesystem resizing classes. +# +# 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 +import os + +from six import add_metaclass + +from ..errors import FSError +from ..size import B, KiB, MiB, GiB, KB, MB, GB +from ..import util + +from . import availability +from . import task + +@add_metaclass(abc.ABCMeta) +class FSResize(task.Task): + """ An abstract class for resizing a filesystem. """ + + description = "resize filesystem" + + app_name = abc.abstractproperty( + doc="The name of the filesystem resizing application.") + + unit = abc.abstractproperty(doc="Resize unit.") + + args = abc.abstractproperty(doc="Resize arguments.") + + @classmethod + def _app(cls): + return availability.Application(availability.Path(), cls.app_name) + + def __init__(self, an_fs): + """ Initializer. + + :param FS an_fs: a filesystem object + """ + self.fs = an_fs + + # TASK methods + + @classmethod + def available(cls): + return cls._app().available + + @property + def unavailable(self): + if not self._app().available: + return "application %s not available." % self._app() + + return False + + @property + def unready(self): + if not self.fs.exists: + return "filesystem has not been created" + + if not os.path.exists(self.fs.device): + return "device does not exist" + + return False + + @property + def unable(self): + return False + + # IMPLEMENTATION methods + + @abc.abstractmethod + def sizeSpec(self): + """ Returns a string specification for the target size of the command. + :returns: size specification + :rtype: str + """ + raise NotImplementedError() + + def _resizeCommand(self): + return [str(self._app())] + self.args + + def doTask(self): + """ Resize the device. + + :raises FSError: on failure + """ + error_msg = self.impossible + if error_msg: + raise FSError(error_msg) + + try: + ret = util.run_program(self._resizeCommand()) + except OSError as e: + raise FSError(e) + + if ret: + raise FSError("resize failed: %s" % ret) + +class Ext2FSResize(FSResize): + app_name = "resize2fs" + unit = MiB + # No unit specifier is interpreted not as bytes, but block size + size_fmt = {KiB: "%dK", MiB: "%dM", GiB: "%dG"}[unit] + + def sizeSpec(self): + return self.size_fmt % self.fs.targetSize.convertTo(self.unit) + + @property + def args(self): + return ["-p", self.fs.device, self.sizeSpec()] + +class NTFSResize(FSResize): + app_name = "ntfsresize" + unit = B + size_fmt = {B: "%d", KB: "%dK", MB: "%dM", GB: "%dG"}[unit] + + def sizeSpec(self): + return self.size_fmt % self.fs.targetSize.convertTo(self.unit) + + @property + def args(self): + return [ + "-ff", # need at least two 'f's to fully supress interaction + "-s", self.sizeSpec(), + self.fs.device + ] + +class TmpFSResize(FSResize): + + app_name = "mount" + unit = MiB + size_fmt = {KiB: "%dk", MiB: "%dm", GiB: "%dg"}[unit] + + def sizeSpec(self): + return "size=%s" % (self.size_fmt % self.fs.targetSize.convertTo(self.unit)) + + @property + def unready(self): + # TmpFS does not require a device in order to be mounted. + if not self.fs.exists: + return "filesystem has not been created" + + return False + + @property + def args(self): + # This is too closely mixed in w/ TmpFS object, due to the + # fact that resizing is done by mounting and that the options are + # therefore mount options. The situation is hard to avoid, though. + opts = self.fs.mountopts or ",".join(self.fs._mount.options) + options = ("remount", opts, self.sizeSpec()) + return ['-o', ",".join(options), self.fs._type, self.fs.systemMountpoint] + +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() + + @property + def size_fmt(self): + raise NotImplementedError() +>>>>>>> 8c59753... with resize
A few things I thought of:
* The last patch isn't meant to be taken seriously, I just wanted to make sure that the systemMountpoint() related errors in action_tests.py didn't mask any errors introduced by my changes to DeviceAction classes. * Unittests should be updated to be environment aware, too. I havn't done that yet, although some already are. * It would be nice to profile filesystem formats regression tests on current version and this branch, to see if there were some noticeable timing changes. * It would be nice to test these change on a few virtual machines with a selection of different things uninstalled, and to make those tests an automatic part of blivet's tests if possible.
Closed.
anaconda-patches@lists.fedorahosted.org