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

[Loïc Minier]

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

[Guilherme Salgado (salgado)]

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

[lp:~lool/linaro-image-tools/lp-711573]

284. By Loïc Minier on 2011-02-02

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

285. By Loïc Minier on 2011-02-02

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

[Guilherme Salgado (salgado)]

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

[Loïc Minier (lool)]

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

=== modified file 'linaro-media-create'
--- linaro-media-create	2011-01-29 16:46:33 +0000
+++ linaro-media-create	2011-02-02 12:13:15 +0000
@@ -63,19 +63,17 @@
     """
     devnull = open('/dev/null', 'w')
     # ignore non-zero return codes
-    try:
-        if BOOT_DISK is not None:
-            cmd_runner.Popen(['umount', BOOT_DISK],
-                  stdout=devnull, stderr=devnull, as_root=True).wait()
-        if ROOT_DISK is not None:
-            cmd_runner.Popen(['umount', ROOT_DISK],
-                  stdout=devnull, stderr=devnull, as_root=True).wait()
-    except SubcommandNonZeroReturnValue:
-        pass
+    for disk in BOOT_DISK, ROOT_DISK:
+        if disk is not None:
+            try:
+                cmd_runner.run(['umount', disk],
+                      stdout=devnull, stderr=devnull, as_root=True).wait()
+            except cmd_runner.SubcommandNonZeroReturnValue:
+                pass
     # Remove TMP_DIR as root because some files written there are
     # owned by root.
     if TMP_DIR is not None:
-        cmd_runner.Popen(['rm', '-rf', TMP_DIR], as_root=True).wait()
+        cmd_runner.run(['rm', '-rf', TMP_DIR], as_root=True).wait()
 
 
 def ensure_required_commands(args):