Changeset 95139 in vbox for trunk/src/VBox

Timestamp:

May 30, 2022 5:19:09 PM (3 years ago)

Author:

vboxsync

Message:

FE/VBoxAutostart: Lots of bigger and smaller cleanups, especially regarding mixed up return types, better and more (optionally verbose) logging for the Windows event log, added missing syntax help.

Location:

trunk/src/VBox/Frontends/VBoxAutostart

Files:

• VBoxAutostart-posix.cpp (modified) (8 diffs)
• VBoxAutostart-win.cpp (modified) (33 diffs)
• VBoxAutostart.h (modified) (14 diffs)
• VBoxAutostartCfg.cpp (modified) (5 diffs)
• VBoxAutostartStart.cpp (modified) (11 diffs)
• VBoxAutostartStop.cpp (modified) (11 diffs)
• VBoxAutostartUtils.cpp (modified) (5 diffs)

trunk/src/VBox/Frontends/VBoxAutostart/VBoxAutostart-posix.cpp

@@ -248 +248 @@
 }
 
-static void displayHeader()
-{
-    RTStrmPrintf(g_pStdErr, VBOX_PRODUCT " Autostart " VBOX_VERSION_STRING "\n"
-                 "(C) " VBOX_C_YEAR " " VBOX_VENDOR "\n"
-                 "All rights reserved.\n\n");
-}
-
 /**
- * Displays the help.
+ * Shows the help.
  *
  * @param   pszImage                Name of program name (image).
  */
