On Tue, 2013-12-17 at 18:13 -0500, mulhern wrote:
Related: rhbz#1038590
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.
Just one question, below.
Signed-off-by: mulhern <amulhern(a)redhat.com>
---
blivet/formats/fs.py | 50 +++++++++++++++++++++++++++++++++-
tests/formats_test/misc_test.py | 59 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 106 insertions(+), 3 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
index 7796625..8c2b528 100644
--- a/blivet/formats/fs.py
+++ b/blivet/formats/fs.py
@@ -191,6 +191,30 @@ class FS(DeviceFormat):
"mountable": self.mountable})
return d
+ def _setLabel(self, label):
+ """Sets the label for this filesystem.
+
+ :param str label: the label for this filesystem
+
+ Raises a FSError if this label is unacceptably formatted for this
+ filesystem.
+
+ Note that some filesystems do not possess a label, so any method
+ that overrides this method must accept None.
+ """
+ self._label = label
+
+ def _getLabel(self):
+ """The label for this filesystem.
+
+ :return: the label for this filesystsm
+ :rtype: str
+ """
+ return self._label
+
+ label = property(lambda s: s._getLabel(), lambda s,l: s._setLabel(l),
+ doc="This filesystem's label.")
+
def _setTargetSize(self, newsize):
""" Set a target size for this filesystem. """
if not self.exists:
@@ -654,7 +678,6 @@ class FS(DeviceFormat):
if rc:
raise FSError("label failed")
- self.label = label
self.notifyKernel()
def _getRandomUUID(self):
@@ -847,6 +870,11 @@ class Ext2FS(FS):
self.errors = False
super(Ext2FS, self).__init__(*args, **kwargs)
+ def _setLabel(self, label):
+ if label is not None and (len(label) > 16 or len(label) == 0):
+ raise FSError("Filesystem label '%s' is incorrectly formatted
for %s." % (label, self.labelfsProg))
+ self._label = label
+
It's not valid to set the label to an empty string?
def _fsckFailed(self, rc):
for errorCode in self._fsckErrors.keys():
if rc & errorCode:
@@ -994,6 +1022,11 @@ class FATFS(FS):
# FIXME this should be fat32 in some cases
partedSystem = fileSystemType["fat16"]
+ def _setLabel(self, label):
+ if label is not None and (len(label) > 11 or len(label) == 0):
+ raise FSError("Filesystem label '%s' is incorrectly formatted
for %s." % (label, self.labelfsProg))
+ self._label = label
+
def _fsckFailed(self, rc):
if rc >= 1:
return True
@@ -1125,6 +1158,11 @@ class JFS(FS):
_existingSizeFields = ["Aggregate block size:", "Aggregate
size:"]
partedSystem = fileSystemType["jfs"]
+ def _setLabel(self, label):
+ if label is not None and (len(label) > 16 or len(label) == 0):
+ raise FSError("Filesystem label '%s' is incorrectly formatted
for %s." % (label, self.labelfsProg))
+ self._label = label
+
def _getLabelArgs(self, label):
return ["-L", label, self.device]
@@ -1161,6 +1199,11 @@ class ReiserFS(FS):
_existingSizeFields = ["Count of blocks on the device:",
"Blocksize:"]
partedSystem = fileSystemType["reiserfs"]
+ def _setLabel(self, label):
+ if label is not None and (len(label) > 16 or len(label) == 0):
+ raise FSError("Filesystem label '%s' is incorrectly formatted
for %s." % (label, self.labelfsProg))
+ self._label = label
+
def _getLabelArgs(self, label):
return ["-l", label, self.device]
@@ -1205,6 +1248,11 @@ class XFS(FS):
def _getLabelArgs(self, label):
return ["-L", label, self.device]
+ def _setLabel(self, label):
+ if label is not None and (' ' in label or len(label) > 12 or
len(label) == 0):
+ raise FSError("Filesystem label '%s' is incorrectly formatted
for %s." % (label, self.labelfsProg))
+ self._label = label
+
def sync(self, root='/'):
""" Ensure that data we've written is at least in the
journal.
diff --git a/tests/formats_test/misc_test.py b/tests/formats_test/misc_test.py
index bcad51f..effbb41 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."""
@@ -34,8 +88,9 @@ class MethodsTestCase(unittest.TestCase):
self.assertEqual(v._getLabelArgs("myfs"), ["/dev",
"myfs"], msg=k)
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__":