[blivet:rhel7/master 3/4] Change FS.label to a property (#1038590)
David Lehman
dlehman at redhat.com
Wed Dec 18 15:31:37 UTC 2013
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 at 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__":
More information about the anaconda-patches
mailing list