-static void displayHelp(const char *pszImage)
+static void showHelp(const char *pszImage)
 {
     AssertPtrReturnVoid(pszImage);
 
-    displayHeader();
+    autostartSvcShowHeader();
 
     RTStrmPrintf(g_pStdErr,
                  "Usage: %s [-v|--verbose] [-h|-?|--help]\n"
+                 "           [-V|--version]\n"
                  "           [-F|--logfile=<file>] [-R|--logrotate=<num>]\n"
                  "           [-S|--logsize=<bytes>] [-I|--loginterval=<seconds>]\n"
@@ -282 +276 @@
         {
             case 'h':
-                pcszDescr = "Print this help message and exit.";
+                pcszDescr = "Prints this help message and exit.";
                 break;
 
@@ -310 +304 @@
                 pcszDescr = "Name of the configuration file for the global overrides.";
                 break;
+
+            case 'V':
+                pcszDescr = "Shows the service version.";
+                break;
+
             default:
                 AssertFailedBreakStmt(pcszDescr = "");
@@ -357 +356 @@
         {
             case 'h':
-                displayHelp(argv[0]);
-                return 0;
+                showHelp(argv[0]);
+                return RTEXITCODE_SUCCESS;
 
             case 'v':
@@ -370 +369 @@
 #endif
             case 'V':
-                RTPrintf("%sr%s\n", RTBldCfgVersion(), RTBldCfgRevisionStr());
-                return 0;
+                autostartSvcShowVersion(false);
+                return RTEXITCODE_SUCCESS;
 
             case 'F':
@@ -412 +411 @@
     if (!fStart && !fStop)
     {
-        displayHelp(argv[0]);
+        showHelp(argv[0]);
         return RTMsgErrorExit(RTEXITCODE_FAILURE, "Either --start or --stop must be present");
     }
     else if (fStart && fStop)
     {
-        displayHelp(argv[0]);
+        showHelp(argv[0]);
         return RTMsgErrorExit(RTEXITCODE_FAILURE, "--start or --stop are mutually exclusive");
     }
@@ -423 +422 @@
     if (!pszConfigFile)
     {
-        displayHelp(argv[0]);
+        showHelp(argv[0]);
         return RTMsgErrorExit(RTEXITCODE_FAILURE, "--config <config file> is missing");
     }
 
     if (!fQuiet)
-        displayHeader();
+        autostartSvcShowHeader();
 
     PCFGAST pCfgAst = NULL;
@@ -544 +543 @@
         return RTEXITCODE_FAILURE;
 
-    RTEXITCODE rcExit;
     if (fStart)
-        rcExit = autostartStartMain(pCfgAstUser);
+        rc = autostartStartMain(pCfgAstUser);
     else
     {
         Assert(fStop);
-        rcExit = autostartStopMain(pCfgAstUser);
+        rc = autostartStopMain(pCfgAstUser);
     }
 
     autostartConfigAstDestroy(pCfgAst);
     NativeEventQueue::getMainEventQueue()->processEventQueue(0);
-
     autostartShutdown();
-    return rcExit;
+
+    return RT_SUCCESS(rc) ? RTEXITCODE_SUCCESS : RTEXITCODE_FAILURE;
 }
 

trunk/src/VBox/Frontends/VBoxAutostart/VBoxAutostart-win.cpp

@@ -20 +20 @@
 *   Header Files                                                                                                                 *
 *********************************************************************************************************************************/
-#include <iprt/buildconfig.h>
 #include <iprt/dir.h>
 #include <iprt/env.h>
-#include <iprt/errcore.h>
+#include <iprt/err.h>
 #include <iprt/getopt.h>
 #include <iprt/initterm.h>
@@ -51 +50 @@
 
 #include <VBox/log.h>
-#include <VBox/version.h>
 
 #include "VBoxAutostart.h"
@@ -261 +259 @@
 DECLHIDDEN(void) autostartSvcOsLogStr(const char *pszMsg, AUTOSTARTLOGTYPE enmLogType)
 {
-    /* write it to the release log too */
+    /* write it to the console + release log too (if configured). */
     LogRel(("%s", pszMsg));
 
+    /** @todo r=andy Only (un)register source once? */
     HANDLE hEventLog = RegisterEventSourceA(NULL /* local computer */, "VBoxAutostartSvc");
     AssertReturnVoid(hEventLog != NULL);
@@ -274 +273 @@
     {
         case AUTOSTARTLOGTYPE_INFO:
+            RTStrmPrintf(g_pStdOut, "%s", pszMsg);
             wType = 0;
             break;
         case AUTOSTARTLOGTYPE_ERROR:
+            RTStrmPrintf(g_pStdErr, "Error: %s", pszMsg);
             wType = EVENTLOG_ERROR_TYPE;
             break;
         case AUTOSTARTLOGTYPE_WARNING:
+            RTStrmPrintf(g_pStdOut, "Warning: %s", pszMsg);
             wType = EVENTLOG_WARNING_TYPE;
             break;
         case AUTOSTARTLOGTYPE_VERBOSE:
-            if (!g_cVerbosity)
-                return;
+            RTStrmPrintf(g_pStdOut, "%s", pszMsg);
             wType = EVENTLOG_INFORMATION_TYPE;
             break;
         default:
-            AssertMsgFailed(("Invalid log type %d\n", enmLogType));
-    }
-
+            AssertMsgFailed(("Invalid log type %#x\n", enmLogType));
+            break;
+    }
+
+    /** @todo r=andy Why ANSI and not Unicode (xxxW)? */
     BOOL fRc = ReportEventA(hEventLog,               /* hEventLog */
                             wType,                   /* wType */
@@ -300 +303 @@
                             apsz,                    /* lpStrings */
                             NULL);                   /* lpRawData */
-    AssertMsg(fRc, ("%u\n", GetLastError())); NOREF(fRc);
+    AssertMsg(fRc, ("ReportEventA failed with %ld\n", GetLastError())); RT_NOREF(fRc);
     DeregisterEventSource(hEventLog);
 }
@@ -342 +345 @@
  * @returns Valid service handle on success.
  *          NULL on failure, will display an error message unless it's ignored.
+ *          Use GetLastError() to find out what the last Windows error was.
  *
  * @param   pszAction           The action which is requesting access to the service.
@@ -364 +368 @@
     else
     {
-        DWORD   err = GetLastError();
-        bool    fIgnored = false;
+        DWORD const dwErr    = GetLastError();
+        bool        fIgnored = false;
         va_list va;
         va_start(va, cIgnoredErrors);
         while (!fIgnored && cIgnoredErrors-- > 0)
-            fIgnored = (DWORD)va_arg(va, int) == err;
+            fIgnored = (DWORD)va_arg(va, int) == dwErr;
         va_end(va);
         if (!fIgnored)
         {
-            switch (err)
+            switch (dwErr)
             {
                 case ERROR_ACCESS_DENIED:
@@ -383 +387 @@
                     break;
                 default:
-                    autostartSvcDisplayError("%s - OpenService failure: %d\n", pszAction, err);
+                    autostartSvcDisplayError("%s - OpenService failure, rc=%Rrc (%#x)\n", RTErrConvertFromWin32(dwErr), dwErr);
                     break;
             }
@@ -389 +393 @@
 
         CloseServiceHandle(hSCM);
-        SetLastError(err);
+        SetLastError(dwErr);
     }
     return hSvc;
@@ -472 +476 @@
  * @param   argv    The action argument vector.
  */
-static int autostartSvcWinDelete(int argc, char **argv)
+static RTEXITCODE autostartSvcWinDelete(int argc, char **argv)
 {
     /*
@@ -508 +512 @@
     int vrc = autostartGetServiceName(pszUser, sServiceName);
     if (RT_FAILURE(vrc))
-        return autostartSvcDisplayError("delete - DeleteService failed, service name for user %s can not be constructed.\n",
+        return autostartSvcDisplayError("delete - DeleteService failed, service name for user %s cannot be constructed.\n",
                                         pszUser);
     /*
-     * Create the service.
+     * Delete the service.
      */
-    RTEXITCODE rc = RTEXITCODE_FAILURE;
-    SC_HANDLE hSvc = autostartSvcWinOpenService(com::Bstr(sServiceName).raw(), "delete", SERVICE_CHANGE_CONFIG, DELETE,
-                                                1, ERROR_SERVICE_DOES_NOT_EXIST);
+    RTEXITCODE rcExit = RTEXITCODE_FAILURE;
+    SC_HANDLE hSvc = autostartSvcWinOpenService(com::Bstr(sServiceName).raw(), "delete", SERVICE_CHANGE_CONFIG, DELETE, 0);
     if (hSvc)
     {
         if (DeleteService(hSvc))
         {
-            RTPrintf("Successfully deleted the %s service.\n", sServiceName.c_str());
-            rc = RTEXITCODE_SUCCESS;
+            if (g_cVerbosity)
+                RTPrintf("Successfully deleted the %s service.\n", sServiceName.c_str());
+            rcExit = RTEXITCODE_SUCCESS;
         }
         else
-            autostartSvcDisplayError("delete - DeleteService failed, err=%d.\n", GetLastError());
+        {
+            DWORD const dwErr = GetLastError();
+            autostartSvcDisplayError("delete - DeleteService failed, rc=%Rrc (%#x)\n", RTErrConvertFromWin32(dwErr), dwErr);
+        }
         CloseServiceHandle(hSvc);
     }
-    else if (GetLastError() == ERROR_SERVICE_DOES_NOT_EXIST)
-    {
-
-        if (g_cVerbosity)
-            RTPrintf("The service %s was not installed, nothing to be done.", sServiceName.c_str());
-        else
-            RTPrintf("Successfully deleted the %s service.\n", sServiceName.c_str());
-        rc = RTEXITCODE_SUCCESS;
-    }
-    return rc;
+    return rcExit;
 }
 
@@ -786 +784 @@
              */
             int rc = RTSemEventMultiSignal(g_hSupSvcWinEvent);
-            if (RT_FAILURE(rc))
-                autostartSvcLogError("SERVICE_CONTROL_STOP: RTSemEventMultiSignal failed, %Rrc\n", rc);
+            if (RT_FAILURE(rc)) /** @todo r=andy Don't we want to report back an error here to SCM? */
+                autostartSvcLogErrorRc(rc, "SERVICE_CONTROL_STOP: RTSemEventMultiSignal failed, %Rrc\n", rc);
 
             return NO_ERROR;
@@ -805 +803 @@
 }
 
-static RTEXITCODE autostartStartVMs(void)
+static int autostartStartVMs(void)
 {
     int rc = autostartSetup();
     if (RT_FAILURE(rc))
-        return RTEXITCODE_FAILURE;
+        return rc;
 
     const char *pszConfigFile = RTEnvGet("VBOXAUTOSTART_CONFIG");
     if (!pszConfigFile)
-        return autostartSvcLogError("Starting VMs failed. VBOXAUTOSTART_CONFIG environment variable is not defined.\n");
+        return autostartSvcLogErrorRc(VERR_ENV_VAR_NOT_FOUND,
+                                      "Starting VMs failed. VBOXAUTOSTART_CONFIG environment variable is not defined.\n");
     bool fAllow = false;
 
@@ -819 +818 @@
     rc = autostartParseConfig(pszConfigFile, &pCfgAst);
     if (RT_FAILURE(rc))
-        return autostartSvcLogError("Starting VMs failed. Failed to parse the config file. Check the access permissions and file structure.\n");
-
+        return autostartSvcLogErrorRc(rc, "Starting VMs failed. Failed to parse the config file. Check the access permissions and file structure.\n");
     PCFGAST pCfgAstPolicy = autostartConfigAstGetByName(pCfgAst, "default_policy");
     /* Check default policy. */
@@ -835 +833 @@
         {
             autostartConfigAstDestroy(pCfgAst);
-            return autostartSvcLogError("'default_policy' must be either 'allow' or 'deny'.\n");
+            return autostartSvcLogErrorRc(VERR_INVALID_PARAMETER, "'default_policy' must be either 'allow' or 'deny'.\n");
         }
     }
@@ -844 +842 @@
     {
         autostartConfigAstDestroy(pCfgAst);
-        return autostartSvcLogError("Failed to query username of the process (%Rrc).\n", rc);
+        return autostartSvcLogErrorRc(rc, "Failed to query username of the process (%Rrc).\n", rc);
     }
 
@@ -877 +875 @@
             {
                 autostartConfigAstDestroy(pCfgAst);
-                return autostartSvcLogError("'allow' must be either 'true' or 'false'.\n");
+                return autostartSvcLogErrorRc(VERR_INVALID_PARAMETER, "'allow' must be either 'true' or 'false'.\n");
             }
         }
@@ -884 +882 @@
     {
         autostartConfigAstDestroy(pCfgAst);
-        return autostartSvcLogError("Invalid config, user is not a compound node.\n");
+        return autostartSvcLogErrorRc(VERR_INVALID_PARAMETER, "Invalid config, user is not a compound node.\n");
     }
 
@@ -890 +888 @@
     {
         autostartConfigAstDestroy(pCfgAst);
-        return autostartSvcLogError("User is not allowed to autostart VMs.\n");
-    }
-
-    RTEXITCODE rcExit = autostartStartMain(pCfgAstUser);
+        return autostartSvcLogErrorRc(VERR_INVALID_PARAMETER, "User is not allowed to autostart VMs.\n");
+    }
+
+    if (RT_SUCCESS(rc))
+        rc = autostartStartMain(pCfgAstUser);
+
     autostartConfigAstDestroy(pCfgAst);
