BmapCopy: downgrade inability to set sysfs parameters to a warning

On distributions where unprivileged users can write to removable
USB disks, this allows such a disk to be written with bmaptool
(albeit with non-optimal performance) without being root.

Artem: got some more explanations from Simon McVittie
<simon.mcvittie@collabora.co.uk> about why we don't just add a special
case for EPERM/EACCESS:

"""
> On 28/06/13 12:47, Artem Bityutskiy wrote:
> For a library, requiring the user to have a logger object is probably
> not the nicest thing. And usually libraries do not print error messages,
> they throw exceptions instead.

I made the logger optional: if the caller doesn't supply one, BmapCopy
will use logging.getLogger(__name__), i.e. the logger named
"bmaptools.BmapCopy", which appears to be best-practice for Python
logging. (You could use it for debug-logging too, if you wanted to.)

Having logger=None cause logging to be suppressed, instead of using the
logging module's defaults (which are to print 'No handlers could be
found for logger "foo.bar"' the first time you use it, and not log
anything...) would also be fine.

Libraries throw exceptions if they couldn't do what you asked, but I
think there's some room for a middle ground between "no, I can't" and
silent success.
"""

Fair enough, I think.

Hoewver, I massged the patch a bit and improved the warnings to make
them a bit more user-friendly and give users the idea what is the
consequence of the warning.

Change-Id: I644d9777b7bf9e97dbe002814f7d4e411a7618db
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@intel.com>

diff --git a/bmaptools/BmapCopy.py b/bmaptools/BmapCopy.py
index 211ce17ed5aa0b7a6698c6dec8976608df6aa992..f7a4ace7c4e285ae42d23eac60567e504f4abac3 100644 (file)
--- a/bmaptools/BmapCopy.py
+++ b/bmaptools/BmapCopy.py
@@ -565,25 +565,32 @@ class BmapBdevCopy(BmapCopy):
                 f_scheduler.seek(0)
                 f_scheduler.write("noop")
         except IOError as err:
-            raise Error("cannot enable the 'noop' I/O scheduler: %s" % err)
-
-        # The file contains a list of scheduler with the current
-        # scheduler in square brackets, e.g., "noop deadline [cfq]".
-        # Fetch the current scheduler name
-        import re
-
-        match = re.match(r'.*\[(.+)\].*', contents)
-        if match:
-            self._old_scheduler_value = match.group(1)
-
-        # Limit the write buffering
+            self._logger.warning("failed to enable I/O optimization, expect " \
+                                 "suboptimal speed (reason: cannot switch "   \
+                                 "to the 'noop' I/O scheduler: %s)" % err)
+        else:
+            # The file contains a list of schedulers with the current
+            # scheduler in square brackets, e.g., "noop deadline [cfq]".
+            # Fetch the name of the current scheduler.
+            import re
+
+            match = re.match(r'.*\[(.+)\].*', contents)
+            if match:
+                self._old_scheduler_value = match.group(1)
+
+        # Limit the write buffering, because we do not need too much of it when
+        # writing sequntially. Excessive buffering makes some systems not very
+        # responsive, e.g., this was observed in Fedora 17.
         try:
             with open(self._sysfs_max_ratio_path, "r+") as f_ratio:
                 self._old_max_ratio_value = f_ratio.read()
                 f_ratio.seek(0)
                 f_ratio.write("1")
         except IOError as err:
-            raise Error("cannot set max. I/O ratio to '1': %s" % err)
+            self._logger.warning("failed to disable excessive buffering, " \
+                                 "expect worse system responsiveness "     \
+                                 "(reason: cannot switch to the 'noop' "   \
+                                 "I/O scheduler: %s)" % err)
 
     def _restore_bdev_settings(self):
         """ Restore old block device settings which we changed in