[master 1/2] Change btrfs.do_self_mount to contextmanager (#1266673)

vojtechtrefny installerbot-noreply at redhat.com
Thu Oct 8 12:59:09 UTC 2015


From: Vojtech Trefny <vtrefny at redhat.com>

Signed-off-by: Vojtech Trefny <vtrefny at redhat.com>
---
 blivet/devices/btrfs.py | 161 +++++++++++++++++++-----------------------------
 1 file changed, 65 insertions(+), 96 deletions(-)

diff --git a/blivet/devices/btrfs.py b/blivet/devices/btrfs.py
index 63f43a4..0cc888a 100644
--- a/blivet/devices/btrfs.py
+++ b/blivet/devices/btrfs.py
@@ -35,6 +35,7 @@
 from ..flags import flags
 from ..storage_log import log_method_call
 from .. import udev
+from .. import util
 from ..formats import getFormat, DeviceFormat
 from ..size import Size
 
@@ -47,6 +48,8 @@
 
 from ..tasks import availability
 
+from contextlib import contextmanager
+
 class BTRFSDevice(StorageDevice):
     """ Base class for BTRFS volume and sub-volume devices. """
     _type = "btrfs"
@@ -102,9 +105,10 @@ def status(self):
     def _temp_dir_prefix(self):
         return "btrfs-tmp.%s" % self.id
 
+    @contextmanager
     def _do_temp_mount(self, orig=False):
         if not self.exists:
-            return
+            raise errors.FSError("format doesn't exist")
 
         if orig:
             fmt = self.originalFormat
@@ -112,25 +116,21 @@ def _do_temp_mount(self, orig=False):
             fmt = self.format
 
         if fmt.status:
-            return
-
-        tmpdir = tempfile.mkdtemp(prefix=self._temp_dir_prefix)
-
-        fmt.mount(mountpoint=tmpdir)
+            yield fmt.systemMountpoint
 
-    def _undo_temp_mount(self):
-        if getattr(self.format, "systemMountpoint", None):
-            fmt = self.format
-        elif getattr(self.originalFormat, "systemMountpoint", None):
-            fmt = self.originalFormat
         else:
-            return
-
-        mountpoint = fmt.systemMountpoint
+            tmpdir = tempfile.mkdtemp(prefix=self._temp_dir_prefix)
+            try:
+                fmt.mount(mountpoint=tmpdir)
+            except errors.FSError as e:
+                log.debug("btrfs temp mount failed: %s", e)
+                raise
 
-        if os.path.basename(mountpoint).startswith(self._temp_dir_prefix):
-            fmt.unmount()
-            os.rmdir(mountpoint)
+            try:
+                yield tmpdir
+            finally:
+                util.umount(mountpoint=tmpdir)
+                os.rmdir(tmpdir)
 
     @property
     def path(self):
@@ -331,28 +331,23 @@ def _removeSubVolume(self, name):
         self.subvolumes.pop(names.index(name))
 
     def listSubVolumes(self, snapshotsOnly=False):
-        subvols = []
+
         if flags.installer_mode:
             self.setup(orig=True)
 
-            try:
-                self._do_temp_mount(orig=True)
-            except errors.FSError as e:
-                log.debug("btrfs temp mount failed: %s", e)
-                return subvols
-        elif not self.originalFormat.status:
-            return subvols
-
+        subvols = []
         try:
-            subvols = blockdev.btrfs.list_subvolumes(self.originalFormat.systemMountpoint,
-                                                     snapshots_only=snapshotsOnly)
-        except blockdev.BtrfsError as e:
-            log.debug("failed to list subvolumes: %s", e)
-        else:
-            self._getDefaultSubVolumeID()
-        finally:
-            if flags.installer_mode:
-                self._undo_temp_mount()
+            with self._do_temp_mount(orig=True) as mountpoint:
+                try:
+                    subvols = blockdev.btrfs.list_subvolumes(mountpoint,
+                                                             snapshots_only=snapshotsOnly)
+                except blockdev.BtrfsError as e:
+                    log.debug("failed to list subvolumes: %s", e)
+                else:
+                    self._getDefaultSubVolumeID()
+
+        except errors.FSError as e:
+            pass
 
         return subvols
 
@@ -370,10 +365,11 @@ def removeSubVolume(self, name):
 
     def _getDefaultSubVolumeID(self):
         subvolid = None
-        try:
-            subvolid = blockdev.btrfs.get_default_subvolume_id(self.originalFormat.systemMountpoint)
-        except blockdev.BtrfsError as e:
-            log.debug("failed to get default subvolume id: %s", e)
+        with self._do_temp_mount() as mountpoint:
+            try:
+                subvolid = blockdev.btrfs.get_default_subvolume_id(mountpoint)
+            except blockdev.BtrfsError as e:
+                log.debug("failed to get default subvolume id: %s", e)
 
         self._defaultSubVolumeID = subvolid
 
@@ -382,17 +378,18 @@ def _setDefaultSubVolumeID(self, vol_id):
 
             This writes the change to the filesystem, which must be mounted.
         """
-        try:
-            blockdev.btrfs.set_default_subvolume(self.originalFormat.systemMountpoint, vol_id)
-        except blockdev.BtrfsError as e:
-            log.error("failed to set new default subvolume id (%s): %s",
-                      vol_id, e)
-            # The only time we set a new default subvolume is so we can remove
-            # the current default. If we can't change the default, we won't be
-            # able to remove the subvolume.
-            raise
-        else:
-            self._defaultSubVolumeID = vol_id
+        with self._do_temp_mount() as mountpoint:
+            try:
+                blockdev.btrfs.set_default_subvolume(mountpoint, vol_id)
+            except blockdev.BtrfsError as e:
+                log.error("failed to set new default subvolume id (%s): %s",
+                          vol_id, e)
+                # The only time we set a new default subvolume is so we can remove
+                # the current default. If we can't change the default, we won't be
+                # able to remove the subvolume.
+                raise
+            else:
+                self._defaultSubVolumeID = vol_id
 
     @property
     def defaultSubVolume(self):
@@ -450,28 +447,14 @@ def _destroy(self):
 
     def _remove(self, member):
         log_method_call(self, self.name, status=self.status)
-        try:
-            self._do_temp_mount(orig=True)
-        except errors.FSError as e:
-            log.debug("btrfs temp mount failed: %s", e)
-            raise
 
-        try:
-            blockdev.btrfs.remove_device(self.originalFormat.systemMountpoint, member.path)
-        finally:
-            self._undo_temp_mount()
+        with self._do_temp_mount() as mountpoint:
+            blockdev.btrfs.remove_device(mountpoint, member.path)
 
     def _add(self, member):
-        try:
-            self._do_temp_mount(orig=True)
-        except errors.FSError as e:
-            log.debug("btrfs temp mount failed: %s", e)
-            raise
 
-        try:
-            blockdev.btrfs.add_device(self.originalFormat.systemMountpoint, member.path)
-        finally:
-            self._undo_temp_mount()
+        with self._do_temp_mount() as mountpoint:
+            blockdev.btrfs.add_device(mountpoint, member.path)
 
     def populateKSData(self, data):
         super(BTRFSVolumeDevice, self).populateKSData(data)
@@ -553,15 +536,9 @@ def setupParents(self, orig=False):
 
     def _create(self):
         log_method_call(self, self.name, status=self.status)
-        self.volume._do_temp_mount()
-        mountpoint = self.volume.format.systemMountpoint
-        if not mountpoint:
-            raise RuntimeError("btrfs subvol create requires mounted volume")
 
-        try:
+        with self.volume._do_temp_mount() as mountpoint:
             blockdev.btrfs.create_subvolume(mountpoint, self.name)
-        finally:
-            self.volume._undo_temp_mount()
 
     def _postCreate(self):
         super(BTRFSSubVolumeDevice, self)._postCreate()
@@ -569,16 +546,13 @@ def _postCreate(self):
 
     def _destroy(self):
         log_method_call(self, self.name, status=self.status)
-        self.volume._do_temp_mount(orig=True)
-        if self.volume._defaultSubVolumeID == self.vol_id:
-            # btrfs does not allow removal of the default subvolume
-            self.volume._setDefaultSubVolumeID(self.volume.vol_id)
 
-        mountpoint = self.volume.originalFormat.systemMountpoint
-        if not mountpoint:
-            raise RuntimeError("btrfs subvol destroy requires mounted volume")
-        blockdev.btrfs.delete_subvolume(mountpoint, self.name)
-        self.volume._undo_temp_mount()
+        with self.volume._do_temp_mount() as mountpoint:
+            if self.volume._defaultSubVolumeID == self.vol_id:
+                # btrfs does not allow removal of the default subvolume
+                self.volume._setDefaultSubVolumeID(self.volume.vol_id)
+
+            blockdev.btrfs.delete_subvolume(mountpoint, self.name)
 
     def removeHook(self, modparent=True):
         if modparent:
@@ -657,21 +631,16 @@ def __init__(self, *args, **kwargs):
 
     def _create(self):
         log_method_call(self, self.name, status=self.status)
-        self.volume._do_temp_mount()
-        mountpoint = self.volume.format.systemMountpoint
-        if not mountpoint:
-            raise RuntimeError("btrfs subvol create requires mounted volume")
 
-        if isinstance(self.source, BTRFSVolumeDevice):
-            source_path = mountpoint
-        else:
-            source_path = "%s/%s" % (mountpoint, self.source.name)
+        with self.volume._do_temp_mount() as mountpoint:
+            if isinstance(self.source, BTRFSVolumeDevice):
+                source_path = mountpoint
+            else:
+                source_path = "%s/%s" % (mountpoint, self.source.name)
+
+            dest_path = "%s/%s" % (mountpoint, self.name)
 
-        dest_path = "%s/%s" % (mountpoint, self.name)
-        try:
             blockdev.btrfs.create_snapshot(source_path, dest_path, ro=self.readOnly)
-        finally:
-            self.volume._undo_temp_mount()
 
     def dependsOn(self, dep):
         return (dep == self.source or


-- 
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/2502ab067a13ed6d7c4dcce0816a5d50410afa88


More information about the anaconda-patches mailing list