-    if (rcExit != RTEXITCODE_SUCCESS)
-        autostartSvcLogError("Starting VMs failed\n");
-
-    return rcExit;
+
+    return rc;
 }
 
@@ -953 +951 @@
                     {
                         /* No one signaled us to stop */
-                        RTEXITCODE ec = autostartStartVMs();
-                        if (ec == RTEXITCODE_SUCCESS)
-                        {
-                            LogFlow(("autostartSvcWinServiceMain: done starting VMs\n"));
-                            dwErr = NO_ERROR;
-                        }
-                        /* No reason to keep started. Shutdown the service*/
+                        rc = autostartStartVMs();
                     }
                     autostartShutdown();
@@ -982 +974 @@
         autostartSvcWinSetServiceStatus(SERVICE_STOPPED, 0, dwErr);
     }
-    else
-    {
-        dwErr = GetLastError();
-        autostartSvcLogError("RegisterServiceCtrlHandlerEx failed, rc=%Rrc (%#x)\n", RTErrConvertFromWin32(dwErr), dwErr);
-    }
-
-    LogFlowFuncLeave();
+    /* else error will be handled by the caller. */
 }
 
@@ -1001 +987 @@
 static RTEXITCODE autostartSvcWinRunIt(int argc, char **argv)
 {
-    int rc;
+    int vrc;
 
     LogFlowFuncEnter();
@@ -1031 +1017 @@
     {
         char szLogFile[RTPATH_MAX];
-        rc = com::GetVBoxUserHomeDirectory(szLogFile, sizeof(szLogFile),
+        vrc = com::GetVBoxUserHomeDirectory(szLogFile, sizeof(szLogFile),
                                            /* :fCreateDir */ false);
-        if (RT_FAILURE(rc))
-        {
-            autostartSvcLogError("Failed to get VirtualBox user home directory: %Rrc\n", rc);
+        if (RT_FAILURE(vrc))
+        {
+            autostartSvcLogError("Failed to get VirtualBox user home directory: %Rrc\n", vrc);
             break;
         }
@@ -1045 +1031 @@
         }
 
-        rc = RTPathAppend(szLogFile, sizeof(szLogFile), "VBoxAutostart.log");
-        if (RT_FAILURE(rc))
-        {
-            autostartSvcLogError("Failed to construct release log file name: %Rrc\n", rc);
-            break;
-        }
-
-        rc = com::VBoxLogRelCreate(AUTOSTART_SERVICE_NAME,
+        vrc = RTPathAppend(szLogFile, sizeof(szLogFile), "VBoxAutostart.log");
+        if (RT_FAILURE(vrc))
+        {
+            autostartSvcLogError( "Failed to construct release log file name: %Rrc\n", vrc);
+            break;
+        }
+
+        vrc = com::VBoxLogRelCreate(AUTOSTART_SERVICE_NAME,
                                    szLogFile,
                                      RTLOGFLAGS_PREFIX_THREAD
@@ -1064 +1050 @@
                                    g_uHistoryFileSize,
                                    NULL);
-        if (RT_FAILURE(rc))
-            autostartSvcLogError("Failed to create release log file: %Rrc\n", rc);
+        if (RT_FAILURE(vrc))
+            autostartSvcLogError("Failed to create release log file: %Rrc\n", vrc);
     } while (0);
 
@@ -1112 +1098 @@
     {
         autostartSvcLogError("runit failed, service name is missing");
-        return RTEXITCODE_FAILURE;
-    }
-
+        return RTEXITCODE_SYNTAX;
+    }
 
     autostartSvcLogInfo("Starting service %ls\n", g_bstrServiceName.raw());
@@ -1127 +1112 @@
         { NULL, NULL}
     };
+
     if (StartServiceCtrlDispatcherW(&s_aServiceStartTable[0]))
     {
@@ -1137 +1123 @@
     {
         case ERROR_FAILED_SERVICE_CONTROLLER_CONNECT:
-            autostartSvcWinServiceMain(0, NULL);//autostartSvcDisplayError("Cannot run a service from the command line. Use the 'start' action to start it the right way.\n");
+            autostartSvcLogWarning("Cannot run a service from the command line. Use the 'start' action to start it the right way.\n");
+            autostartSvcWinServiceMain(0 /* cArgs */, NULL /* papwszArgs */);
             break;
         default:
@@ -1147 +1134 @@
 
     return RTEXITCODE_FAILURE;
-}
-
-
-static void autostartSvcShowHeader(void)
-{
-    RTPrintf(VBOX_PRODUCT " VirtualBox Autostart Service Version " VBOX_VERSION_STRING " - r%s\n"
-             "(C) " VBOX_C_YEAR " " VBOX_VENDOR "\n"
-             "All rights reserved.\n\n", RTBldCfgRevisionStr());
 }
 
@@ -1187 +1166 @@
      * Do the printing.
      */
-    if (fBrief)
-        RTPrintf("%s\n", VBOX_VERSION_STRING);
-    else
-        autostartSvcShowHeader();
+    autostartSvcShowVersion(fBrief);
+
     return RTEXITCODE_SUCCESS;
 }
@@ -1211 +1188 @@
              "\n"
              "Global options:\n"
-             "  <help|-?|-h|--help> [...]\n"
-             "    Displays this help screen.\n"
-             "  <version|-v|--version> [-brief]\n"
-             "    Displays the version.\n"
-             "\n"
+             "  -v\n"
+             "    Increases the verbosity. Can be specified multiple times."
+             "\n\n"
              "No command given:\n"
              "  Runs the service.\n"
+             "Options:\n"
              "  --service <name>\n"
              "    Specifies the service name to run.\n"
+             "\n"
+             "Command </help|help|-?|-h|--help> [...]\n"
+             "    Displays this help screen.\n"
+             "\n"
+             "Command </version|version|-V|--version> [-brief]\n"
+             "    Displays the version.\n"
              "\n"
              "Command </i|install|/RegServer> --user <username> --password-file <...>\n"
@@ -1225 +1207 @@
              "Options:\n"
              "  --user <username>\n"
-             "    Specifies the user name the service should use for installation.\n"
+             "    Specifies the user name the service should be installed for.\n"
              "  --password-file <path/to/file>\n"
              "    Specifies the file for user password to use for installation.\n"
              "\n"
              "Command </u|uninstall|delete|/UnregServer>\n"
-             "  Uninstalls the service.\n",
+             "  Uninstalls the service.\n"
+             "  --user <username>\n"
+             "    Specifies the user name the service should will be deleted for.\n",
              pszExe);
     return RTEXITCODE_SUCCESS;
