[blivet:rhel7/master 11/16] Use filesystem creation app to set filesystem label (#1038590)

mulhern amulhern at redhat.com
Thu Jan 16 16:10:12 UTC 2014


Related: rhbz#1038590

It turns out that every filesystem that has a distinct labeling app
can also set the label via the the filesystem creation app, so we set the
label there and do not call writeLabel from doFormat at all any more.

If the label has a bad format for the filesystem, we do not try to set,
but we do
log the problem. It turns out that some filesystems creation apps
will truncate excessively long labels, while others will refuse to proceed,
but I think it's best that blivet avoid making such nice distinctions
between different apps and act uniformly for all.

Tests updated to check setting label at creation a little more thoroughly.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 blivet/formats/fs.py            | 13 +++++++--
 blivet/formats/fslabeling.py    | 58 +++++++++++++++++++++++++++++++++++++
 tests/formats_test/misc_test.py | 63 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 126 insertions(+), 8 deletions(-)

diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
index 05f921c..c365581 100644
--- a/blivet/formats/fs.py
+++ b/blivet/formats/fs.py
@@ -321,15 +321,24 @@ class FS(DeviceFormat):
         return free
 
     def _getFormatOptions(self, options=None):
+        """Get a list of format options to be used when creating the
+           filesystem.
+
+           :param options: any special options
+        """
         argv = []
         if options and isinstance(options, list):
             argv.extend(options)
         argv.extend(self.defaultFormatOptions)
         if self._fsProfileSpecifier and self.fsprofile:
             argv.extend([self._fsProfileSpecifier, self.fsprofile])
+        if self.labelFormatOK(self.label):
+            argv.extend(self._labelfs.labelingArgs(self.label))
+        else:
+            log.warning("Could not create label (%s) on filesystem %s", self.label, self.type)
         argv.append(self.device)
         return argv
-    
+
     def doFormat(self, *args, **kwargs):
         """ Create the filesystem.
 
@@ -372,8 +381,6 @@ class FS(DeviceFormat):
         self.exists = True
         self.notifyKernel()
 
-        if self.label and self._labelfs:
-            self.writeLabel()
 
     @property
     def resizeArgs(self):
diff --git a/blivet/formats/fslabeling.py b/blivet/formats/fslabeling.py
index 4b67f4c..7d91a7c 100644
--- a/blivet/formats/fslabeling.py
+++ b/blivet/formats/fslabeling.py
@@ -50,6 +50,29 @@ class FSLabeling(object):
         """
         raise NotImplementedError
 
+    @abc.abstractmethod
+    def labelingArgs(self, label):
+        """Returns the arguments for writing the label during filesystem
+           creation. These arguments are intended to be passed to the
+           appropriate mkfs application.
+
+           :param label: the label to use
+           :type label: str or None
+           :return: the arguments
+           :rtype: list of str
+        """
+        raise NotImplementedError
+
+    @abc.abstractproperty
+    def defaultLabel(self):
+        """Returns the default label for this filesystem, which will be used
+           if no label is specified.
+
+           :return: the default label
+           :rtype: str or None
+        """
+        raise NotImplementedError
+
 class Ext2FSLabeling(FSLabeling):
 
     @property
@@ -59,6 +82,13 @@ class Ext2FSLabeling(FSLabeling):
     def labelFormatOK(self, label):
         return len(label) < 17
 
+    def labelingArgs(self, label):
+        return ["-L", label if label else ""]
+
+    @property
+    def defaultLabel(self):
+        return None
+
 class FATFSLabeling(FSLabeling):
 
     @property
@@ -68,6 +98,13 @@ class FATFSLabeling(FSLabeling):
     def labelFormatOK(self, label):
         return len(label) < 12
 
+    def labelingArgs(self, label):
+        return ["-n", label if label else ""]
+
+    @property
+    def defaultLabel(self):
+        return "NO NAME"
+
 class JFSLabeling(FSLabeling):
 
     @property
@@ -77,6 +114,13 @@ class JFSLabeling(FSLabeling):
     def labelFormatOK(self, label):
         return len(label) < 17
 
+    def labelingArgs(self, label):
+        return ["-L", label if label else ""]
+
+    @property
+    def defaultLabel(self):
+        return None
+
 class ReiserFSLabeling(FSLabeling):
 
     @property
@@ -86,6 +130,13 @@ class ReiserFSLabeling(FSLabeling):
     def labelFormatOK(self, label):
         return len(label) < 17
 
+    def labelingArgs(self, label):
+        return ["-l", label if label else ""]
+
+    @property
+    def defaultLabel(self):
+        return None
+
 class XFSLabeling(FSLabeling):
 
     @property
@@ -94,3 +145,10 @@ class XFSLabeling(FSLabeling):
 
     def labelFormatOK(self, label):
         return ' ' not in label and len(label) < 13
+
+    def labelingArgs(self, label):
+        return ["-L", label if label else ""]
+
+    @property
+    def defaultLabel(self):
+        return None
diff --git a/tests/formats_test/misc_test.py b/tests/formats_test/misc_test.py
index d282321..9a6ab6b 100755
--- a/tests/formats_test/misc_test.py
+++ b/tests/formats_test/misc_test.py
@@ -85,8 +85,9 @@ class LabelingAsRootTestCase(baseclass.DevicelibsTestCase):
     def testLabelingXFS(self):
         _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
 
-        an_fs = fs.XFS(device=_LOOP_DEV0)
+        an_fs = fs.XFS(device=_LOOP_DEV0, label="root___filesystem")
         self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), an_fs._labelfs.defaultLabel)
 
         an_fs.label = "temeraire"
         self.assertIsNone(an_fs.writeLabel())
