----- Original Message -----
From: "Vratislav Podzimek" <vpodzime(a)redhat.com>
To: anaconda-patches(a)lists.fedorahosted.org
Sent: Thursday, November 27, 2014 7:46:27 AM
Subject: [PATCH 2/3] Try to get FS info first before doing an FS check
Running an FS check implicitly may slow things down quite a big and may
result
in changes users didn't expect to happen while probing their file systems.
Related: rhbz#1162215
Signed-off-by: Vratislav Podzimek <vpodzime(a)redhat.com>
---
blivet/formats/fs.py | 69
+++++++++++++++++++++++++++++++++-------------------
1 file changed, 44 insertions(+), 25 deletions(-)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
index b20a252..9c17470 100644
--- a/blivet/formats/fs.py
+++ b/blivet/formats/fs.py
@@ -234,41 +234,55 @@ class FS(DeviceFormat):
self._resizable = self.__class__._resizable
# We can't allow resize if the filesystem has errors.
+ errors = False
+ ret, info = self._getFSInfo()
+ if ret != 0:
+ try:
+ self.doCheck()
+ except FSError:
+ errors = True
+ raise
+ else:
+ errors = False
+ finally:
+ # try to gather current size info anyway
The comment above doesn't match anymore.
+ info = self._getFSInfo()[1]
+
+ self._size = self._getExistingSize(info=info)
+ self._minSize = self._size # default to current size
+ # We absolutely need a current size to enable resize. To shrink the
+ # filesystem we need a real minimum size provided by the resize
+ # tool. Failing that, we can default to the current size,
+ # effectively disabling shrink.
+ if errors or self._size == Size(0):
+ self._resizable = False
+
It would be helpful to have a comment here explaining why it makes sense
to force fsck again if _getMinSize has error. Also, it would be better
to make _getMinSize() be consistent about error raising, or else to document
in _getMinSize() method header
why it raises an error when it can not find min size in one way, but just
sets _minInstanceSize to 0 when it can not find min size in some other way.
try:
- self.doCheck()
+ self._getMinSize(info=info) # force calculation of minimum
size
except FSError:
- errors = True
- raise
- else:
- errors = False
- finally:
- # try to gather current size info anyway
- info = self._getFSInfo()
- self._size = self._getExistingSize(info=info)
- self._minSize = self._size # default to current size
- # We absolutely need a current size to enable resize. To shrink
the
- # filesystem we need a real minimum size provided by the resize
- # tool. Failing that, we can default to the current size,
- # effectively disabling shrink.
- if errors or self._size == Size(0):
- self._resizable = False
-
- self._getMinSize(info=info) # force calculation of minimum size
+ # if failed, try to check filesystem first
+ try:
+ self.doCheck()
+ except FSError:
+ errors = True
+ raise
+ self._getMinSize(info=info)
def _getMinSize(self, info=None):
pass
def _getFSInfo(self):
buf = ""
+ ret = 0
if self.infofsProg and self.exists and \
util.find_program_in_path(self.infofsProg):
argv = self._defaultInfoOptions + [ self.device ]
try:
- buf = util.capture_output([self.infofsProg] + argv)
+ ret, buf =
util.run_program_and_capture_output([self.infofsProg] + argv)
except OSError as e:
log.error("failed to gather fs info: %s", e)
- return buf
+ return ret, buf
Would be nice to throw in a method docstring here, since changing interface.
Possibly would make sense to just return value of "" for buf if system call
result was non zero.
def _getExistingSize(self, info=None):
""" Determine the size of this filesystem.
@@ -313,7 +327,7 @@ class FS(DeviceFormat):
if self.exists and not size:
if info is None:
- info = self._getFSInfo()
+ info = self._getFSInfo()[1]
try:
values = []
@@ -987,7 +1001,7 @@ class Ext2FS(FS):
if self.exists and os.path.exists(self.device):
if info is None:
# get block size
- info = self._getFSInfo()
+ info = self._getFSInfo()[1]
for line in info.splitlines():
if line.startswith("Block size:"):
@@ -1002,8 +1016,11 @@ class Ext2FS(FS):
"on %s" % (self.mountType, self.device))
# get minimum size according to resize2fs
- buf = util.capture_output([self.resizefsProg,
- "-P", self.device])
+ ret, buf =
util.run_program_and_capture_output([self.resizefsProg,
+ "-P",
self.device])
+ if ret != 0:
+ raise FSError("failed to get filesystem's minimum size")
+
_size = None
for line in buf.splitlines():
# line will look like:
@@ -1426,7 +1443,9 @@ class NTFS(FS):
if self.exists and os.path.exists(self.device) and \
util.find_program_in_path(self.resizefsProg):
minSize = None
- buf = util.capture_output([self.resizefsProg, "-m",
self.device])
+ ret, buf =
util.run_program_and_capture_output([self.resizefsProg, "-m", self.device])
+ if ret != 0:
+ raise FSError("Failed to get filesystem's minimal size")
for l in buf.split("\n"):
if not l.startswith("Minsize"):
continue
--
1.9.3
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
- mulhern