@@ -1324 +1308 @@
             return autostartSvcWinShowHelp();
         else if (   !stricmp(argv[iArg], "version")
-                 || !stricmp(argv[iArg], "/v")
-                 || !stricmp(argv[iArg], "-v")
+                 || !stricmp(argv[iArg], "/ver")
+                 || !stricmp(argv[iArg], "-V") /* Note: "-v" is used for specifying the verbosity. */
                  || !stricmp(argv[iArg], "/version")
                  || !stricmp(argv[iArg], "-version")

trunk/src/VBox/Frontends/VBoxAutostart/VBoxAutostart.h

@@ -126 +126 @@
 
 /**
+ * Prints the service header header (product name, version, ++) to stdout.
+ */
+DECLHIDDEN(void) autostartSvcShowHeader(void);
+
+/**
+ * Prints the service version information header to stdout.
+ *
+ * @param   fBrief            Whether to show brief information or not.
+ */
+DECLHIDDEN(void) autostartSvcShowVersion(bool fBrief);
+
+/**
  * Log messages to the system and release log.
  *
@@ -181 +193 @@
  * Main routine for the autostart daemon.
  *
- * @returns exit status code.
+ * @returns VBox status code.
  * @param   pCfgAst        Config AST for the startup part of the autostart daemon.
  */
-DECLHIDDEN(RTEXITCODE) autostartStartMain(PCFGAST pCfgAst);
+DECLHIDDEN(int) autostartStartMain(PCFGAST pCfgAst);
 
 /**
@@ -190 +202 @@
  * during system shutdown.
  *
- * @returns exit status code.
+ * @returns VBox status code.
  * @param   pCfgAst        Config AST for the shutdown part of the autostart daemon.
  */
-DECLHIDDEN(RTEXITCODE) autostartStopMain(PCFGAST pCfgAst);
-
-/**
- * Logs a verbose message to the appropriate system log.
+DECLHIDDEN(int) autostartStopMain(PCFGAST pCfgAst);
+
+/**
+ * Logs a verbose message to the appropriate system log and stdout + release log (if configured).
  *
  * @param   cVerbosity  Verbosity level when logging should happen.
@@ -205 +217 @@
 
 /**
- * Logs a verbose message to the appropriate system log.
+ * Logs a verbose message to the appropriate system log and stdout + release log (if configured).
  *
  * @param   cVerbosity  Verbosity level when logging should happen.
@@ -214 +226 @@
 
 /**
- * Logs a warning message to the appropriate system log.
+ * Logs a warning message to the appropriate system log and stdout + release log (if configured).
  *
  * @param   pszFormat   The log string. No trailing newline.
@@ -222 +234 @@
 
 /**
- * Logs a warning message to the appropriate system log.
+ * Logs a warning message to the appropriate system log and stdout + release log (if configured).
  *
  * @param   pszFormat   The log string. No trailing newline.
@@ -230 +242 @@
 
 /**
- * Logs a info message to the appropriate system log.
+ * Logs a info message to the appropriate system log and stdout + release log (if configured).
  *
  * @param   pszFormat   The log string. No trailing newline.
@@ -238 +250 @@
 
 /**
- * Logs a info message to the appropriate system log.
+ * Logs a info message to the appropriate system log and stdout + release log (if configured).
  *
  * @param   pszFormat   The log string. No trailing newline.
@@ -246 +258 @@
 
 /**
+ * Logs the message to the appropriate system log and stderr + release log (if configured).
+ *
+ * In debug builds this will also put it in the debug log.
+ *
+ * @returns VBox status code.
+ * @param   pszFormat   The log string. No trailing newline.
+ * @param   ...         Format arguments.
+ */
+DECLHIDDEN(int) autostartSvcLogErrorV(const char *pszFormat, va_list va);
+
+/**
+ * Logs the message to the appropriate system log and stderr + release log (if configured).
+ *
+ * In debug builds this will also put it in the debug log.
+ *
+ * @returns VBox status code.
+ * @param   pszFormat   The log string. No trailing newline.
+ * @param   ...         Format arguments.
+ */
+DECLHIDDEN(int) autostartSvcLogError(const char *pszFormat, ...);
+
+/**
  * Logs the message to the appropriate system log.
  *
  * In debug builds this will also put it in the debug log.
  *
- * @param   pszFormat   The log string. No trailing newline.
- * @param   ...         Format arguments.
- *
- * @todo    This should later be replaced by the release logger and callback destination(s).
- */
-DECLHIDDEN(RTEXITCODE) autostartSvcLogErrorV(const char *pszFormat, va_list va);
+ * @returns VBox status code specified by \a rc.
+ * @param   pszFormat   The log string. No trailing newline.
+ * @param   ...         Format arguments.
+ *
+ * @note    Convenience function to return directly with the specified \a rc.
+ */
+DECLHIDDEN(int) autostartSvcLogErrorRcV(int rc, const char *pszFormat, va_list va);
 
 /**
@@ -262 +297 @@
  * In debug builds this will also put it in the debug log.
  *
- * @param   pszFormat   The log string. No trailing newline.
- * @param   ...         Format arguments.
- *
- * @todo    This should later be replaced by the release logger and callback destination(s).
- */
-DECLHIDDEN(RTEXITCODE) autostartSvcLogError(const char *pszFormat, ...);
+ * @returns VBox status code specified by \a rc.
+ * @param   pszFormat   The log string. No trailing newline.
+ * @param   ...         Format arguments.
+ *
+ * @note    Convenience function to return directly with the specified \a rc.
+ */
+DECLHIDDEN(int) autostartSvcLogErrorRc(int rc, const char *pszFormat, ...);
 
 /**
  * Deals with RTGetOpt failure, bitching in the system log.
  *
- * @returns 1
+ * @returns VBox status code specified by \a rc.
  * @param   pszAction       The action name.
  * @param   rc              The RTGetOpt return value.
@@ -280 +316 @@
  * @param   pValue          The value returned by RTGetOpt.
  */
-DECLHIDDEN(RTEXITCODE) autostartSvcLogGetOptError(const char *pszAction, int rc, int argc, char **argv, int iArg, PCRTGETOPTUNION pValue);
+DECLHIDDEN(int) autostartSvcLogGetOptError(const char *pszAction, int rc, int argc, char **argv, int iArg, PCRTGETOPTUNION pValue);
 
 /**
  * Bitch about too many arguments (after RTGetOpt stops) in the system log.
  *
- * @returns 1
+ * @returns VERR_INVALID_PARAMETER
  * @param   pszAction       The action name.
  * @param   argc            The argument count.
@@ -291 +327 @@
  * @param   iArg            The argument index.
  */
-DECLHIDDEN(RTEXITCODE) autostartSvcLogTooManyArgsError(const char *pszAction, int argc, char **argv, int iArg);
+DECLHIDDEN(int) autostartSvcLogTooManyArgsError(const char *pszAction, int argc, char **argv, int iArg);
 
 /**
  * Prints an error message to the screen.
  *
+ * @returns RTEXITCODE
  * @param   pszFormat   The message format string.
  * @param   va          Format arguments.
@@ -304 +341 @@
  * Prints an error message to the screen.
  *
+ * @returns RTEXITCODE
  * @param   pszFormat   The message format string.
  * @param   ...         Format arguments.
@@ -319 +357 @@
 DECLHIDDEN(RTEXITCODE) autostartSvcDisplayGetOptError(const char *pszAction, int rc, PCRTGETOPTUNION pValue);
 
-DECLHIDDEN(int) autostartSetup();
-
-DECLHIDDEN(void) autostartShutdown();
+DECLHIDDEN(int) autostartSetup(void);
+
+DECLHIDDEN(void) autostartShutdown(void);
 
 #endif /* !VBOX_INCLUDED_SRC_VBoxAutostart_VBoxAutostart_h */

trunk/src/VBox/Frontends/VBoxAutostart/VBoxAutostartCfg.cpp

@@ -456 +456 @@
  * Log unexpected token error.
  *
- * @returns nothing.
+ * @returns VBox status code (VERR_INVALID_PARAMETER).
  * @param   pToken          The token which caused the error.
  * @param   pszExpected     String of the token which was expected.
  */
-static void autostartConfigTokenizerMsgUnexpectedToken(PCFGTOKEN pToken, const char *pszExpected)
-{
-    autostartSvcLogError("Unexpected token '%s' at %d:%d.%d, expected '%s'",
-                         autostartConfigTokenToString(pToken),
-                         pToken->iLine, pToken->cchStart,
-                         pToken->cchStart + autostartConfigTokenGetLength(pToken) - 1, pszExpected);
+static int autostartConfigTokenizerMsgUnexpectedToken(PCFGTOKEN pToken, const char *pszExpected)
+{
+    return autostartSvcLogErrorRc(VERR_INVALID_PARAMETER, "Unexpected token '%s' at %d:%d.%d, expected '%s'",
+                                autostartConfigTokenToString(pToken),
+                                pToken->iLine, pToken->cchStart,
+                                pToken->cchStart + autostartConfigTokenGetLength(pToken) - 1, pszExpected);
 }
 
@@ -484 +484 @@
     {
         if (pCfgToken->enmType != enmType)
-        {
-            autostartConfigTokenizerMsgUnexpectedToken(pCfgToken, autostartConfigTokenTypeToStr(enmType));
-            rc = VERR_INVALID_PARAMETER;
-        }
+            return autostartConfigTokenizerMsgUnexpectedToken(pCfgToken, autostartConfigTokenTypeToStr(enmType));
 
         autostartConfigTokenFree(pCfgTokenizer, pCfgToken);
@@ -573 +570 @@
     }
     else
-    {
-        autostartConfigTokenizerMsgUnexpectedToken(pToken, "non reserved token");
-        rc = VERR_INVALID_PARAMETER;
-    }
+        rc = autostartConfigTokenizerMsgUnexpectedToken(pToken, "non reserved token");
 
     return rc;
@@ -641 +635 @@
         }
         else if (RT_SUCCESS(rc))
-        {
-            autostartConfigTokenizerMsgUnexpectedToken(pToken, "non reserved token");
-            rc = VERR_INVALID_PARAMETER;
-        }
+            rc = autostartConfigTokenizerMsgUnexpectedToken(pToken, "non reserved token");
 
         /* Add to the current compound node. */
@@ -724 +715 @@
         }
         case CFGASTNODETYPE_LIST:
