From patchwork Tue Feb 1 17:31:29 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guilherme Salgado X-Patchwork-Id: 56 Return-Path: Delivered-To: unknown Received: from imap.gmail.com (74.125.159.109) by localhost6.localdomain6 with IMAP4-SSL; 08 Jun 2011 14:39:35 -0000 Delivered-To: patches@linaro.org Received: by 10.147.124.5 with SMTP id b5cs111861yan; Tue, 1 Feb 2011 09:31:31 -0800 (PST) Received: by 10.216.154.136 with SMTP id h8mr5662278wek.84.1296581490403; Tue, 01 Feb 2011 09:31:30 -0800 (PST) Received: from adelie.canonical.com (adelie.canonical.com [91.189.90.139]) by mx.google.com with ESMTP id n33si20115630wei.194.2011.02.01.09.31.30; Tue, 01 Feb 2011 09:31:30 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.139 as permitted sender) client-ip=91.189.90.139; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.139 as permitted sender) smtp.mail=bounces@canonical.com Received: from loganberry.canonical.com ([91.189.90.37]) by adelie.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1PkK4f-0004CW-96 for ; Tue, 01 Feb 2011 17:31:29 +0000 Received: from loganberry.canonical.com (localhost [127.0.0.1]) by loganberry.canonical.com (Postfix) with ESMTP id 401052E80AE for ; Tue, 1 Feb 2011 17:31:29 +0000 (UTC) MIME-Version: 1.0 X-Launchpad-Project: linaro-image-tools X-Launchpad-Branch: ~linaro-maintainers/linaro-image-tools/trunk X-Launchpad-Message-Rationale: Subscriber X-Launchpad-Branch-Revision-Number: 281 X-Launchpad-Notification-Type: branch-revision To: Linaro Patch Tracker From: noreply@launchpad.net Subject: [Branch ~linaro-maintainers/linaro-image-tools/trunk] Rev 281: Make sure we cleanup properly when install_hwpacks fails Message-Id: <20110201173129.11543.61609.launchpad@loganberry.canonical.com> Date: Tue, 01 Feb 2011 17:31:29 -0000 Reply-To: noreply@launchpad.net Sender: bounces@canonical.com Errors-To: bounces@canonical.com Precedence: bulk X-Generated-By: Launchpad (canonical.com); Revision="12274"; Instance="initZopeless config overlay" X-Launchpad-Hash: a36b9bd3c92934625b2c26269fb1be6083f4013b Merge authors: Andy Doan (doanac) Guilherme Salgado (salgado) Related merge proposals: https://code.launchpad.net/~doanac/linaro-image-tools/slash-proc-spam-version2/+merge/47436 proposed by: Andy Doan (doanac) review: Approve - Guilherme Salgado (salgado) ------------------------------------------------------------ revno: 281 [merge] committer: Guilherme Salgado branch nick: trunk timestamp: Tue 2011-02-01 15:29:25 -0200 message: Make sure we cleanup properly when install_hwpacks fails modified: linaro_media_create/hwpack.py linaro_media_create/tests/test_media_create.py --- lp:linaro-image-tools https://code.launchpad.net/~linaro-maintainers/linaro-image-tools/trunk You are subscribed to branch lp:linaro-image-tools. To unsubscribe from this branch go to https://code.launchpad.net/~linaro-maintainers/linaro-image-tools/trunk/+edit-subscription === modified file 'linaro_media_create/hwpack.py' --- linaro_media_create/hwpack.py 2011-01-28 19:50:48 +0000 +++ linaro_media_create/hwpack.py 2011-02-01 17:07:04 +0000 @@ -18,6 +18,7 @@ # along with Linaro Image Tools. If not, see . import os +import sys from linaro_media_create import cmd_runner from linaro_media_create.utils import is_arm_host @@ -49,12 +50,12 @@ copy_file(linaro_hwpack_install_path, os.path.join(chroot_dir, 'usr', 'bin')) - mount_chroot_proc(chroot_dir) - - for hwpack_file in hwpack_files: - install_hwpack(chroot_dir, hwpack_file, hwpack_force_yes) - - run_local_atexit_funcs() + try: + mount_chroot_proc(chroot_dir) + for hwpack_file in hwpack_files: + install_hwpack(chroot_dir, hwpack_file, hwpack_force_yes) + finally: + run_local_atexit_funcs() def install_hwpack(chroot_dir, hwpack_file, hwpack_force_yes): @@ -82,12 +83,14 @@ Also register a function in local_atexit to unmount that /proc filesystem. """ chroot_proc = os.path.join(chroot_dir, 'proc') + + def umount_chroot_proc(): + cmd_runner.run(['umount', '-v', chroot_proc], as_root=True).wait() + local_atexit.append(umount_chroot_proc) + proc = cmd_runner.run( ['mount', 'proc', chroot_proc, '-t', 'proc'], as_root=True) proc.wait() - def umount_chroot_proc(): - cmd_runner.run(['umount', '-v', chroot_proc], as_root=True).wait() - local_atexit.append(umount_chroot_proc) def copy_file(filepath, directory): @@ -130,5 +133,18 @@ def run_local_atexit_funcs(): # Run the funcs in LIFO order, just like atexit does. + exc_info = None while len(local_atexit) > 0: - local_atexit.pop()() + func = local_atexit.pop() + try: + func() + except SystemExit: + exc_info = sys.exc_info() + except: + import traceback + print >> sys.stderr, "Error in local_atexit:" + traceback.print_exc() + exc_info = sys.exc_info() + + if exc_info is not None: + raise exc_info[0], exc_info[1], exc_info[2] === modified file 'linaro_media_create/tests/test_media_create.py' --- linaro_media_create/tests/test_media_create.py 2011-02-01 17:23:43 +0000 +++ linaro_media_create/tests/test_media_create.py 2011-02-01 17:29:25 +0000 @@ -1109,3 +1109,64 @@ ['sudo', 'mv', '-f', '/tmp/dir/hosts', 'chroot/etc'], ['sudo', 'mv', '-f', '/tmp/dir/resolv.conf', 'chroot/etc']], fixture.mock.calls) + + def test_run_local_atexit_funcs(self): + self.useFixture(MockSomethingFixture( + sys, 'stderr', open('/dev/null', 'w'))) + self.call_order = [] + class TestException(Exception): + pass + def raising_func(): + self.call_order.append('raising_func') + raise TestException() + def behaving_func(): + self.call_order.append('behaving_func') + self.behaving_func_called = True + # run_local_atexit_funcs() runs the atexit handlers in LIFO order, but + # even though the first function called (raising_func) will raise + # an exception, the second one will still be called after it. + linaro_media_create.hwpack.local_atexit = [ + behaving_func, raising_func] + # run_local_atexit_funcs() also propagates the last exception raised + # by one of the functions. + self.assertRaises( + TestException, linaro_media_create.hwpack.run_local_atexit_funcs) + self.assertEquals( + ['raising_func', 'behaving_func'], self.call_order) + + def test_hwpack_atexit(self): + self.run_local_atexit_functions_called = False + + def mock_run_local_atexit_functions(): + self.run_local_atexit_functions_called = True + + def mock_install_hwpack(p1, p2, p3): + raise Exception('hwpack mock exception') + + self.useFixture(MockSomethingFixture( + sys, 'stdout', open('/dev/null', 'w'))) + self.useFixture(MockCmdRunnerPopenFixture()) + self.useFixture(MockSomethingFixture( + linaro_media_create.hwpack, 'install_hwpack', mock_install_hwpack)) + self.useFixture(MockSomethingFixture( + linaro_media_create.hwpack, 'run_local_atexit_funcs', + mock_run_local_atexit_functions)) + + force_yes = True + exception_caught = False + try: + install_hwpacks( + 'chroot', '/tmp/dir', force_yes, 'hwp.tgz', 'hwp2.tgz') + except: + exception_caught = True + self.assertTrue(self.run_local_atexit_functions_called) + self.assertTrue(exception_caught) + + def setUp(self): + super(TestInstallHWPack, self).setUp() + # Ensure the list of cleanup functions gets cleared to make sure tests + # don't interfere with one another. + def clear_atexits(): + linaro_media_create.hwpack.local_atexit = [] + self.addCleanup(clear_atexits) +