@@ -102,11 +103,28 @@ class LabelingAsRootTestCase(baseclass.DevicelibsTestCase):
            an_fs.writeLabel)
 
     @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
+    def testCreatingXFS(self):
+        _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
+
+        an_fs = fs.XFS(device=_LOOP_DEV0, label="start")
+        self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), "start")
+
+    @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
+    def testCreatingXFSNone(self):
+        _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
+
+        an_fs = fs.XFS(device=_LOOP_DEV0, label=None)
+        self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), None)
+
+    @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
     def testLabelingFATFS(self):
         _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
 
-        an_fs = fs.FATFS(device=_LOOP_DEV0)
+        an_fs = fs.FATFS(device=_LOOP_DEV0, label="root___filesystem")
         self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), an_fs._labelfs.defaultLabel)
 
         an_fs.label = "an fs"
         self.assertIsNone(an_fs.writeLabel())
@@ -122,11 +140,30 @@ class LabelingAsRootTestCase(baseclass.DevicelibsTestCase):
            an_fs.writeLabel)
 
     @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
+    def testCreatingFATFS(self):
+        _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
+
+        # for some reason mkdosfs capitalizes the flag when it sets it,
+        # while dosfslabel does not
+        an_fs = fs.FATFS(device=_LOOP_DEV0, label="start")
+        self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), "START")
+
+    @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
+    def testCreatingFATFSNone(self):
+        _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
+
+        an_fs = fs.FATFS(device=_LOOP_DEV0, label=None)
+        self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), None)
+
+    @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
     def testLabelingExt2FS(self):
         _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
 
-        an_fs = fs.Ext2FS(device=_LOOP_DEV0)
+        an_fs = fs.Ext2FS(device=_LOOP_DEV0, label="root___filesystem")
         self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), an_fs._labelfs.defaultLabel)
 
         an_fs.label = "an fs"
         self.assertIsNone(an_fs.writeLabel())
@@ -142,10 +179,26 @@ class LabelingAsRootTestCase(baseclass.DevicelibsTestCase):
            an_fs.writeLabel)
 
     @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
+    def testCreatingExt2FS(self):
+        _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
+
+        an_fs = fs.Ext2FS(device=_LOOP_DEV0, label="start")
+        self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), "start")
+
+    @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
+    def testCreatingExt2FSNone(self):
+        _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
+
+        an_fs = fs.Ext2FS(device=_LOOP_DEV0, label=None)
+        self.assertIsNone(an_fs.create())
+        self.assertEqual(an_fs.readLabel(), None)
+
+    @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
     def testLabelingJFS(self):
         _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
 
-        an_fs = fs.JFS(device=_LOOP_DEV0)
+        an_fs = fs.JFS(device=_LOOP_DEV0, label="root___filesystem")
         self.assertIsNone(an_fs.create())
 
         an_fs.label = "an fs"
@@ -167,7 +220,7 @@ class LabelingAsRootTestCase(baseclass.DevicelibsTestCase):
     def testLabelingReiserFS(self):
         _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
 
-        an_fs = fs.ReiserFS(device=_LOOP_DEV0)
+        an_fs = fs.ReiserFS(device=_LOOP_DEV0, label="root___filesystem")
         self.assertIsNone(an_fs.create())
 
         an_fs.label = "an fs"
-- 
1.8.3.1



More information about the anaconda-patches mailing list