From 91a0980537dc4a023bd8e65b74d8d47ba067a944 Mon Sep 17 00:00:00 2001
From: David Ribeiro Alves <davidralves@gmail.com>
Date: Mon, 12 Mar 2012 11:59:45 +0000
Subject: [PATCH] made destroying nodes synchornized due to session lock issues

---
 .../compute/VirtualBoxComputeServiceAdapter.java   |  6 +++---
 ...isterMachineFromIMachineIfNotAlreadyExists.java |  5 +++--
 .../jclouds/virtualbox/functions/NodeCreator.java  | 14 ++++++++++----
 .../admin/StartVBoxIfNotAlreadyRunning.java        |  6 ++++++
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapter.java b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapter.java
index 771ce60b61..1f15abbb0b 100644
--- a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapter.java
+++ b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapter.java
@@ -150,7 +150,7 @@ public class VirtualBoxComputeServiceAdapter implements ComputeServiceAdapter<IM
    }
 
    @Override
-   public void destroyNode(String vmName) {
+   public synchronized void destroyNode(String vmName) {
       IMachine machine = manager.get().getVBox().findMachine(vmName);
       powerDownMachine(machine);
       new UnregisterMachineIfExistsAndForceDeleteItsMedia().apply(machine);
@@ -198,10 +198,10 @@ public class VirtualBoxComputeServiceAdapter implements ComputeServiceAdapter<IM
    private void powerDownMachine(IMachine machine) {
       try {
          if (machine.getState() == MachineState.PoweredOff){
-            logger.debug("vm was already powered down: ", machine.getName());
+            logger.debug("vm was already powered down: ", machine.getId());
             return;
          }
-         logger.debug("powering down vm: ", machine.getName());
+         logger.debug("powering down vm: ", machine.getId());
          ISession machineSession = manager.get().openMachineSession(machine);
          IProgress progress = machineSession.getConsole().powerDown();
          progress.waitForCompletion(-1);
diff --git a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/CloneAndRegisterMachineFromIMachineIfNotAlreadyExists.java b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/CloneAndRegisterMachineFromIMachineIfNotAlreadyExists.java
index dd21171765..abca1e02b0 100644
--- a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/CloneAndRegisterMachineFromIMachineIfNotAlreadyExists.java
+++ b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/CloneAndRegisterMachineFromIMachineIfNotAlreadyExists.java
@@ -24,6 +24,7 @@ import java.util.List;
 
 import javax.annotation.Resource;
 import javax.inject.Named;
+import javax.inject.Singleton;
 
 import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.logging.Logger;
@@ -47,7 +48,7 @@ import com.google.inject.Inject;
 
 /**
  * CloneAndRegisterMachineFromIMachineIfNotAlreadyExists will take care of the followings: - cloning
- * the master - register the clone machine -
+ * the master - register the clone machine.
  * 
  * @author Andrea Turli
  */
@@ -88,7 +89,7 @@ public class CloneAndRegisterMachineFromIMachineIfNotAlreadyExists implements Fu
                || e.getMessage().contains("Could not find a registered machine with UUID {");
    }
 
-   private synchronized IMachine cloneMachine(CloneSpec cloneSpec) {
+   private IMachine cloneMachine(CloneSpec cloneSpec) {
       VmSpec vmSpec = cloneSpec.getVmSpec();
       NetworkSpec networkSpec = cloneSpec.getNetworkSpec();
       boolean isLinkedClone = cloneSpec.isLinked();
diff --git a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/NodeCreator.java b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/NodeCreator.java
index a4ec912af4..900118dc89 100644
--- a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/NodeCreator.java
+++ b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/NodeCreator.java
@@ -56,13 +56,13 @@ public class NodeCreator implements Function<NodeSpec, NodeAndInitialCredentials
 
    // TODO parameterize
    public static final int NODE_PORT_INIT = 3000;
-   
+
    // TODO parameterize
    public static final String VMS_NETWORK = "33.33.33.";
-   
+
    // TODO parameterize
    public static final String HOST_ONLY_IFACE_NAME = "vboxnet0";
-   
+
    // TODO parameterize
    public static final boolean USE_LINKED = true;
 
@@ -84,8 +84,13 @@ public class NodeCreator implements Function<NodeSpec, NodeAndInitialCredentials
       this.imachineToNodeMetadata = imachineToNodeMetadata;
    }
 
+   /**
+    * Creates a clone based on the {@link NodeSpec}. It is synchronized because it needs sole access
+    * to the master. Could be improved by locking on a master basis (would allow concurrent cloning
+    * as long as form different masters."
+    */
    @Override
-   public NodeAndInitialCredentials<IMachine> apply(NodeSpec nodeSpec) {
+   public synchronized NodeAndInitialCredentials<IMachine> apply(NodeSpec nodeSpec) {
 
       checkNotNull(nodeSpec, "NodeSpec");
 
@@ -102,6 +107,7 @@ public class NodeCreator implements Function<NodeSpec, NodeAndInitialCredentials
          session.getConsole().deleteSnapshot(master.getMachine().getCurrentSnapshot().getId());
          session.unlockMachine();
       }
+
       String masterNameWithoutPrefix = master.getSpec().getVmSpec().getVmName().replace(VIRTUALBOX_IMAGE_PREFIX, "");
 
       String cloneName = VIRTUALBOX_NODE_PREFIX + masterNameWithoutPrefix + "-" + nodeSpec.getTag() + "-"
diff --git a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java
index 7374471fa9..750dac9399 100644
--- a/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java
+++ b/labs/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java
@@ -93,6 +93,12 @@ public class StartVBoxIfNotAlreadyRunning implements Supplier<VirtualBoxManager>
 
          runScriptOnNodeFactory.create(host.get(), Statements.exec(vboxwebsrv),
                   runAsRoot(false).wrapInInitScript(false).blockOnComplete(false).nameTask("vboxwebsrv")).init().call();
+         
+         try {
+            // wait for a couple of seconds to make sure vbox has correctly started
+            Thread.sleep(2000L);
+         } catch (InterruptedException e) {
+         }
       }
       manager = managerForNode.apply(host);
       manager.connect(provider.toASCIIString(), identity, credential);