Fix extra params execution

Author: nvazquez

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java

@@ -20,6 +20,7 @@
 
 import java.net.URLEncoder;
 import java.nio.charset.Charset;
+import java.util.Arrays;
 import java.util.List;
 import java.util.UUID;
 
@@ -238,16 +239,32 @@ protected boolean performInstanceConversion(String originalVMName, String source
             script.add("-v");
         }
         if (StringUtils.isNotBlank(extraParams)) {
-            script.add(extraParams);
+            addExtraParamsToScript(extraParams, script);
         }
 
         String logPrefix = String.format("(%s) virt-v2v ovf source: %s progress", originalVMName, sourceOVFDirPath);
         OutputInterpreter.LineByLineOutputLogger outputLogger = new OutputInterpreter.LineByLineOutputLogger(logger, logPrefix);
-        script.execute(outputLogger, serverResource.getConvertInstanceEnv());
+        script.execute(outputLogger);
         int exitValue = script.getExitValue();
         return exitValue == 0;
     }
 
+    protected void addExtraParamsToScript(String extraParams, Script script) {
+        List<String> separatedArgs = Arrays.asList(extraParams.split(" "));
+        int i = 0;
+        while (i < separatedArgs.size()) {
+            String current = separatedArgs.get(i);
+            String next = (i + 1) < separatedArgs.size() ? separatedArgs.get(i + 1) : null;
+            if (next == null || next.startsWith("-")) {
+                script.add(current);
+                i = i + 1;
+            } else {
+                script.add(current, next);
+                i = i + 2;
+            }
+        }
+    }
+
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java

@@ -171,4 +171,22 @@ public void testExecuteConvertFailure() {
                     Mockito.anyString(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean(), Mockito.nullable(String.class), Mockito.any(LibvirtComputingResource.class));
         }
     }
+
+    @Test
+    public void testAddExtraParamsToScriptSameKeysAndValues() {
+        Script script = Mockito.mock(Script.class);
+        String extraParams = "--mac 00:0c:29:e6:3d:9d:ip:192.168.0.89,192.168.0.1,24,192.168.0.254";
+        convertInstanceCommandWrapper.addExtraParamsToScript(extraParams, script);
+        Mockito.verify(script).add("--mac", "00:0c:29:e6:3d:9d:ip:192.168.0.89,192.168.0.1,24,192.168.0.254");
+    }
+
+    @Test
+    public void testAddExtraParamsToScriptDifferentArgs() {
+        Script script = Mockito.mock(Script.class);
+        String extraParams = "--mac 00:0c:29:e6:3d:9d:ip:192.168.0.89,192.168.0.1,24,192.168.0.254 -x -v";
+        convertInstanceCommandWrapper.addExtraParamsToScript(extraParams, script);
+        Mockito.verify(script).add("--mac", "00:0c:29:e6:3d:9d:ip:192.168.0.89,192.168.0.1,24,192.168.0.254");
+        Mockito.verify(script).add("-x");
+        Mockito.verify(script).add("-v");
+    }
 }

utils/src/main/java/com/cloud/utils/script/Script.java

@@ -43,7 +43,6 @@
 
 import org.apache.cloudstack.utils.security.KeyStoreUtils;
 import org.apache.commons.io.IOUtils;
-import org.apache.commons.lang3.ArrayUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.joda.time.Duration;
@@ -237,14 +236,6 @@ static String stackTraceAsString(Throwable throwable) {
     }
 
     public String execute(OutputInterpreter interpreter) {
-        return execute(interpreter, null);
-    }
-
-    public String execute(OutputInterpreter interpreter, String[] environment) {
-        return executeInternal(interpreter, environment);
-    }
-
-    public String executeInternal(OutputInterpreter interpreter, String[] environment) {
         String[] command = _command.toArray(new String[_command.size()]);
         String commandLine = buildCommandLine(command);
         if (_logger.isDebugEnabled() && !avoidLoggingCommand) {
@@ -254,22 +245,13 @@ public String executeInternal(OutputInterpreter interpreter, String[] environmen
         try {
             _logger.trace(String.format("Creating process for command [%s].", commandLine));
 
-            if (ArrayUtils.isNotEmpty(environment)) {
-                // Since Runtime.exec() does not support redirecting the error stream, then append 2>&1 to the command
-                String[] commands = new String[] {"sh", "-c", String.format("%s 2>&1", commandLine)};
-                // The PATH variable must be added for indirect calls within the running command
-                // Example: virt-v2v invokes qemu-img, which cannot be found if PATH is not set
-                String[] env = ArrayUtils.add(environment, String.format("PATH=%s", System.getenv("PATH")));
-                _process = Runtime.getRuntime().exec(commands, env, _workDir != null ? new File(_workDir) : null);
-            } else {
-                ProcessBuilder pb = new ProcessBuilder(command);
-                pb.redirectErrorStream(true);
-                if (_workDir != null)
-                    pb.directory(new File(_workDir));
+            ProcessBuilder pb = new ProcessBuilder(command);
+            pb.redirectErrorStream(true);
+            if (_workDir != null)
+                pb.directory(new File(_workDir));
 
-                _logger.trace(String.format("Starting process for command [%s].", commandLine));
-                _process = pb.start();
-            }
+            _logger.trace(String.format("Starting process for command [%s].", commandLine));
+            _process = pb.start();
             if (_process == null) {
                 _logger.warn(String.format("Unable to execute command [%s] because no process was created.", commandLine));
                 return "Unable to execute the command: " + command[0];