+            RT_FALL_THROUGH();
         default:
             AssertMsgFailed(("Invalid AST node type %d\n", pCfgAst->enmType));

trunk/src/VBox/Frontends/VBoxAutostart/VBoxAutostartStart.cpp

@@ -54 +54 @@
 }
 
-DECLHIDDEN(RTEXITCODE) autostartStartMain(PCFGAST pCfgAst)
+DECLHIDDEN(int) autostartStartMain(PCFGAST pCfgAst)
 {
-    RTEXITCODE rcExit = RTEXITCODE_SUCCESS;
     int vrc = VINF_SUCCESS;
     std::list<AUTOSTARTVM> listVM;
     uint32_t uStartupDelay = 0;
+
+    autostartSvcLogVerbose(1, "Starting machines ...\n");
 
     pCfgAst = autostartConfigAstGetByName(pCfgAst, "startup_delay");
@@ -68 +69 @@
             vrc = RTStrToUInt32Full(pCfgAst->u.KeyValue.aszValue, 10, &uStartupDelay);
             if (RT_FAILURE(vrc))
-                return RTMsgErrorExit(RTEXITCODE_FAILURE, "'startup_delay' must be an unsigned number");
+                return autostartSvcLogErrorRc(vrc, "'startup_delay' must be an unsigned number");
         }
     }
