[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