Merge lp:~jamesodhunt/ubuntu/vivid/ubuntu-core-upgrader/bug-1435774 into lp:ubuntu/vivid/ubuntu-core-upgrader

[James Hunt]

* Reformat "other" if fsck is unable to repair it (LP: #1435774).

[Timo Jyrinki (timo-jyrinki)]

This should be resubmitted to xenial if still needed.

review: Resubmit

Unmerged revisions

30. By James Hunt on 2015-04-16

Reformat "other" if fsck is unable to repair it (LP: #1435774).

=== modified file 'debian/changelog'
--- debian/changelog	2015-04-16 14:42:14 +0000
+++ debian/changelog	2015-04-16 19:42:51 +0000
@@ -1,3 +1,9 @@
+ubuntu-core-upgrader (0.7.11) UNRELEASED; urgency=medium
+
+  * Reformat "other" if fsck is unable to repair it (LP: #1435774).
+
+ -- James Hunt <[e-mail]>  Thu, 16 Apr 2015 20:12:43 +0100
+
 ubuntu-core-upgrader (0.7.10) vivid; urgency=medium
 
   * ubuntucoreupgrader/upgrader.py: _cmd_mount(): Don't sync partitions

=== modified file 'ubuntucoreupgrader/tests/test_upgrader.py'
--- ubuntucoreupgrader/tests/test_upgrader.py	2015-04-16 14:42:14 +0000
+++ ubuntucoreupgrader/tests/test_upgrader.py	2015-04-16 19:42:51 +0000
@@ -425,5 +425,61 @@
 
         shutil.rmtree(cache_dir)
 
+    @patch('ubuntucoreupgrader.upgrader.get_mount_details')
+    @patch('ubuntucoreupgrader.upgrader.remount')
+    @patch('ubuntucoreupgrader.upgrader.mount')
+    @patch('ubuntucoreupgrader.upgrader.fsck')
+    @patch('ubuntucoreupgrader.upgrader.mkfs')
+    @patch('ubuntucoreupgrader.upgrader.unmount')
+    def test_remount_rootfs_with_good_fsck(self, mock_umount, mock_mkfs,
+                                           mock_fsck, mock_mount,
+                                           mock_remount, mock_mount_details):
+        MOCK_FS_TUPLE = ("device", "fstype", "label")
+        mock_mount_details.return_value = MOCK_FS_TUPLE
+        mock_fsck.return_value = True
+
+        args = ['cmdfile']
+        options = parse_args(args=args)
+        commands = make_commands([self.TARFILE])
+
+        upgrader = Upgrader(options, commands, [])
+        upgrader.TIMESTAMP_FILE = '/dev/null'
+        upgrader.MOUNTPOINT_CMD = "true"
+        upgrader.remount_rootfs(writable=True)
+
+        self.assertTrue(mock_fsck.called)
+
+        # fsck succeeded, so mkfs should not be called
+        self.assertFalse(mock_mkfs.called)
+
+    @patch('ubuntucoreupgrader.upgrader.get_mount_details')
+    @patch('ubuntucoreupgrader.upgrader.remount')
+    @patch('ubuntucoreupgrader.upgrader.mount')
+    @patch('ubuntucoreupgrader.upgrader.fsck')
+    @patch('ubuntucoreupgrader.upgrader.mkfs')
+    @patch('ubuntucoreupgrader.upgrader.unmount')
+    def test_remount_rootfs_with_bad_fsck(self, mock_umount, mock_mkfs,
+                                          mock_fsck, mock_mount,
+                                          mock_remount, mock_mount_details):
+        MOCK_FS_TUPLE = ("device", "fstype", "label")
+        mock_mount_details.return_value = MOCK_FS_TUPLE
+
+        # arrange for fsck to fail
+        mock_fsck.return_value = False
+
+        args = ['cmdfile']
+        options = parse_args(args=args)
+        commands = make_commands([self.TARFILE])
+
+        upgrader = Upgrader(options, commands, [])
+        upgrader.TIMESTAMP_FILE = '/dev/null'
+        upgrader.MOUNTPOINT_CMD = "true"
+        upgrader.remount_rootfs(writable=True)
+
+        self.assertTrue(mock_fsck.called)
+
+        # fsck failed, so "other" should have been reformatted
+        self.assertTrue(mock_mkfs.called)
+
 if __name__ == "__main__":
     unittest.main()

=== modified file 'ubuntucoreupgrader/upgrader.py'
--- ubuntucoreupgrader/upgrader.py	2015-04-16 14:42:14 +0000
+++ ubuntucoreupgrader/upgrader.py	2015-04-16 19:42:51 +0000
@@ -145,11 +145,10 @@
 def fsck(device):
     '''
     Run fsck(8) on specified device.
+    Returns True on success, else False.
     '''
     assert (os.path.exists(device))
 
-    failed = False
-
     cmd = '/sbin/fsck'
     args = []
 
@@ -171,23 +170,22 @@
     ret = proc.wait()
 
     if ret == 0:
-        return
+        return True
 
     stdout, stderr = proc.communicate()
 
     # fsck's return code 1 means: "Filesystem errors corrected"
     # (aka a warning - FS is consistent [now]).
-    failed = False if ret == 1 else True
+    success = True if ret == 1 else False
 
     log.error('{} returned {} ({}): {}, {}'
               .format(args,
                       ret,
-                      "failed" if failed else "warning",
+                      "failed" if not success else "warning",
                       stdout,
                       stderr))
 
-    if failed:
-        sys.exit(1)
+    return success
 
 
 def mkfs(device, fs_type, label):
@@ -738,15 +736,19 @@
         target = self.get_mount_target()
 
         if writable:
-            root, _, _ = get_mount_details(target)
+            device, fstype, label = get_mount_details(target)
 
             # ro->rw so need to fsck first.
             unmount(target)
 
             # needs to be mounted writable, so check it first!
-            fsck(root)
+            if not fsck(device):
+                log.warning('fsck for device {} (fstype {}, label {} failed) '
+                            '- reformatting'
+                            .format(device, fstype, label))
+                mkfs(device, fstype, label)
 
-            mount(root, target, "rw")
+            mount(device, target, "rw")
         else:
             # rw->ro so no fsck required.
             remount(target, "ro")