@@ -79 +80 @@
 
     if (vrc == VERR_INTERRUPTED)
-        return RTEXITCODE_SUCCESS;
+        return VINF_SUCCESS;
 
     /*
@@ -96 +97 @@
             if (machines[i])
             {
+                Bstr strName;
+                CHECK_ERROR_BREAK(machines[i], COMGETTER(Name)(strName.asOutParam()));
+
                 BOOL fAccessible;
                 CHECK_ERROR_BREAK(machines[i], COMGETTER(Accessible)(&fAccessible));
                 if (!fAccessible)
+                {
+                    autostartSvcLogVerbose(1, "Machine '%ls' is not accessible, skipping\n", strName.raw());
                     continue;
+                }
+
+                AUTOSTARTVM autostartVM;
 
                 BOOL fAutostart;
@@ -105 +114 @@
                 if (fAutostart)
                 {
-                    AUTOSTARTVM autostartVM;
-
                     CHECK_ERROR_BREAK(machines[i], COMGETTER(Id)(autostartVM.strId.asOutParam()));
                     CHECK_ERROR_BREAK(machines[i], COMGETTER(AutostartDelay)(&autostartVM.uStartupDelay));
@@ -112 +119 @@
                     listVM.push_back(autostartVM);
                 }
+
+                autostartSvcLogVerbose(1, "Machine '%ls': Autostart is %s (startup delay is %RU32 seconds)\n",
+                                       strName.raw(), fAutostart ? "enabled" : "disabled",
+                                       fAutostart ? autostartVM.uStartupDelay : 0);
             }
         }
@@ -120 +131 @@
          * just to add this log, hence a bit of duplicate logic here.
          */
-        if (SUCCEEDED(rc) && listVM.empty())
-            LogRel(("No VMs configured for autostart\n"));
+        if (SUCCEEDED(rc))
+        {
+            if (machines.size() == 0)
+                autostartSvcLogWarning("No virtual machines found.\n"
+                                       "This either could be a configuration problem (access rights), "
+                                       "or there are no VMs configured yet.");
+            else if (listVM.empty())
+                autostartSvcLogWarning("No virtual machines configured for autostart.\n"
+                                       "Please consult the manual about how to enable auto starting VMs.\n");
+        }
+        else
+            autostartSvcLogError("Enumerating virtual machines failed with %Rhrc\n", rc);
 
         if (   SUCCEEDED(rc)
             && !listVM.empty())
         {
-            ULONG uDelayCurr = 0;
+            ULONG uDelayCur = 0;
 
             /* Sort by startup delay and apply base override. */
@@ -137 +158 @@
                 ComPtr<IProgress> progress;
 
-                if ((*it).uStartupDelay > uDelayCurr)
-                {
-                    autostartSvcLogVerbose(1, "Delaying start of the next VMs for %ul seconds ...\n",
-                                           (*it).uStartupDelay - uDelayCurr);
-                    RTThreadSleep(((*it).uStartupDelay - uDelayCurr) * 1000);
-                    uDelayCurr = (*it).uStartupDelay;
-                }
-
-                CHECK_ERROR_BREAK(g_pVirtualBox, FindMachine((*it).strId.raw(),
-                                                             machine.asOutParam()));
+                CHECK_ERROR_BREAK(g_pVirtualBox, FindMachine((*it).strId.raw(), machine.asOutParam()));
+
+                Bstr strName;
+                CHECK_ERROR_BREAK(machine, COMGETTER(Name)(strName.asOutParam()));
+
+                if ((*it).uStartupDelay > uDelayCur)
+                {
+                    autostartSvcLogVerbose(1, "Waiting for %ul seconds before starting machine '%s' ...\n",
+                                           (*it).uStartupDelay - uDelayCur, strName.raw());
+                    RTThreadSleep(((*it).uStartupDelay - uDelayCur) * 1000);
+                    uDelayCur = (*it).uStartupDelay;
+                }
 
                 CHECK_ERROR_BREAK(machine, LaunchVMProcess(g_pSession, Bstr("headless").raw(),
@@ -152 +175 @@
                 if (SUCCEEDED(rc) && !progress.isNull())
                 {
-                    autostartSvcLogVerbose(1, "Waiting for VM '%ls' to power on...\n", (*it).strId.raw());
+                    autostartSvcLogVerbose(1, "Waiting for machine '%ls' to power on ...\n", strName.raw());
                     CHECK_ERROR(progress, WaitForCompletion(-1));
                     if (SUCCEEDED(rc))
@@ -172 +195 @@
                                 }
                                 else
-                                    autostartSvcLogVerbose(1, "VM '%ls' has been successfully started.\n", (*it).strId.raw());
+                                    autostartSvcLogVerbose(1, "Machine '%ls' has been successfully started.\n", strName.raw());
                             }
                         }
@@ -185 +208 @@
     }
 
