Resolves: rhbz#1038590
* Change FS.label to a property
- Want to guard setting the label format, so it matches the requirements of
the labelfs program that will use it.
- Don't set label property in writeLabel. writeLabel is only called from
doFormat, in a context where self.label is guaranteed to be set.
* Removed _defaultLabelOptions field and defaultLabelOptions property. The
property is only used in this file, and it is only used by _getLabelArgs.
The property is set from the field.
* Remove label parameter from writeLabel method
- writeLabel is only called in one place and in that place self.label is
guaranteed to be set, so this change does not change the behavior of the
program.
- Since writeLabel uses the label in self.label instead of a parameter
it is guaranteed that this value has passed the checks in self._setLabel.
- Other methods that are only called by writeLabel are changed in the same way.
- writeLabel now throws an exception if it can not set the label because there
is no program to set it. This does not change the behavior of this program.
* Factor out filesystem relabeling functionality into a separate package
* Add the ability to read the filesystem label
- In some cases, the application that is used to write the label is also
used to read the label.
- This is certainly useful for testing, and may come in handy otherwise.
Signed-off-by: mulhern <amulhern(a)redhat.com>
---
blivet/formats/fs.py | 127 ++++++++++++++++-------
blivet/formats/fslabel.py | 216 ++++++++++++++++++++++++++++++++++++++++
tests/formats_test/misc_test.py | 94 +++++++++++++----
3 files changed, 380 insertions(+), 57 deletions(-)
create mode 100644 blivet/formats/fslabel.py
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
index 340a6e3..47028ae 100644
--- a/blivet/formats/fs.py
+++ b/blivet/formats/fs.py
@@ -28,6 +28,7 @@ import sys
import tempfile
import selinux
+from . import fslabel
from ..errors import *
from . import DeviceFormat, register_device_format
from .. import util
@@ -118,13 +119,12 @@ class FS(DeviceFormat):
_mkfs = "" # mkfs utility
_modules = [] # kernel modules required for support
_resizefs = "" # resize utility
- _labelfs = "" # labeling utility
+ _labelfs = None # labeling utility
_fsck = "" # fs check utility
_fsckErrors = {} # fs check command error codes & msgs
_infofs = "" # fs info utility
_defaultFormatOptions = [] # default options passed to mkfs
_defaultMountOptions = ["defaults"] # default options passed to mount
- _defaultLabelOptions = []
_defaultCheckOptions = []
_defaultInfoOptions = []
_existingSizeFields = []
@@ -192,6 +192,41 @@ class FS(DeviceFormat):
"mountable": self.mountable})
return d
+ def _setLabel(self, label):
+ """Sets the label for this filesystem.
+
+ :param label: the label for this filesystem
+ :type label: str or None
+
+ Raises a FSError if this label is unacceptably formatted for this
+ filesystem.
+
+ Note that some filesystems do not possess a label, so this method
+ always accept the value None for label.
+
+ This method is not intended to be overridden.
+ """
+ if label is None:
+ self._label = None
+ elif label == "":
+ raise FSError("Empty filesystem label not permitted.")
+ elif self._labelfs == None or self._labelfs.labelFormatOK(label):
+ self._label = label
+ else:
+ raise FSError("Filesystem label '%s' is incorrectly formatted
for %s." % (label, self._labelfs.name))
+
+ def _getLabel(self):
+ """The label for this filesystem.
+
+ :return: the label for this filesystsm
+ :rtype: str
+
+ This method is not intended to be overridden.
+ """
+ return self._label
+
+ label = property(_getLabel, _setLabel, doc="this filesystem's label")
+
def _setTargetSize(self, newsize):
""" Set a target size for this filesystem. """
if not self.exists:
@@ -400,8 +435,8 @@ class FS(DeviceFormat):
self.exists = True
self.notifyKernel()
- if self.label:
- self.writeLabel(self.label)
+ if self._labelfs:
+ self.writeLabel()
@property
def resizeArgs(self):
@@ -629,29 +664,59 @@ class FS(DeviceFormat):
self._mountpoint = None
- def _getLabelArgs(self, label):
- argv = []
- argv.extend(self.defaultLabelOptions)
- argv.extend([self.device, label])
- return argv
+ def readLabel(self):
+ """Read this filesystem's label.
+
+ :return: the filesystem's label
+ :rtype: str
+
+ Raises a FSError if the label can not be read.
- def writeLabel(self, label):
- """ Create a label for this filesystem. """
+ Returns None if there is no label.
+ """
if not self.exists:
raise FSError("filesystem has not been created")
- if not self.labelfsProg:
+ if not os.path.exists(self.device):
+ raise FSError("device does not exist")
+
+ if not self._labelfs or not self._labelfs.reads:
+ raise FSError("no application to read label for filesystem %s" %
self.type)
+
+ (rc, out) =
util.run_program_and_capture_output(self._labelfs.readLabelCommand(self))
+ if rc:
+ raise FSError("read label failed")
+
+ label = out.strip()
+
+ if label == "":
+ return None
+ else:
+ return self._labelfs.extractLabel(label)
+
+ def writeLabel(self):
+ """Create a label on this filesystem.
+
+ Does nothing if self.label is None.
+
+ Raises a FSError if the label can not be set.
+ """
+ if not self.label:
return
+ if not self.exists:
+ raise FSError("filesystem has not been created")
+
+ if not self._labelfs:
+ raise FSError("no application to set label for filesystem %s" %
self.type)
+
if not os.path.exists(self.device):
raise FSError("device does not exist")
- argv = self._getLabelArgs(label)
- rc = util.run_program([self.labelfsProg] + argv)
+ rc = util.run_program(self._labelfs.setLabelCommand(self))
if rc:
raise FSError("label failed")
- self.label = label
self.notifyKernel()
def _getRandomUUID(self):
@@ -682,8 +747,11 @@ class FS(DeviceFormat):
@property
def labelfsProg(self):
- """ Program used to manage labels for this filesystem type.
"""
- return self._labelfs
+ """ Program used to manage labels for this filesystem type.
+
+ May be None if no such program exists.
+ """
+ return self._labelfs.name if self._labelfs else None
@property
def infofsProg(self):
@@ -741,12 +809,6 @@ class FS(DeviceFormat):
return self._defaultMountOptions[:]
@property
- def defaultLabelOptions(self):
- """ Default options passed to labeler for this filesystem type.
"""
- # return a copy to prevent modification
- return self._defaultLabelOptions[:]
-
- @property
def defaultCheckOptions(self):
""" Default options passed to checker for this filesystem type.
"""
# return a copy to prevent modification
@@ -820,7 +882,7 @@ class Ext2FS(FS):
_mkfs = "mke2fs"
_modules = ["ext2"]
_resizefs = "resize2fs"
- _labelfs = "e2label"
+ _labelfs = fslabel.E2Label()
_fsck = "e2fsck"
_fsckErrors = {4: _("File system errors left uncorrected."),
8: _("Operational error."),
@@ -984,7 +1046,7 @@ class FATFS(FS):
_type = "vfat"
_mkfs = "mkdosfs"
_modules = ["vfat"]
- _labelfs = "dosfslabel"
+ _labelfs = fslabel.DosFsLabel()
_fsck = "dosfsck"
_fsckErrors = {1: _("Recoverable errors have been detected or dosfsck has
"
"discovered an internal inconsistency."),
@@ -1114,9 +1176,8 @@ class JFS(FS):
_type = "jfs"
_mkfs = "mkfs.jfs"
_modules = ["jfs"]
- _labelfs = "jfs_tune"
+ _labelfs = fslabel.JFSTune()
_defaultFormatOptions = ["-q"]
- _defaultLabelOptions = ["-L"]
_maxLabelChars = 16
_maxSize = 8 * 1024 * 1024
_formattable = True
@@ -1146,10 +1207,9 @@ class ReiserFS(FS):
_type = "reiserfs"
_mkfs = "mkreiserfs"
_resizefs = "resize_reiserfs"
- _labelfs = "reiserfstune"
+ _labelfs = fslabel.ReiserFSTune()
_modules = ["reiserfs"]
_defaultFormatOptions = ["-f", "-f"]
- _defaultLabelOptions = ["-l"]
_maxLabelChars = 16
_maxSize = 16 * 1024 * 1024
_formattable = True
@@ -1185,9 +1245,8 @@ class XFS(FS):
_type = "xfs"
_mkfs = "mkfs.xfs"
_modules = ["xfs"]
- _labelfs = "xfs_admin"
+ _labelfs = fslabel.XFSAdmin()
_defaultFormatOptions = ["-f"]
- _defaultLabelOptions = ["-L"]
_maxLabelChars = 16
_maxSize = 16 * 1024 * 1024 * 1024 * 1024
_formattable = True
@@ -1202,12 +1261,6 @@ class XFS(FS):
_existingSizeFields = ["dblocks =", "blocksize ="]
partedSystem = fileSystemType["xfs"]
- def _getLabelArgs(self, label):
- argv = []
- argv.extend(self.defaultLabelOptions)
- argv.extend([label, self.device])
- return argv
-
def sync(self, root='/'):
""" Ensure that data we've written is at least in the
journal.
diff --git a/blivet/formats/fslabel.py b/blivet/formats/fslabel.py
new file mode 100644
index 0000000..452bc78
--- /dev/null
+++ b/blivet/formats/fslabel.py
@@ -0,0 +1,216 @@
+# fslabel.py
+# Filesystem labelling classes for anaconda's storage configuration module.
+#
+# Copyright (C) 2009 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(a)redhat.com>
+
+import abc
+import re
+
+class FSLabelApp(object):
+ """An abstract class that represents actions associated with a
+ filesystem's labeling application.
+ """
+
+ __metaclass__ = abc.ABCMeta
+
+ @property
+ def name(self):
+ """The name of the filesystem labeling application.
+
+ :rtype: str
+ """
+ return self._name
+
+ @abc.abstractproperty
+ def reads(self):
+ """Returns True if this app can also read a label.
+
+ :rtype: bool
+ """
+ raise NotImplementedError
+
+ @abc.abstractmethod
+ def labelFormatOK(self, label):
+ """Returns True if this label is correctly formatted for this
+ filesystem labelling application, otherwise False.
+
+ :param str label: the label for this filesystem
+ :rtype: bool
+ """
+ raise NotImplementedError
+
+ @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
+ """
+ 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
+ """
+ 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 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:
+ raise FSError("Unknown format for application %s" % self.name)
+ match = re.match(self._labelstrRegex(), labelstr)
+ if match is None:
+ raise FSError("Unknown format for application %s" % self.name)
+ return match.group('label')
+
+class E2Label(FSLabelApp):
+ """Application used by ext2 and its descendants."""
+
+ _name = "e2label"
+
+ def reads(self):
+ return True
+
+ def _writeLabelArgs(self, fs):
+ return [fs.device, fs.label]
+
+ def labelFormatOK(self, label):
+ return len(label) < 17
+
+ def _readLabelArgs(self, fs):
+ return [fs.device]
+
+ def _labelstrRegex(self):
+ return r'(?P<label>.*)'
+
+class DosFsLabel(FSLabelApp):
+ """Application used by FATFS."""
+
+ _name = "dosfslabel"
+
+ def reads(self):
+ return True
+
+ def _writeLabelArgs(self, fs):
+ return [fs.device, fs.label]
+
+ def labelFormatOK(self, label):
+ return len(label) < 12
+
+ def _readLabelArgs(sefl, fs):
+ return [fs.device]
+
+ def _labelstrRegex(self):
+ return r'(?P<label>.*)'
+
+class JFSTune(FSLabelApp):
+ """Application used by JFS."""
+
+ _name = "jfs_tune"
+
+ def reads(self):
+ return False
+
+ def _writeLabelArgs(self, fs):
+ return ["-L", fs.label, fs.device]
+
+ def labelFormatOK(self, label):
+ return len(label) < 17
+
+ def _readLabelArgs(sefl, fs):
+ raise NotImplementedError
+
+ def _labelstrRegex(self):
+ raise NotImplementedError
+
+class ReiserFSTune(FSLabelApp):
+ """Application used by ReiserFS."""
+
+ _name = "reiserfstune"
+
+ def reads(self):
+ return False
+
+ def _writeLabelArgs(self, fs):
+ return ["-l", fs.label, fs.device]
+
+ def labelFormatOK(self, label):
+ return len(label) < 17
+
+ def _readLabelArgs(self, fs):
+ raise NotImplementedError
+
+ def _labelstrRegex(self):
+ raise NotImplementedError
+
+class XFSAdmin(FSLabelApp):
+ """Application used by XFS."""
+
+ _name = "xfs_admin"
+
+ def reads(self):
+ return True
+
+ def _writeLabelArgs(self, fs):
+ return ["-L", fs.label, fs.device]
+
+ def labelFormatOK(self, label):
+ return ' ' not in label and len(label) < 13
+
+ def _readLabelArgs(sefl, fs):
+ return ["-l", fs.device]
+
+ def _labelstrRegex(self):
+ return r'label = "(?P<label>.*)"'
diff --git a/tests/formats_test/misc_test.py b/tests/formats_test/misc_test.py
index 816f756..cae20eb 100755
--- a/tests/formats_test/misc_test.py
+++ b/tests/formats_test/misc_test.py
@@ -4,6 +4,60 @@ import unittest
from blivet.formats import device_formats
import blivet.formats.fs as fs
+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.assertRaisesRegexp(fs.FSError,
+ "Filesystem label.*incorrectly formatted",
+ fs.Ext2FS,
+ device="/dev", label="root___filesystem")
+
+ self.assertIsNotNone(fs.Ext2FS(label="root__filesystem"))
+
+ # FATFS has a maximum length of 11
+ self.assertRaisesRegexp(fs.FSError,
+ "Filesystem label.*incorrectly formatted",
+ fs.FATFS,
+ device="/dev", label="rtfilesystem")
+
+ self.assertIsNotNone(fs.FATFS(label="rfilesystem"))
+
+ # JFS has a maximum length of 16
+ self.assertRaisesRegexp(fs.FSError,
+ "Filesystem label.*incorrectly formatted",
+ fs.JFS,
+ device="/dev", label="root___filesystem")
+
+ self.assertIsNotNone(fs.JFS(label="root__filesystem"))
+
+ # ReiserFS has a maximum length of 16
+ self.assertRaisesRegexp(fs.FSError,
+ "Filesystem label.*incorrectly formatted",
+ fs.ReiserFS,
+ device="/dev", label="root___filesystem")
+
+ self.assertIsNotNone(fs.ReiserFS(label="root__filesystem"))
+
+ #XFS has a maximum length 12 and does not allow spaces
+ self.assertRaisesRegexp(fs.FSError,
+ "Filesystem label.*incorrectly formatted",
+ fs.XFS,
+ device="/dev", label="root filesyst")
+ self.assertRaisesRegexp(fs.FSError,
+ "Filesystem label.*incorrectly formatted",
+ fs.XFS,
+ device="/dev", label="root file")
+
+ self.assertIsNotNone(fs.XFS(label="root_filesys"))
+
+ # all devices are permitted to have a label of None
+ 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."""
@@ -11,36 +65,36 @@ class MethodsTestCase(unittest.TestCase):
self.fs = {}
for k, v in device_formats.items():
if issubclass(v, fs.FS) and not issubclass(v, fs.NFS):
- self.fs[k] = v(device="/dev")
+ self.fs[k] = v(device="/dev", label="myfs")
def testGetLabelArgs(self):
self.longMessage = True
- # ReiserFS is currently backwards, needs the label after the l flag
- for k, v in [(k, v) for k, v in self.fs.items() if isinstance(v, fs.ReiserFS)]:
- self.assertEqual(v._getLabelArgs("myfs"), ["-l",
"/dev", "myfs"], msg=k)
-
- # JFS is backward as well
- for k, v in [(k, v) for k, v in self.fs.items() if isinstance(v, fs.JFS)]:
- self.assertEqual(v._getLabelArgs("myfs"), ["-L",
"/dev", "myfs"], msg=k)
-
- #XFS uses a -L label
- for k, v in [(k, v) for k, v in self.fs.items() if isinstance(v, fs.XFS)]:
- self.assertEqual(v._getLabelArgs("myfs"), ["-L",
"myfs", "/dev"], msg=k)
+ # ReiserFS uses a -l flag
+ reiserfs = self.fs["reiserfs"]
+ self.assertEqual(reiserfs._labelfs.setLabelCommand(reiserfs),
+ ["reiserfstune", "-l", "myfs",
"/dev"], msg="reiserfs")
+ # JFS, XFS use a -L flag
+ lflag_classes = [fs.JFS, fs.XFS]
+ for k, v in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in
lflag_classes)]:
+ self.assertEqual(v._labelfs.setLabelCommand(v), [v._labelfs.name,
"-L", "myfs", "/dev"], msg=k)
- # All NoDeviceFSs ignore the device argument passed and set device
- # to the fs type
- for k, v in [(k, v) for k, v in self.fs.items() if isinstance(v, fs.NoDevFS)]:
- self.assertEqual(v._getLabelArgs("myfs"), [v.type,
"myfs"], msg=k)
+ # Ext2FS and descendants and FATFS do not use a flag
+ noflag_classes = [fs.Ext2FS, fs.FATFS]
+ for k, v in [(k, v) for k, v in self.fs.items() if any(isinstance(v, c) for c in
noflag_classes)]:
+ self.assertEqual(v._labelfs.setLabelCommand(v), [v._labelfs.name,
"/dev", "myfs"], msg=k)
- for k, v in [(k, v) for k, v in self.fs.items() if not (isinstance(v, fs.NoDevFS)
or isinstance(v, fs.ReiserFS) or isinstance(v, fs.XFS) or isinstance(v, fs.JFS))]:
- self.assertEqual(v._getLabelArgs("myfs"), ["/dev",
"myfs"], msg=k)
+ # all of the remaining should have no labelfsProg
+ omit_classes = [ fs.ReiserFS ] + lflag_classes + noflag_classes
+ for k, v in [(k, v) for k, v in self.fs.items() if not any(isinstance(v, c) for c
in omit_classes)]:
+ self.assertIsNone(v.labelfsProg)
def suite():
- suite1 = unittest.TestLoader().loadTestsFromTestCase(MethodsTestCase)
- return unittest.TestSuite(suite1)
+ suite1 = unittest.TestLoader().loadTestsFromTestCase(InitializationTestCase)
+ suite2 = unittest.TestLoader().loadTestsFromTestCase(MethodsTestCase)
+ return unittest.TestSuite([suite1, suite2])
if __name__ == "__main__":
--
1.8.3.1