This might not be necessary, but in the event that something goes wrong
and the user wants to re-run the script, it would be nice if it didn't
fail just because /target was mounted, or simply already existed, on the
bootstrap OS. If the script fails it will still try to unmount /target,
and if the formatting step fails it will retry after sending a command
to unmount.

Signed-off-by: Ben Lipton <[email protected]>
---
Modified to apply to current master.

 p2v-transfer/p2v_transfer.py           |   28 +++++++++++++++++++++++++---
 p2v-transfer/test/p2v_transfer_test.py |   30 +++++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/p2v-transfer/p2v_transfer.py b/p2v-transfer/p2v_transfer.py
index 56f8e38..54ff12e 100755
--- a/p2v-transfer/p2v_transfer.py
+++ b/p2v-transfer/p2v_transfer.py
@@ -253,11 +253,16 @@ EOF
   other_commands = [
     "mkfs.ext3 /dev/xvda1",
     "mkswap /dev/xvda2",
-    "mkdir /target",
-    "mount /dev/xvda1 /target",
+    "mkdir -p %s" % TARGET_MOUNT,
+    "mount /dev/xvda1 %s" % TARGET_MOUNT,
     ]
 
-  _RunCommandAndWait(client, " && ".join(other_commands))
+  try:
+    _RunCommandAndWait(client, " && ".join(other_commands))
+  except P2VError:
+    # Make sure target is unmounted, then try again
+    CleanUpTarget(client)
+    _RunCommandAndWait(client, " && ".join(other_commands))
 
 
 def TransferFiles(user, host, keyfile):
@@ -307,12 +312,27 @@ def UnmountSourceFilesystems():
     sys.exit(1)
 
 
+def CleanUpTarget(client):
+  """Unmount target filesystem, remove /target directory.
+
+  Cleans up the target to make it look like the p2v_transfer was never run.
+  This means that if it doesn't complete successfully, we should be able to do
+  it again.
+
+  @type client: paramiko.SSHClient
+  @param client: SSH client object used to connect to the instance.
+  """
+  _RunCommandAndWait(client, "umount %s ; rmdir %s" % (TARGET_MOUNT,
+                                                       TARGET_MOUNT))
+
+
 def main(argv):
   options, args = ParseOptions(argv)
 
   user = "root"
   root_dev, host, keyfile = args
 
+  client = None
   try:
     try:
       uid = os.getuid()
@@ -335,6 +355,8 @@ def main(argv):
   finally:
     if uid == 0:
       UnmountSourceFilesystems()
+    if client:
+      CleanUpTarget(client)
 
 
 if __name__ == "__main__":
diff --git a/p2v-transfer/test/p2v_transfer_test.py 
b/p2v-transfer/test/p2v_transfer_test.py
index bbca6d2..bd1fdb5 100755
--- a/p2v-transfer/test/p2v_transfer_test.py
+++ b/p2v-transfer/test/p2v_transfer_test.py
@@ -95,6 +95,7 @@ class P2vtransferTest(unittest.TestCase):
       "ShutDownTarget",
       "VerifyKernelMatches",
       "LoadSSHKey",
+      "CleanUpTarget",
       ]
     for func in self.module_functions:
       self.mox.StubOutWithMock(self.module, func)
@@ -199,7 +200,7 @@ EOF
 
     commands = ("mkfs.ext3 /dev/xvda1"
                 " && mkswap /dev/xvda2"
-                " && mkdir %s"
+                " && mkdir -p %s"
                 " && mount /dev/xvda1 %s") % (self.module.TARGET_MOUNT,
                                               self.module.TARGET_MOUNT)
     self._MockRunCommandAndWait(commands)
@@ -208,6 +209,31 @@ EOF
     self.module.PartitionTargetDisks(self.client, self.totsize, self.swapsize)
     self.mox.VerifyAll()
 
+  def testPartitionTargetDisksRetriesFormatOnError(self):
+    self.mox.StubOutWithMock(self.module, 'CleanUpTarget')
+
+    sfdisk_command = """sfdisk -uM /dev/xvda <<EOF
+0,%d,83
+,,82
+EOF
+""" % (self.totsize - self.swapsize)
+
+    self._MockRunCommandAndWait(sfdisk_command)
+
+    commands = ("mkfs.ext3 /dev/xvda1"
+                " && mkswap /dev/xvda2"
+                " && mkdir -p %s"
+                " && mount /dev/xvda1 %s") % (self.module.TARGET_MOUNT,
+                                              self.module.TARGET_MOUNT)
+    self._MockRunCommandAndWait(commands, 1)  # maybe /target is mounted
+    self.module.CleanUpTarget(self.client)    # so, make sure it's unmounted
+    self._MockRunCommandAndWait(commands)     # and try again
+
+    self.mox.ReplayAll()
+    self.module.PartitionTargetDisks(self.client, self.totsize, self.swapsize)
+    self.mox.VerifyAll()
+    pass
+
   def testTransferFilesExitsOnError(self):
     user = "root"
     host = "instance"
@@ -264,6 +290,7 @@ EOF
     self.module.RunFixScripts(self.client)
     self.module.ShutDownTarget(self.client)
     self.module.UnmountSourceFilesystems()
+    self.module.CleanUpTarget(self.client)
 
     self.mox.ReplayAll()
     self.module.main(self.test_argv)
@@ -307,6 +334,7 @@ EOF
     call.AndRaise(self.module.P2VError("meep"))
     # Transfer is cancelled because of the error, but still we have:
     self.module.UnmountSourceFilesystems()
+    self.module.CleanUpTarget(self.client)
 
     self.mox.ReplayAll()
     self.assertRaises(SystemExit, self.module.main, self.test_argv)
-- 
1.7.3.1

Reply via email to