Index: /trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp =================================================================== --- /trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp (revision 35936) +++ /trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp (revision 35937) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2010 Oracle Corporation + * Copyright (C) 2011 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -76,4 +76,31 @@ } DIRECTORYENTRY, *PDIRECTORYENTRY; +/* + * Special exit codes for returning errors/information of a + * started guest process to the command line VBoxManage was started from. + * Useful for e.g. scripting. + */ +enum RTEXITCODE_EXEC +{ + RTEXITCODE_EXEC_SUCCESS = RTEXITCODE_SUCCESS, + RTEXITCODE_EXEC_FAILED = 16, + RTEXITCODE_EXEC_TERM_SIGNAL = 17, + RTEXITCODE_EXEC_TERM_ABEND = 18, + RTEXITCODE_EXEC_TIMEOUT = 19, + RTEXITCODE_EXEC_CANCELED = 20 +}; + +/* + * RTGetOpt-IDs for the guest execution control command line. + */ +enum RTGETOPTDEF_EXEC +{ + RTGETOPTDEF_EXEC_IGNOREORPHANEDPROCESSES = 1000, + RTGETOPTDEF_EXEC_WAITFOREXIT, + RTGETOPTDEF_EXEC_WAITFORSTDOUT, + RTGETOPTDEF_EXEC_WAITFORSTDERR, + RTGETOPTDEF_EXEC_ARGS +}; + #endif /* VBOX_ONLY_DOCS */ @@ -81,25 +108,25 @@ { RTStrmPrintf(pStrm, - "VBoxManage guestcontrol exec[ute] |\n" - " \n" + "VBoxManage guestcontrol | exec[ute]\n" + " --image \n" " --username --password \n" - " [--arguments \"\"]\n" " [--environment \"= [=]\"]\n" - " [--flags ] [--timeout ]\n" - " [--verbose] [--wait-for exit,stdout,stderr||]\n" + " [--timeout ] [--verbose]\n" + " [--wait-exit] [--wait-stdout] [--wait-stdout]\n" + " [-- [] ... []\n" /** @todo Add a "--" parameter (has to be last parameter) to directly execute * stuff, e.g. "VBoxManage guestcontrol execute --username <> ... -- /bin/rm -Rf /foo". */ "\n" - " copyto|cp |\n" + " | copyto|cp\n" " \n" " --username --password \n" " [--dryrun] [--follow] [--recursive] [--verbose]\n" "\n" - " createdir[ectory]|mkdir|md |\n" + " | createdir[ectory]|mkdir|md\n" " \n" " --username --password \n" " [--parents] [--mode ] [--verbose]\n" "\n" - " updateadditions |\n" + " | updateadditions\n" " [--source ] [--verbose]\n" "\n"); @@ -148,5 +175,5 @@ * string. */ -static const char *ctrlExecGetStatus(ULONG uStatus) +static const char *ctrlExecProcessStatusToText(ULONG uStatus) { switch (uStatus) @@ -169,6 +196,45 @@ return "error"; default: - return "unknown"; - } + break; + } + return "unknown"; +} + +static int ctrlExecProcessStatusToExitCode(ULONG uStatus) +{ + int rc = RTEXITCODE_EXEC_SUCCESS; + switch (uStatus) + { + case guestControl::PROC_STS_STARTED: + rc = RTEXITCODE_EXEC_SUCCESS; + break; + case guestControl::PROC_STS_TEN: + rc = RTEXITCODE_EXEC_SUCCESS; + break; + case guestControl::PROC_STS_TES: + rc = RTEXITCODE_EXEC_TERM_SIGNAL; + break; + case guestControl::PROC_STS_TEA: + rc = RTEXITCODE_EXEC_TERM_ABEND; + break; + case guestControl::PROC_STS_TOK: + rc = RTEXITCODE_EXEC_TIMEOUT; + break; + case guestControl::PROC_STS_TOA: + rc = RTEXITCODE_EXEC_TIMEOUT; + break; + case guestControl::PROC_STS_DWN: + /* Service/OS is stopping, process was killed, so + * not exactly an error of the started process ... */ + rc = RTEXITCODE_EXEC_SUCCESS; + break; + case guestControl::PROC_STS_ERROR: + rc = RTEXITCODE_EXEC_FAILED; + break; + default: + AssertMsgFailed(("Unknown exit code (%u) from guest process returned!\n", uStatus)); + break; + } + return rc; } @@ -282,22 +348,28 @@ /* */ -static int handleCtrlExecProgram(HandlerArg *a) -{ +static int handleCtrlExecProgram(ComPtr guest, HandlerArg *pArg) +{ + AssertPtrReturn(pArg, VERR_INVALID_PARAMETER); + /* * Parse arguments. */ - if (a->argc < 2) /* At least the command we want to execute in the guest should be present :-). */ + if (pArg->argc < 2) /* At least the command we want to execute in the guest should be present :-). */ return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); static const RTGETOPTDEF s_aOptions[] = { - { "--arguments", 'a', RTGETOPT_REQ_STRING }, - { "--environment", 'e', RTGETOPT_REQ_STRING }, - { "--flags", 'f', RTGETOPT_REQ_STRING }, - { "--password", 'p', RTGETOPT_REQ_STRING }, - { "--timeout", 't', RTGETOPT_REQ_UINT32 }, - { "--username", 'u', RTGETOPT_REQ_STRING }, - { "--verbose", 'v', RTGETOPT_REQ_NOTHING }, - { "--wait-for", 'w', RTGETOPT_REQ_STRING } + { "--environment", 'e', RTGETOPT_REQ_STRING }, + { "--flags", 'f', RTGETOPT_REQ_STRING }, + { "--ignore-operhaned-processes", RTGETOPTDEF_EXEC_IGNOREORPHANEDPROCESSES, RTGETOPT_REQ_NOTHING }, + { "--image", 'i', RTGETOPT_REQ_STRING }, + { "--password", 'p', RTGETOPT_REQ_STRING }, + { "--timeout", 't', RTGETOPT_REQ_UINT32 }, + { "--username", 'u', RTGETOPT_REQ_STRING }, + { "--verbose", 'v', RTGETOPT_REQ_NOTHING }, + { "--wait-exit", RTGETOPTDEF_EXEC_WAITFOREXIT, RTGETOPT_REQ_NOTHING }, + { "--wait-stdout", RTGETOPTDEF_EXEC_WAITFORSTDOUT, RTGETOPT_REQ_NOTHING }, + { "--wait-stderr", RTGETOPTDEF_EXEC_WAITFORSTDERR, RTGETOPT_REQ_NOTHING }, + { "--", RTGETOPTDEF_EXEC_ARGS, RTGETOPT_REQ_STRING } }; @@ -305,5 +377,5 @@ RTGETOPTUNION ValueUnion; RTGETOPTSTATE GetState; - RTGetOptInit(&GetState, a->argc, a->argv, s_aOptions, RT_ELEMENTS(s_aOptions), 1, 0); + RTGetOptInit(&GetState, pArg->argc, pArg->argv, s_aOptions, RT_ELEMENTS(s_aOptions), 0, 0); Utf8Str Utf8Cmd; @@ -327,20 +399,4 @@ switch (ch) { - case 'a': /* Arguments */ - { - char **papszArg; - int cArgs; - - vrc = RTGetOptArgvFromString(&papszArg, &cArgs, ValueUnion.psz, NULL); - if (RT_SUCCESS(vrc)) - { - for (int j = 0; j < cArgs; j++) - args.push_back(Bstr(papszArg[j]).raw()); - - RTGetOptArgvFree(papszArg); - } - break; - } - case 'e': /* Environment */ { @@ -349,5 +405,7 @@ vrc = RTGetOptArgvFromString(&papszArg, &cArgs, ValueUnion.psz, NULL); - if (RT_SUCCESS(vrc)) + if (RT_FAILURE(vrc)) + return errorSyntax(USAGE_GUESTCONTROL, "Failed to parse environment value, rc=%Rrc", vrc); + else { for (int j = 0; j < cArgs; j++) @@ -359,18 +417,13 @@ } - case 'f': /* Flags */ - /** @todo r=bird: As stated before, this generic flags stuff - * does NOT make sense! The IgnoreOprphanedProcesses - * features should have been added as: - * { "--ignore-operhaned-processes", DEFINE, RTGETOPT_REQ_NOTHING } - * - * Please, remove -f in 4.1, replace it with above. */ - /** @todo Needs a bit better processing as soon as we have more flags. */ - /** @todo Add a hidden flag. */ - if (!RTStrICmp(ValueUnion.psz, "ignoreorphanedprocesses")) - fFlags |= ExecuteProcessFlag_IgnoreOrphanedProcesses; - else - fUsageOK = false; - break; + case 'i': + Utf8Cmd = ValueUnion.psz; + break; + + case RTGETOPTDEF_EXEC_IGNOREORPHANEDPROCESSES: + fFlags |= ExecuteProcessFlag_IgnoreOrphanedProcesses; + break; + + /** @todo Add a hidden flag. */ case 'p': /* Password */ @@ -390,47 +443,55 @@ break; - case 'w': /* Wait for ... */ + case RTGETOPTDEF_EXEC_WAITFOREXIT: + fWaitForExit = true; + break; + + case RTGETOPTDEF_EXEC_WAITFORSTDOUT: + fWaitForExit = true; + fWaitForStdOut = true; + break; + + case RTGETOPTDEF_EXEC_WAITFORSTDERR: + fWaitForExit = true; + fWaitForStdErr = true; + break; + + case RTGETOPTDEF_EXEC_ARGS: { - /** @todo r=bird: Same as for -f: Use individual options for - * indicating what to wait for. This is unix tradition and - * much simpler to write code for. It also avoids people - * finding out that the following sequence does not work - * (contrary to what one would expect): - * -w stdout,stderr - * */ - if (!RTStrICmp(ValueUnion.psz, "exit")) - fWaitForExit = true; - else if (!RTStrICmp(ValueUnion.psz, "stdout")) - { - fWaitForExit = true; - fWaitForStdOut = true; - } - else if (!RTStrICmp(ValueUnion.psz, "stderr")) - { - fWaitForExit = true; - fWaitForStdErr = true; - } + /* Push current parameter to vector. */ + args.push_back(Bstr(ValueUnion.psz).raw()); + + /* + * Add all following parameters after this one to our guest process + * argument vector. + */ + while ( (ch = RTGetOpt(&GetState, &ValueUnion)) + && RT_SUCCESS(vrc)) + { + /* + * Is this option unknown or not recognized an option? Then just + * add the raw string value to our argument vector. + */ + if ( ch == VINF_GETOPT_NOT_OPTION + || ch == VERR_GETOPT_UNKNOWN_OPTION) + args.push_back(Bstr(ValueUnion.psz).raw()); + /* + * If this is an option/parameter we already defined for our actual + * execution command we need to take the pDef->pszLong value instead. + */ + else if (ValueUnion.pDef) + args.push_back(Bstr(ValueUnion.pDef->pszLong).raw()); + else + AssertMsgFailed(("Unknown parameter type detected!\n")); + } + break; + } + + case VINF_GETOPT_NOT_OPTION: + if (Utf8Cmd.isEmpty()) + Utf8Cmd = ValueUnion.psz; else - fUsageOK = false; - break; - } - - case VINF_GETOPT_NOT_OPTION: - { - /** @todo r=bird: Guess what the following does: - * VBoxManage guestcontrol exec myvm /bin/ls 1 2 3 4; - * - * In 4.1 this SHALL be changed to treat 1 2 3 4 as arguments - * to /bin/ls. Drop --arguments and replace it with --image - * . The --image defaults to the first argument if - * not specified. Users should be encouraged to use '--' to - * separate 'exec' options from options to the guest - * program: - * VBoxManage guestcontrol myvm exec --image /bin/busybox -- ln -s /foo /bar - */ - /* The actual command we want to execute on the guest. */ - Utf8Cmd = ValueUnion.psz; - break; - } + return RTGetOptPrintError(ch, &ValueUnion); + break; default: @@ -439,6 +500,9 @@ } - if (!fUsageOK) /** @todo r=bird: there is no clean up, so just return directly on failure with a specific message. */ - return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); + if (RT_FAILURE(vrc)) + { + RTMsgError("Failed to parse argument '%c', rc=%Rrc", ch, vrc); + return RTEXITCODE_FAILURE; + } if (Utf8Cmd.isEmpty()) @@ -448,236 +512,211 @@ return errorSyntax(USAGE_GUESTCONTROL, "No user name specified!"); - /** @todo r=bird: You don't check vrc here, so if RTGetOptArgvFromString - * or RTGetOptArgvFromString failed above, you'll ignore it. - * - * Just simplify the argument parsing to not use unnecessary state - * variables and instead return directly on error with a specific error - * message. (See fUsageOk comment above.) */ - - /* - * - */ HRESULT rc = S_OK; - ComPtr guest; - vrc = ctrlInitVM(a, a->argv[0] /* VM Name */, &guest); - if (RT_SUCCESS(vrc)) + if (fVerbose) + { + if (u32TimeoutMS == 0) + RTPrintf("Waiting for guest to start process ...\n"); + else + RTPrintf("Waiting for guest to start process (within %ums)\n", u32TimeoutMS); + } + + /* Get current time stamp to later calculate rest of timeout left. */ + uint64_t u64StartMS = RTTimeMilliTS(); + + /* Execute the process. */ + int rcProc = RTEXITCODE_EXEC_SUCCESS; + ComPtr progress; + ULONG uPID = 0; + rc = guest->ExecuteProcess(Bstr(Utf8Cmd).raw(), + fFlags, + ComSafeArrayAsInParam(args), + ComSafeArrayAsInParam(env), + Bstr(Utf8UserName).raw(), + Bstr(Utf8Password).raw(), + u32TimeoutMS, + &uPID, + progress.asOutParam()); + if (FAILED(rc)) + vrc = ctrlPrintError(guest, COM_IIDOF(IGuest)); + else { if (fVerbose) - { - if (u32TimeoutMS == 0) - RTPrintf("Waiting for guest to start process ...\n"); - else - RTPrintf("Waiting for guest to start process (within %ums)\n", u32TimeoutMS); - } - - /* Get current time stamp to later calculate rest of timeout left. */ - uint64_t u64StartMS = RTTimeMilliTS(); - - /* Execute the process. */ - ComPtr progress; - ULONG uPID = 0; - rc = guest->ExecuteProcess(Bstr(Utf8Cmd).raw(), - fFlags, - ComSafeArrayAsInParam(args), - ComSafeArrayAsInParam(env), - Bstr(Utf8UserName).raw(), - Bstr(Utf8Password).raw(), - u32TimeoutMS, - &uPID, - progress.asOutParam()); - if (FAILED(rc)) - vrc = ctrlPrintError(guest, COM_IIDOF(IGuest)); - else - { - if (fVerbose) - RTPrintf("Process '%s' (PID: %u) started\n", Utf8Cmd.c_str(), uPID); - if (fWaitForExit) + RTPrintf("Process '%s' (PID: %u) started\n", Utf8Cmd.c_str(), uPID); + if (fWaitForExit) + { + if (u32TimeoutMS) /* Wait with a certain timeout. */ { - if (u32TimeoutMS) /* Wait with a certain timeout. */ - { - /* Calculate timeout value left after process has been started. */ - uint64_t u64Elapsed = RTTimeMilliTS() - u64StartMS; - /* Is timeout still bigger than current difference? */ - if (u32TimeoutMS > u64Elapsed) + /* Calculate timeout value left after process has been started. */ + uint64_t u64Elapsed = RTTimeMilliTS() - u64StartMS; + /* Is timeout still bigger than current difference? */ + if (u32TimeoutMS > u64Elapsed) + { + if (fVerbose) + RTPrintf("Waiting for process to exit (%ums left) ...\n", u32TimeoutMS - u64Elapsed); + } + else + { + if (fVerbose) + RTPrintf("No time left to wait for process!\n"); + } + } + else if (fVerbose) /* Wait forever. */ + RTPrintf("Waiting for process to exit ...\n"); + + /* Setup signal handling if cancelable. */ + ASSERT(progress); + bool fCanceledAlready = false; + BOOL fCancelable; + HRESULT hrc = progress->COMGETTER(Cancelable)(&fCancelable); + if (FAILED(hrc)) + fCancelable = FALSE; + if (fCancelable) + ctrlSignalHandlerInstall(); + + /* Wait for process to exit ... */ + BOOL fCompleted = FALSE; + BOOL fCanceled = FALSE; + int cMilliesSleep = 0; + while (SUCCEEDED(progress->COMGETTER(Completed(&fCompleted)))) + { + SafeArray aOutputData; + ULONG cbOutputData = 0; + + /* + * Some data left to output? + */ + if ( fWaitForStdOut + || fWaitForStdErr) + { + rc = guest->GetProcessOutput(uPID, 0 /* aFlags */, + RT_MAX(0, u32TimeoutMS - (RTTimeMilliTS() - u64StartMS)) /* Timeout in ms */, + _64K, ComSafeArrayAsOutParam(aOutputData)); + if (FAILED(rc)) { - if (fVerbose) - RTPrintf("Waiting for process to exit (%ums left) ...\n", u32TimeoutMS - u64Elapsed); + vrc = ctrlPrintError(guest, COM_IIDOF(IGuest)); + cbOutputData = 0; } else { - if (fVerbose) - RTPrintf("No time left to wait for process!\n"); - } - } - else if (fVerbose) /* Wait forever. */ - RTPrintf("Waiting for process to exit ...\n"); - - /* Setup signal handling if cancelable. */ - ASSERT(progress); - bool fCanceledAlready = false; - BOOL fCancelable; - HRESULT hrc = progress->COMGETTER(Cancelable)(&fCancelable); - if (FAILED(hrc)) - fCancelable = FALSE; - if (fCancelable) - ctrlSignalHandlerInstall(); - - /* Wait for process to exit ... */ - BOOL fCompleted = FALSE; - BOOL fCanceled = FALSE; - int cMilliesSleep = 0; - while (SUCCEEDED(progress->COMGETTER(Completed(&fCompleted)))) - { - SafeArray aOutputData; - ULONG cbOutputData = 0; - - /* - * Some data left to output? - */ - if ( fWaitForStdOut - || fWaitForStdErr) - { - rc = guest->GetProcessOutput(uPID, 0 /* aFlags */, - RT_MAX(0, u32TimeoutMS - (RTTimeMilliTS() - u64StartMS)) /* Timeout in ms */, - _64K, ComSafeArrayAsOutParam(aOutputData)); - if (FAILED(rc)) + cbOutputData = aOutputData.size(); + if (cbOutputData > 0) { - vrc = ctrlPrintError(guest, COM_IIDOF(IGuest)); - cbOutputData = 0; - } - else - { - cbOutputData = aOutputData.size(); - if (cbOutputData > 0) +/** @todo r=bird: cat'ing binary data from the guest is not going to work +* reliably if we do conversions like this. Should probably just +* write the output as it is by default, but bypassing RTStrWrite and +* it's automatic translation. Adding exec options to convert unix2dos +* and dos2unix. Use a VFS I/O stream filter for doing this, it's a +* generic problem and the new VFS APIs will handle it more +* transparently. (requires writing dos2unix/unix2dos filters ofc) */ + /* aOutputData has a platform dependent line ending, standardize on + * Unix style, as RTStrmWrite does the LF -> CR/LF replacement on + * Windows. Otherwise we end up with CR/CR/LF on Windows. */ + ULONG cbOutputDataPrint = cbOutputData; + for (BYTE *s = aOutputData.raw(), *d = s; + s - aOutputData.raw() < (ssize_t)cbOutputData; + s++, d++) { -/** @todo r=bird: cat'ing binary data from the guest is not going to work - * reliably if we do conversions like this. Should probably just - * write the output as it is by default, but bypassing RTStrWrite and - * it's automatic translation. Adding exec options to convert unix2dos - * and dos2unix. Use a VFS I/O stream filter for doing this, it's a - * generic problem and the new VFS APIs will handle it more - * transparently. (requires writing dos2unix/unix2dos filters ofc) */ - /* aOutputData has a platform dependent line ending, standardize on - * Unix style, as RTStrmWrite does the LF -> CR/LF replacement on - * Windows. Otherwise we end up with CR/CR/LF on Windows. */ - ULONG cbOutputDataPrint = cbOutputData; - for (BYTE *s = aOutputData.raw(), *d = s; - s - aOutputData.raw() < (ssize_t)cbOutputData; - s++, d++) + if (*s == '\r') { - if (*s == '\r') - { - /* skip over CR, adjust destination */ - d--; - cbOutputDataPrint--; - } - else if (s != d) - *d = *s; + /* skip over CR, adjust destination */ + d--; + cbOutputDataPrint--; } - RTStrmWrite(g_pStdOut, aOutputData.raw(), cbOutputDataPrint); + else if (s != d) + *d = *s; } + RTStrmWrite(g_pStdOut, aOutputData.raw(), cbOutputDataPrint); } } - - /* No more output data left? */ - if (cbOutputData <= 0) - { - /* Only break out from process handling loop if we processed (displayed) - * all output data or if there simply never was output data and the process - * has been marked as complete. */ - if (fCompleted) - break; - - /* Then wait a little while ... */ - progress->WaitForCompletion(1 /* ms */); - } - - /* Process async cancelation */ - if (g_fGuestCtrlCanceled && !fCanceledAlready) - { - hrc = progress->Cancel(); - if (SUCCEEDED(hrc)) - fCanceledAlready = TRUE; - else - g_fGuestCtrlCanceled = false; - } - - /* Progress canceled by Main API? */ - if ( SUCCEEDED(progress->COMGETTER(Canceled(&fCanceled))) - && fCanceled) + } + + /* No more output data left? */ + if (cbOutputData <= 0) + { + /* Only break out from process handling loop if we processed (displayed) + * all output data or if there simply never was output data and the process + * has been marked as complete. */ + if (fCompleted) break; - /* Did we run out of time? */ - if ( u32TimeoutMS - && RTTimeMilliTS() - u64StartMS > u32TimeoutMS) - { - progress->Cancel(); - break; - } - - /* Don't hog the CPU in a busy loop! */ + /* Then wait a little while ... */ + progress->WaitForCompletion(1 /* ms */); + } + + /* Process async cancelation */ + if (g_fGuestCtrlCanceled && !fCanceledAlready) + { + hrc = progress->Cancel(); + if (SUCCEEDED(hrc)) + fCanceledAlready = TRUE; + else + g_fGuestCtrlCanceled = false; + } + + /* Progress canceled by Main API? */ + if ( SUCCEEDED(progress->COMGETTER(Canceled(&fCanceled))) + && fCanceled) + break; + + /* Did we run out of time? */ + if ( u32TimeoutMS + && RTTimeMilliTS() - u64StartMS > u32TimeoutMS) + { + progress->Cancel(); + break; + } + + /* Don't hog the CPU in a busy loop! */ /** @todo r=bird: I believe I already mentioned that this problem is better - * solved by using WaitForCompletion and GetProcessOutput with timeouts. The - * 1ms hack above is not what I had in mind. This quick fix must go away. */ - if (cbOutputData <= 0) - { - if (cMilliesSleep < 100) - cMilliesSleep++; - RTThreadSleep(cMilliesSleep); - } - else - cMilliesSleep = 0; - } /* while */ - - /* Undo signal handling */ - if (fCancelable) - ctrlSignalHandlerUninstall(); - - /* Report status back to the user. */ - if (fCanceled) - { - if (fVerbose) - RTPrintf("Process execution canceled!\n"); - } - else if ( fCompleted - && SUCCEEDED(rc)) - { - LONG iRc; - CHECK_ERROR_RET(progress, COMGETTER(ResultCode)(&iRc), rc); - if (FAILED(iRc)) - vrc = ctrlPrintProgressError(progress); - else if (fVerbose) - { - ULONG uRetStatus, uRetExitCode, uRetFlags; - rc = guest->GetProcessStatus(uPID, &uRetExitCode, &uRetFlags, &uRetStatus); - if (SUCCEEDED(rc)) - RTPrintf("Exit code=%u (Status=%u [%s], Flags=%u)\n", uRetExitCode, uRetStatus, ctrlExecGetStatus(uRetStatus), uRetFlags); - /** @todo r=bird: The guest application exit code - * should by default be returned by the command. - * Non-normal exits and exit codes outside - * the portable range (0..127) should be - * translated to more portable values. - * - * Alternatively, we could return say exit code 16 if - * the guest command failed, 17 if it terminated on a - * signal, 18 if it abended and exit code 19 if it - * timed out. This is possibly a better solution. - * - * Think about people using VBoxManage for scripting. */ - } +* solved by using WaitForCompletion and GetProcessOutput with timeouts. The +* 1ms hack above is not what I had in mind. This quick fix must go away. */ + if (cbOutputData <= 0) + { + if (cMilliesSleep < 100) + cMilliesSleep++; + RTThreadSleep(cMilliesSleep); } else - { - if (fVerbose) - RTPrintf("Process execution aborted!\n"); - } + cMilliesSleep = 0; + } /* while */ + + /* Undo signal handling */ + if (fCancelable) + ctrlSignalHandlerUninstall(); + + /* Report status back to the user. */ + if (fCanceled) + { + if (fVerbose) + RTPrintf("Process execution canceled!\n"); + rcProc = RTEXITCODE_EXEC_CANCELED; } - } - ctrlUninitVM(a); + else if ( fCompleted + && SUCCEEDED(rc)) + { + LONG iRc; + CHECK_ERROR_RET(progress, COMGETTER(ResultCode)(&iRc), rc); + if (FAILED(iRc)) + vrc = ctrlPrintProgressError(progress); + else + { + ULONG uRetStatus, uRetExitCode, uRetFlags; + rc = guest->GetProcessStatus(uPID, &uRetExitCode, &uRetFlags, &uRetStatus); + if (SUCCEEDED(rc) && fVerbose) + RTPrintf("Exit code=%u (Status=%u [%s], Flags=%u)\n", uRetExitCode, uRetStatus, ctrlExecProcessStatusToText(uRetStatus), uRetFlags); + rcProc = ctrlExecProcessStatusToExitCode(uRetStatus); + } + } + else + { + if (fVerbose) + RTPrintf("Process execution aborted!\n"); + } + } } if (RT_FAILURE(vrc) || FAILED(rc)) return RTEXITCODE_FAILURE; - return RTEXITCODE_SUCCESS; + return rcProc; } @@ -1047,4 +1086,5 @@ uint32_t fFlags) { + AssertPtrReturn(pGuest, VERR_INVALID_PARAMETER); AssertPtrReturn(pszSource, VERR_INVALID_PARAMETER); AssertPtrReturn(pszDest, VERR_INVALID_PARAMETER); @@ -1068,11 +1108,13 @@ } -static int handleCtrlCopyTo(HandlerArg *a) -{ +static int handleCtrlCopyTo(ComPtr guest, HandlerArg *pArg) +{ + AssertPtrReturn(pArg, VERR_INVALID_PARAMETER); + /* * Check the syntax. We can deduce the correct syntax from the number of * arguments. */ - if (a->argc < 3) /* At least the source + destination should be present :-). */ + if (pArg->argc < 2) /* At least the source + destination should be present :-). */ return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); @@ -1090,5 +1132,5 @@ RTGETOPTUNION ValueUnion; RTGETOPTSTATE GetState; - RTGetOptInit(&GetState, a->argc, a->argv, s_aOptions, RT_ELEMENTS(s_aOptions), 1, 0); + RTGetOptInit(&GetState, pArg->argc, pArg->argv, s_aOptions, RT_ELEMENTS(s_aOptions), 0, 0); Utf8Str Utf8Source; @@ -1179,88 +1221,84 @@ "No user name specified!"); HRESULT rc = S_OK; - ComPtr guest; - vrc = ctrlInitVM(a, a->argv[0] /* VM Name */, &guest); - if (RT_SUCCESS(vrc)) - { - if (fVerbose) - { - if (fDryRun) - RTPrintf("Dry run - no files copied!\n"); - RTPrintf("Gathering file information ...\n"); - } - - RTLISTNODE listToCopy; - uint32_t cObjects = 0; - vrc = ctrlCopyInit(Utf8Source.c_str(), Utf8Dest.c_str(), fFlags, - &cObjects, &listToCopy); - if (RT_FAILURE(vrc)) - { - switch (vrc) + if (fVerbose) + { + if (fDryRun) + RTPrintf("Dry run - no files copied!\n"); + RTPrintf("Gathering file information ...\n"); + } + + RTLISTNODE listToCopy; + uint32_t cObjects = 0; + vrc = ctrlCopyInit(Utf8Source.c_str(), Utf8Dest.c_str(), fFlags, + &cObjects, &listToCopy); + if (RT_FAILURE(vrc)) + { + switch (vrc) + { + case VERR_NOT_FOUND: + RTMsgError("No files to copy found!\n"); + break; + + case VERR_FILE_NOT_FOUND: + RTMsgError("Source path \"%s\" not found!\n", Utf8Source.c_str()); + break; + + default: + RTMsgError("Failed to initialize, rc=%Rrc\n", vrc); + break; + } + } + else + { + PDIRECTORYENTRY pNode; + if (RT_SUCCESS(vrc)) + { + if (fVerbose) { - case VERR_NOT_FOUND: - RTMsgError("No files to copy found!\n"); + if (fCopyRecursive) + RTPrintf("Recursively copying \"%s\" to \"%s\" (%u file(s)) ...\n", + Utf8Source.c_str(), Utf8Dest.c_str(), cObjects); + else + RTPrintf("Copying \"%s\" to \"%s\" (%u file(s)) ...\n", + Utf8Source.c_str(), Utf8Dest.c_str(), cObjects); + } + + uint32_t uCurObject = 1; + RTListForEach(&listToCopy, pNode, DIRECTORYENTRY, Node) + { + if (!fDryRun) + { + if (fVerbose) + RTPrintf("Copying \"%s\" to \"%s\" (%u/%u) ...\n", + pNode->pszSourcePath, pNode->pszDestPath, uCurObject, cObjects); + /* Finally copy the desired file (if no dry run selected). */ + if (!fDryRun) + vrc = ctrlCopyFileToGuest(guest, fVerbose, pNode->pszSourcePath, pNode->pszDestPath, + Utf8UserName.c_str(), Utf8Password.c_str(), fFlags); + } + if (RT_FAILURE(vrc)) break; - - case VERR_FILE_NOT_FOUND: - RTMsgError("Source path \"%s\" not found!\n", Utf8Source.c_str()); - break; - - default: - RTMsgError("Failed to initialize, rc=%Rrc\n", vrc); - break; + uCurObject++; } - } - else - { - PDIRECTORYENTRY pNode; - if (RT_SUCCESS(vrc)) - { - if (fVerbose) - { - if (fCopyRecursive) - RTPrintf("Recursively copying \"%s\" to \"%s\" (%u file(s)) ...\n", - Utf8Source.c_str(), Utf8Dest.c_str(), cObjects); - else - RTPrintf("Copying \"%s\" to \"%s\" (%u file(s)) ...\n", - Utf8Source.c_str(), Utf8Dest.c_str(), cObjects); - } - - uint32_t uCurObject = 1; - RTListForEach(&listToCopy, pNode, DIRECTORYENTRY, Node) - { - if (!fDryRun) - { - if (fVerbose) - RTPrintf("Copying \"%s\" to \"%s\" (%u/%u) ...\n", - pNode->pszSourcePath, pNode->pszDestPath, uCurObject, cObjects); - /* Finally copy the desired file (if no dry run selected). */ - if (!fDryRun) - vrc = ctrlCopyFileToGuest(guest, fVerbose, pNode->pszSourcePath, pNode->pszDestPath, - Utf8UserName.c_str(), Utf8Password.c_str(), fFlags); - } - if (RT_FAILURE(vrc)) - break; - uCurObject++; - } - if (RT_SUCCESS(vrc) && fVerbose) - RTPrintf("Copy operation successful!\n"); - } - ctrlDirectoryListDestroy(&listToCopy); - } - ctrlUninitVM(a); + if (RT_SUCCESS(vrc) && fVerbose) + RTPrintf("Copy operation successful!\n"); + } + ctrlDirectoryListDestroy(&listToCopy); } if (RT_FAILURE(vrc)) rc = VBOX_E_IPRT_ERROR; - return SUCCEEDED(rc) ? 0 : 1; -} - -static int handleCtrlCreateDirectory(HandlerArg *a) -{ + return SUCCEEDED(rc) ? RTEXITCODE_SUCCESS : RTEXITCODE_FAILURE; +} + +static int handleCtrlCreateDirectory(ComPtr guest, HandlerArg *pArg) +{ + AssertPtrReturn(pArg, VERR_INVALID_PARAMETER); + /* * Check the syntax. We can deduce the correct syntax from the number of * arguments. */ - if (a->argc < 2) /* At least the directory we want to create should be present :-). */ + if (pArg->argc < 2) /* At least the directory we want to create should be present :-). */ return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); @@ -1277,5 +1315,5 @@ RTGETOPTUNION ValueUnion; RTGETOPTSTATE GetState; - RTGetOptInit(&GetState, a->argc, a->argv, s_aOptions, RT_ELEMENTS(s_aOptions), 1, 0); + RTGetOptInit(&GetState, pArg->argc, pArg->argv, s_aOptions, RT_ELEMENTS(s_aOptions), 0, 0); Utf8Str Utf8UserName; @@ -1351,43 +1389,36 @@ HRESULT rc = S_OK; - ComPtr guest; - vrc = ctrlInitVM(a, a->argv[0] /* VM Name */, &guest); - if (RT_SUCCESS(vrc)) - { - if (fVerbose && uNumDirs > 1) - RTPrintf("Creating %ld directories ...\n", uNumDirs); - - PDIRECTORYENTRY pNode; - RTListForEach(&listDirs, pNode, DIRECTORYENTRY, Node) - { - if (fVerbose) - RTPrintf("Creating directory \"%s\" ...\n", pNode->pszDestPath); - - ComPtr progress; - rc = guest->CreateDirectory(Bstr(pNode->pszDestPath).raw(), - Bstr(Utf8UserName).raw(), Bstr(Utf8Password).raw(), - uMode, fFlags, progress.asOutParam()); - if (FAILED(rc)) - { - vrc = ctrlPrintError(guest, COM_IIDOF(IGuest)); - break; - } - } - ctrlUninitVM(a); + if (fVerbose && uNumDirs > 1) + RTPrintf("Creating %u directories ...\n", uNumDirs); + + PDIRECTORYENTRY pNode; + RTListForEach(&listDirs, pNode, DIRECTORYENTRY, Node) + { + if (fVerbose) + RTPrintf("Creating directory \"%s\" ...\n", pNode->pszDestPath); + + ComPtr progress; + rc = guest->CreateDirectory(Bstr(pNode->pszDestPath).raw(), + Bstr(Utf8UserName).raw(), Bstr(Utf8Password).raw(), + uMode, fFlags, progress.asOutParam()); + if (FAILED(rc)) + { + /* rc ignored */ ctrlPrintError(guest, COM_IIDOF(IGuest)); + break; + } } ctrlDirectoryListDestroy(&listDirs); - - if (RT_FAILURE(vrc)) - rc = VBOX_E_IPRT_ERROR; - return SUCCEEDED(rc) ? 0 : 1; -} - -static int handleCtrlUpdateAdditions(HandlerArg *a) -{ + return SUCCEEDED(rc) ? RTEXITCODE_SUCCESS : RTEXITCODE_FAILURE; +} + +static int handleCtrlUpdateAdditions(ComPtr guest, HandlerArg *pArg) +{ + AssertPtrReturn(pArg, VERR_INVALID_PARAMETER); + /* * Check the syntax. We can deduce the correct syntax from the number of * arguments. */ - if (a->argc < 1) /* At least the VM name should be present :-). */ + if (pArg->argc < 1) /* At least the VM name should be present :-). */ return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); @@ -1395,97 +1426,95 @@ bool fVerbose = false; -/** @todo r=bird: Use RTGetOpt here, no new code using strcmp-if-switching! */ - /* Iterate through all possible commands (if available). */ - bool usageOK = true; - for (int i = 1; usageOK && i < a->argc; i++) - { - if (!strcmp(a->argv[i], "--source")) - { - if (i + 1 >= a->argc) - usageOK = false; - else - { - Utf8Source = a->argv[i + 1]; - ++i; - } - } - else if (!strcmp(a->argv[i], "--verbose")) - fVerbose = true; + static const RTGETOPTDEF s_aOptions[] = + { + { "--source", 's', RTGETOPT_REQ_STRING }, + { "--verbose", 'v', RTGETOPT_REQ_NOTHING } + }; + + int ch; + RTGETOPTUNION ValueUnion; + RTGETOPTSTATE GetState; + RTGetOptInit(&GetState, pArg->argc, pArg->argv, s_aOptions, RT_ELEMENTS(s_aOptions), 0, 0); + + int vrc = VINF_SUCCESS; + while ( (ch = RTGetOpt(&GetState, &ValueUnion)) + && RT_SUCCESS(vrc)) + { + switch (ch) + { + case 's': + Utf8Source = ValueUnion.psz; + break; + + case 'v': + fVerbose = true; + break; + + default: + return RTGetOptPrintError(ch, &ValueUnion); + } + } + + if (fVerbose) + RTPrintf("Updating Guest Additions ...\n"); + +#ifdef DEBUG_andy + if (Utf8Source.isEmpty()) + Utf8Source = "c:\\Downloads\\VBoxGuestAdditions-r67158.iso"; +#endif + + /* Determine source if not set yet. */ + if (Utf8Source.isEmpty()) + { + char strTemp[RTPATH_MAX]; + vrc = RTPathAppPrivateNoArch(strTemp, sizeof(strTemp)); + AssertRC(vrc); + Utf8Str Utf8Src1 = Utf8Str(strTemp).append("/VBoxGuestAdditions.iso"); + + vrc = RTPathExecDir(strTemp, sizeof(strTemp)); + AssertRC(vrc); + Utf8Str Utf8Src2 = Utf8Str(strTemp).append("/additions/VBoxGuestAdditions.iso"); + + /* Check the standard image locations */ + if (RTFileExists(Utf8Src1.c_str())) + Utf8Source = Utf8Src1; + else if (RTFileExists(Utf8Src2.c_str())) + Utf8Source = Utf8Src2; else - return errorSyntax(USAGE_GUESTCONTROL, - "Invalid parameter '%s'", Utf8Str(a->argv[i]).c_str()); - } - - if (!usageOK) - return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); - - HRESULT rc = S_OK; - ComPtr guest; - int vrc = ctrlInitVM(a, a->argv[0] /* VM Name */, &guest); + { + RTMsgError("Source could not be determined! Please use --source to specify a valid source.\n"); + vrc = VERR_FILE_NOT_FOUND; + } + } + else if (!RTFileExists(Utf8Source.c_str())) + { + RTMsgError("Source \"%s\" does not exist!\n", Utf8Source.c_str()); + vrc = VERR_FILE_NOT_FOUND; + } + if (RT_SUCCESS(vrc)) { if (fVerbose) - RTPrintf("Updating Guest Additions of machine \"%s\" ...\n", a->argv[0]); - -#ifdef DEBUG_andy - if (Utf8Source.isEmpty()) - Utf8Source = "c:\\Downloads\\VBoxGuestAdditions-r67158.iso"; -#endif - /* Determine source if not set yet. */ - if (Utf8Source.isEmpty()) - { - char strTemp[RTPATH_MAX]; - vrc = RTPathAppPrivateNoArch(strTemp, sizeof(strTemp)); - AssertRC(vrc); - Utf8Str Utf8Src1 = Utf8Str(strTemp).append("/VBoxGuestAdditions.iso"); - - vrc = RTPathExecDir(strTemp, sizeof(strTemp)); - AssertRC(vrc); - Utf8Str Utf8Src2 = Utf8Str(strTemp).append("/additions/VBoxGuestAdditions.iso"); - - /* Check the standard image locations */ - if (RTFileExists(Utf8Src1.c_str())) - Utf8Source = Utf8Src1; - else if (RTFileExists(Utf8Src2.c_str())) - Utf8Source = Utf8Src2; - else - { - RTMsgError("Source could not be determined! Please use --source to specify a valid source.\n"); - vrc = VERR_FILE_NOT_FOUND; - } - } - else if (!RTFileExists(Utf8Source.c_str())) - { - RTMsgError("Source \"%s\" does not exist!\n", Utf8Source.c_str()); - vrc = VERR_FILE_NOT_FOUND; - } - - if (RT_SUCCESS(vrc)) - { - if (fVerbose) - RTPrintf("Using source: %s\n", Utf8Source.c_str()); - - ComPtr progress; - CHECK_ERROR(guest, UpdateGuestAdditions(Bstr(Utf8Source).raw(), - /* Wait for whole update process to complete. */ - AdditionsUpdateFlag_None, - progress.asOutParam())); + RTPrintf("Using source: %s\n", Utf8Source.c_str()); + + HRESULT rc = S_OK; + ComPtr progress; + CHECK_ERROR(guest, UpdateGuestAdditions(Bstr(Utf8Source).raw(), + /* Wait for whole update process to complete. */ + AdditionsUpdateFlag_None, + progress.asOutParam())); + if (FAILED(rc)) + vrc = ctrlPrintError(guest, COM_IIDOF(IGuest)); + else + { + rc = showProgress(progress); if (FAILED(rc)) - vrc = ctrlPrintError(guest, COM_IIDOF(IGuest)); - else - { - rc = showProgress(progress); - if (FAILED(rc)) - vrc = ctrlPrintProgressError(progress); - else if (fVerbose) - RTPrintf("Guest Additions update successful.\n"); - } - } - ctrlUninitVM(a); - } - - if (RT_FAILURE(vrc)) - rc = VBOX_E_IPRT_ERROR; - return SUCCEEDED(rc) ? 0 : 1; + vrc = ctrlPrintProgressError(progress); + else if (fVerbose) + RTPrintf("Guest Additions update successful.\n"); + } + } + + return RT_SUCCESS(vrc) ? RTEXITCODE_SUCCESS : RTEXITCODE_FAILURE; } @@ -1496,48 +1525,52 @@ * @note see the command line API description for parameters */ -int handleGuestControl(HandlerArg *a) -{ - /** @todo This command does not follow the syntax where the - * comes between the command and subcommand. The commands controlvm, - * snapshot and debugvm puts it between. The commands guestproperty, - * guestcontrol and sharedfolder puts it after (the latter is - * questionable). - * - * I would propose changing guestcontrol and guestproperty to the controlvm - * syntax in 4.1. Whether sharedfolder should be changed as well is a bit - * more open. - */ - - HandlerArg arg = *a; - arg.argc = a->argc - 1; - arg.argv = a->argv + 1; - - if (a->argc <= 0) +int handleGuestControl(HandlerArg *pArg) +{ + AssertPtrReturn(pArg, VERR_INVALID_PARAMETER); + + /* At least the VM name + sub command needs to be specified. */ + if (pArg->argc <= 2) return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); - /* switch (cmd) */ - /** @todo r=bird: Subcommands are not case insensitive for other commands - * like controlvm. Drop it. */ - if ( !RTStrICmp(a->argv[0], "exec") - || !RTStrICmp(a->argv[0], "execute")) - return handleCtrlExecProgram(&arg); - - if ( !RTStrICmp(a->argv[0], "copyto") - || !RTStrICmp(a->argv[0], "cp")) - return handleCtrlCopyTo(&arg); - - if ( !RTStrICmp(a->argv[0], "createdirectory") - || !RTStrICmp(a->argv[0], "createdir") - || !RTStrICmp(a->argv[0], "mkdir") - || !RTStrICmp(a->argv[0], "md")) - return handleCtrlCreateDirectory(&arg); - - if ( !RTStrICmp(a->argv[0], "updateadditions") - || !RTStrICmp(a->argv[0], "updateadds")) - return handleCtrlUpdateAdditions(&arg); - - /* default: */ - return errorSyntax(USAGE_GUESTCONTROL, "Incorrect parameters"); + HandlerArg arg = *pArg; + arg.argc = pArg->argc - 2; /* Skip VM name and sub command. */ + arg.argv = pArg->argv + 2; /* Same here. */ + + ComPtr guest; + int vrc = ctrlInitVM(pArg, pArg->argv[0] /* VM Name */, &guest); + if (RT_SUCCESS(vrc)) + { + int rcExit; + if ( !strcmp(pArg->argv[1], "exec") + || !strcmp(pArg->argv[1], "execute")) + { + rcExit = handleCtrlExecProgram(guest, &arg); + } + else if ( !strcmp(pArg->argv[1], "copyto") + || !strcmp(pArg->argv[1], "cp")) + { + rcExit = handleCtrlCopyTo(guest, &arg); + } + else if ( !strcmp(pArg->argv[1], "createdirectory") + || !strcmp(pArg->argv[1], "createdir") + || !strcmp(pArg->argv[1], "mkdir") + || !strcmp(pArg->argv[1], "md")) + { + rcExit = handleCtrlCreateDirectory(guest, &arg); + } + else if ( !strcmp(pArg->argv[1], "updateadditions") + || !strcmp(pArg->argv[1], "updateadds")) + { + rcExit = handleCtrlUpdateAdditions(guest, &arg); + } + else + rcExit = errorSyntax(USAGE_GUESTCONTROL, "No sub command specified!"); + + ctrlUninitVM(pArg); + return rcExit; + } + return RTEXITCODE_FAILURE; } #endif /* !VBOX_ONLY_DOCS */ +