[blivet:rhel7/master 1/3] Changes to the handling of filesystem labeling (#1038590)

mulhern amulhern at redhat.com
Fri Dec 20 23:12:51 UTC 2013


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 at 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 at 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



More information about the anaconda-patches mailing list