-    return rcExit;
+    return vrc;
 }
 

trunk/src/VBox/Frontends/VBoxAutostart/VBoxAutostartStop.cpp

@@ -16 +16 @@
  */
 
+#include <iprt/assert.h>
+#include <iprt/log.h>
+#include <iprt/message.h>
+#include <iprt/stream.h>
+#include <iprt/thread.h>
+#include <iprt/time.h>
+
 #include <VBox/com/com.h>
 #include <VBox/com/string.h>
@@ -22 +29 @@
 #include <VBox/com/ErrorInfo.h>
 #include <VBox/com/errorprint.h>
-
-#include <iprt/thread.h>
-#include <iprt/stream.h>
-#include <iprt/log.h>
-#include <iprt/assert.h>
-#include <iprt/message.h>
 
 #include <list>
@@ -103 +104 @@
 }
 
-DECLHIDDEN(RTEXITCODE) autostartStopMain(PCFGAST pCfgAst)
+DECLHIDDEN(int) autostartStopMain(PCFGAST pCfgAst)
 {
     RT_NOREF(pCfgAst);
-    RTEXITCODE rcExit = RTEXITCODE_SUCCESS;
     std::list<AUTOSTOPVM> listVM;
+
+    autostartSvcLogVerbose(1, "Stopping machines ...\n");
 
     /*
@@ -125 +127 @@
             if (machines[i])
             {
+                Bstr strName;
+                CHECK_ERROR_BREAK(machines[i], COMGETTER(Name)(strName.asOutParam()));
+
                 BOOL fAccessible;
                 CHECK_ERROR_BREAK(machines[i], COMGETTER(Accessible)(&fAccessible));
                 if (!fAccessible)
+                {
+                    autostartSvcLogVerbose(1, "Machine '%ls' is not accessible, skipping\n", strName.raw());
                     continue;
+                }
+
+                AUTOSTOPVM autostopVM;
 
                 AutostopType_T enmAutostopType;
@@ -134 +144 @@
                 if (enmAutostopType != AutostopType_Disabled)
                 {
-                    AUTOSTOPVM autostopVM;
-
                     CHECK_ERROR_BREAK(machines[i], COMGETTER(Id)(autostopVM.strId.asOutParam()));
                     autostopVM.enmAutostopType = enmAutostopType;
@@ -141 +149 @@
                     listVM.push_back(autostopVM);
                 }
+
+                autostartSvcLogVerbose(1, "Machine '%ls': Autostop type is %#x\n",
+                                       strName.raw(), autostopVM.enmAutostopType);
             }
         }
@@ -155 +166 @@
                 CHECK_ERROR_BREAK(g_pVirtualBox, FindMachine((*it).strId.raw(),
                                                              machine.asOutParam()));
+
+                Bstr strName;
+                CHECK_ERROR_BREAK(machine, COMGETTER(Name)(strName.asOutParam()));
 
                 CHECK_ERROR_BREAK(machine, COMGETTER(State)(&enmMachineState));
@@ -191 +205 @@
 
                             rc = showProgress(progress);
-                            CHECK_PROGRESS_ERROR(progress, ("Failed to power off machine"));
+                            CHECK_PROGRESS_ERROR(progress, ("Failed to powering off machine '%ls'", strName.raw()));
+                            if (FAILED(rc))
+                                autostartSvcLogError("Powering off machine '%ls' failed with %Rhrc\n", strName.raw(), rc);
                             break;
                         }
@@ -198 +214 @@
                             BOOL fGuestEnteredACPI = false;
                             CHECK_ERROR_BREAK(console, GetGuestEnteredACPIMode(&fGuestEnteredACPI));
-                            if (fGuestEnteredACPI && enmMachineState == MachineState_Running)
+                            if (   fGuestEnteredACPI
+                                && enmMachineState == MachineState_Running)
                             {
                                 CHECK_ERROR_BREAK(console, PowerButton());
 
-                                autostartSvcLogInfo("Waiting for VM \"%ls\" to power off...\n", (*it).strId.raw());
-
-                                do
+                                autostartSvcLogVerbose(1, "Waiting for machine '%ls' to power off...\n", strName.raw());
+
+                                uint64_t     const tsStartMs = RTTimeMilliTS();
+                                RTMSINTERVAL const msTimeout = RT_MS_5MIN; /* Should be enough time, shouldn't it? */
+
+                                while (RTTimeMilliTS() - tsStartMs <= msTimeout)
                                 {
-                                    RTThreadSleep(1000);
                                     CHECK_ERROR_BREAK(machine, COMGETTER(State)(&enmMachineState));
-                                } while (enmMachineState == MachineState_Running);
+                                    if (enmMachineState != MachineState_Running)
+                                        break;
+                                    RTThreadSleep(RT_MS_1SEC);
+                                }
+
+                                if (RTTimeMilliTS() - tsStartMs > msTimeout)
+                                    autostartSvcLogWarning("Machine '%ls' did not power off via ACPI within time\n", strName.raw());
                             }
                             else
                             {
                                 /* Use save state instead and log this to the console. */
-                                autostartSvcLogWarning("The guest of VM \"%ls\" does not support ACPI shutdown or is currently paused, saving state...\n",
-                                                       (*it).strId.raw());
+                                autostartSvcLogWarning("The guest of machine '%ls' does not support ACPI shutdown or is currently paused, saving state...\n",
+                                                       strName.raw());
                                 rc = autostartSaveVMState(console);
                             }
@@ -220 +245 @@
                         }
                         default:
-                            autostartSvcLogWarning("Unknown autostop type for VM \"%ls\"\n", (*it).strId.raw());
+                            autostartSvcLogWarning("Unknown autostop type for machine '%ls', skipping\n", strName.raw());
                     }
                     g_pSession->UnlockMachine();
