Merge lp:~lool/linaro-image-tools/lp-711573 into lp:linaro-image-tools/11.11

Proposed by Loïc Minier
Status: Merged
Merged at revision: 283
Proposed branch: lp:~lool/linaro-image-tools/lp-711573
Merge into: lp:linaro-image-tools/11.11
Diff against target: 31 lines (+8/-10)
1 file modified
linaro-media-create (+8/-10)
To merge this branch: bzr merge lp:~lool/linaro-image-tools/lp-711573
Reviewer Review Type Date Requested Status
Guilherme Salgado (community) Approve
Review via email: mp+48289@code.launchpad.net

Description of the change

This fixes the SubcommandNonZeroReturnValue not defined part, but I don't
understand the _child_created part.

To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :

As I said in the bug, this won't be enough to fix the bug; lmc will still fail because we're passing cmd_runner.Popen() an argument it is not expecting.

Also, we should not wrap both umount statements in a single try/except block as if the first one fails the second won't execute. This is not the same behavior exhibited by the previous code, which was using raw Popen() to ignore failures in the subprocesses

review: Needs Fixing
284. By Loïc Minier

Use cmd_runner.run() instead of .Popen() as the latter doesn't accept as_root.

285. By Loïc Minier

Always try to unmount ROOT_DISK, even if BOOT_DISK didn't unmount successfully.

Revision history for this message
Guilherme Salgado (salgado) wrote :

Ok, this should fix the issue and make lmc cleanup properly again. It'd be nice to at least generate an image to make sure this works as expected and doesn't break anything else.

review: Approve
Revision history for this message
Loïc Minier (lool) wrote :

13:37 < salgado> lool, I ran it here and it worked

14:21 < lool> salgado: I ran it too over lunch

I created a qemu image file and didn't get the traceback while I did get it with trunk

Mattias Backman says he might look into adding tests for linaro-media-create

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'linaro-media-create'
2--- linaro-media-create 2011-01-29 16:46:33 +0000
3+++ linaro-media-create 2011-02-02 12:13:15 +0000
4@@ -63,19 +63,17 @@
5 """
6 devnull = open('/dev/null', 'w')
7 # ignore non-zero return codes
8- try:
9- if BOOT_DISK is not None:
10- cmd_runner.Popen(['umount', BOOT_DISK],
11- stdout=devnull, stderr=devnull, as_root=True).wait()
12- if ROOT_DISK is not None:
13- cmd_runner.Popen(['umount', ROOT_DISK],
14- stdout=devnull, stderr=devnull, as_root=True).wait()
15- except SubcommandNonZeroReturnValue:
16- pass
17+ for disk in BOOT_DISK, ROOT_DISK:
18+ if disk is not None:
19+ try:
20+ cmd_runner.run(['umount', disk],
21+ stdout=devnull, stderr=devnull, as_root=True).wait()
22+ except cmd_runner.SubcommandNonZeroReturnValue:
23+ pass
24 # Remove TMP_DIR as root because some files written there are
25 # owned by root.
26 if TMP_DIR is not None:
27- cmd_runner.Popen(['rm', '-rf', TMP_DIR], as_root=True).wait()
28+ cmd_runner.run(['rm', '-rf', TMP_DIR], as_root=True).wait()
29
30
31 def ensure_required_commands(args):

Subscribers

People subscribed via source and target branches