@@ -228 +253 @@
     }
 
-    return rcExit;
+    return VINF_SUCCESS; /** @todo r=andy Report back the overall status here. */
 }
 

trunk/src/VBox/Frontends/VBoxAutostart/VBoxAutostartUtils.cpp

@@ -25 +25 @@
 
 #include <VBox/err.h>
-
+#include <VBox/version.h>
+
+#include <iprt/buildconfig.h>
 #include <iprt/message.h>
 #include <iprt/thread.h>
@@ -94 +96 @@
 }
 
-DECLHIDDEN(RTEXITCODE) autostartSvcLogErrorV(const char *pszFormat, va_list va)
-{
-    if (*pszFormat)
-    {
-        char *pszMsg = NULL;
-        if (RTStrAPrintfV(&pszMsg, pszFormat, va) != -1)
-        {
-            autostartSvcOsLogStr(pszMsg, AUTOSTARTLOGTYPE_ERROR);
-            RTStrFree(pszMsg);
-        }
-        else
-            autostartSvcOsLogStr(pszFormat, AUTOSTARTLOGTYPE_ERROR);
-    }
-    return RTEXITCODE_FAILURE;
-}
-
-DECLHIDDEN(RTEXITCODE) autostartSvcLogError(const char *pszFormat, ...)
-{
-    va_list va;
-    va_start(va, pszFormat);
-    autostartSvcLogErrorV(pszFormat, va);
-    va_end(va);
-    return RTEXITCODE_FAILURE;
+DECLHIDDEN(void) autostartSvcShowHeader(void)
+{
+    RTPrintf(VBOX_PRODUCT " VirtualBox Autostart Service Version " VBOX_VERSION_STRING " - r%s\n"
+             "(C) " VBOX_C_YEAR " " VBOX_VENDOR "\n"
+             "All rights reserved.\n\n", RTBldCfgRevisionStr());
+}
+
+DECLHIDDEN(void) autostartSvcShowVersion(bool fBrief)
+{
+    if (fBrief)
+        RTPrintf("%s\n", VBOX_VERSION_STRING);
+    else
+        autostartSvcShowHeader();
+}
+
+DECLHIDDEN(int) autostartSvcLogErrorV(const char *pszFormat, va_list va)
+{
+    AssertPtrReturn(pszFormat, VERR_INVALID_POINTER);
+
+    char *pszMsg = NULL;
+    if (RTStrAPrintfV(&pszMsg, pszFormat, va) != -1)
+    {
+        autostartSvcOsLogStr(pszMsg, AUTOSTARTLOGTYPE_ERROR);
+        RTStrFree(pszMsg);
+        return VINF_SUCCESS;
+    }
+
+    return VERR_BUFFER_OVERFLOW;
+}
+
+DECLHIDDEN(int) autostartSvcLogError(const char *pszFormat, ...)
+{
+    AssertPtrReturn(pszFormat, VERR_INVALID_POINTER);
+
+    va_list va;
+    va_start(va, pszFormat);
+    int rc = autostartSvcLogErrorV(pszFormat, va);
+    va_end(va);
+
+    return rc;
+}
+
+DECLHIDDEN(int) autostartSvcLogErrorRcV(int rc, const char *pszFormat, va_list va)
+{
+    AssertPtrReturn(pszFormat, VERR_INVALID_POINTER);
+
+    int rc2 = autostartSvcLogErrorV(pszFormat, va);
+    if (RT_SUCCESS(rc2))
+        return rc; /* Return handed-in rc. */
+    return rc2;
+}
+
+DECLHIDDEN(int) autostartSvcLogErrorRc(int rc, const char *pszFormat, ...)
+{
+    AssertPtrReturn(pszFormat, VERR_INVALID_POINTER);
+
+    va_list va;
+    va_start(va, pszFormat);
+    int rc2 = autostartSvcLogErrorRcV(rc, pszFormat, va);
+    va_end(va);
+    return rc2;
 }
 
@@ -129 +169 @@
     if (RTStrAPrintfV(&pszMsg, pszFormat, va) != -1)
     {
-        RTPrintf("%s", pszMsg);
         autostartSvcOsLogStr(pszMsg, AUTOSTARTLOGTYPE_VERBOSE);
         RTStrFree(pszMsg);
@@ -178 +217 @@
     if (RTStrAPrintfV(&pszMsg, pszFormat, va) != -1)
     {
-        RTPrintf("%s", pszMsg);
         autostartSvcOsLogStr(pszMsg, AUTOSTARTLOGTYPE_INFO);
         RTStrFree(pszMsg);
@@ -184 +222 @@
 }
 
-DECLHIDDEN(RTEXITCODE) autostartSvcLogGetOptError(const char *pszAction, int rc, int argc, char **argv, int iArg, PCRTGETOPTUNION pValue)
-{
-    NOREF(pValue);
-    autostartSvcLogError("%s - RTGetOpt failure, %Rrc (%d): %s",
-                   pszAction, rc, rc, iArg < argc ? argv[iArg] : "<null>");
-    return RTEXITCODE_FAILURE;
-}
-
-DECLHIDDEN(RTEXITCODE) autostartSvcLogTooManyArgsError(const char *pszAction, int argc, char **argv, int iArg)
-{
-    Assert(iArg < argc);
+DECLHIDDEN(int) autostartSvcLogGetOptError(const char *pszAction, int rc, int argc, char **argv, int iArg, PCRTGETOPTUNION pValue)
+{
+    RT_NOREF(pValue);
+    autostartSvcLogError("%s - RTGetOpt failure, %Rrc (%d): %s", pszAction, rc, rc, iArg < argc ? argv[iArg] : "<null>");
+    return RTEXITCODE_SYNTAX;
+}
+
+DECLHIDDEN(int) autostartSvcLogTooManyArgsError(const char *pszAction, int argc, char **argv, int iArg)
+{
+    AssertReturn(iArg < argc, RTEXITCODE_FAILURE);
     autostartSvcLogError("%s - Too many arguments: %s", pszAction, argv[iArg]);
     for ( ; iArg < argc; iArg++)
         LogRel(("arg#%i: %s\n", iArg, argv[iArg]));
-    return RTEXITCODE_FAILURE;
+    return VERR_INVALID_PARAMETER;
 }