From: VMware, Inc <> Date: Wed, 24 Feb 2010 22:09:13 +0000 (-0800) Subject: Cleanup timesync in preparation for further slewing changes. X-Git-Tag: 2010.02.23-236320~39 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d3a869983af1f8f04df5efb8c7b50d09eab0d2b7;p=thirdparty%2Fopen-vm-tools.git Cleanup timesync in preparation for further slewing changes. These are a couple of changes that I pulled out of my tree that implements NTP style time synchronization. * TimeSyncStepTime now handles everything involved in stepping the time. Before this change I think we would call BDOOR_CMD_STOPCATCHUP in cases where we hadn't stepped the time. * TimeSyncSlewTime handles all of the steps of slewing the time. This will be expanded in future checkins. * Add a comment explaining the landscape at the top of the file. * Keep slewing enabled when the periodic time sync is enabled. If slewing isn't supported leave the slew at nominal. This makes things more uniform and removes the need for TimeSyncDisableWinTimeDaemon. * Cleanup some of the logging a bit (more to follow). Signed-off-by: Marcelo Vanzin --- diff --git a/open-vm-tools/services/plugins/timeSync/timeSync.c b/open-vm-tools/services/plugins/timeSync/timeSync.c index b2249ade0..cd4fea5bc 100644 --- a/open-vm-tools/services/plugins/timeSync/timeSync.c +++ b/open-vm-tools/services/plugins/timeSync/timeSync.c @@ -20,6 +20,69 @@ * @file timeSync.c * * Plugin to handle time synchronization between the guest and host. + * + * There are two types of corrections this plugin makes: one time and periodic. + * + * Periodic time synchronization is done when tools.timeSync is enabled + * (this corresponds with the Synchronize Host and Guest Time checkbox in + * the toolbox). When it is active time is corrected once per period + * (typically every 60 seconds). + * + * One time corrections are done: at tools startup, resuming from suspend, + * after disk shrink and other times when the guest has not been running + * for a while. + * + * There are two basic methods for correcting the time: stepping and slewing. + * + * Stepping the time explictly sets the time in the guest to the time on + * the host. This a brute force approach that isn't very accurate. Any + * delay between deciding what to set the time to and actually setting the + * time introduces error into the new time. Additionally setting the time + * backwards can confuse some applications. During normal operation this + * plugin only steps the time forward and only if the error is greater + * than one second. + * + * Slewing time changes the rate of time advancement allowing errors to be + * corrected smoothly (thus it is possible to correct time in the guest + * being ahead of time on the host without time in the guest ever going + * backwards). An additional advantage is that only a relative change is + * made, so delays in effecting a change don't introduce a large error + * like they might with stepping the time. One thing to note is that + * windows has a notion of slewing being enabled/disabled independant of + * whether the slew is set to nominal, so we track three states: disabled, + * enabled-nominal, and enabled-active. + * + * Interacting with other time sync agents: + * + * When stepping it is relatively easy to co-exist with another time sync + * agent. We will only run into issues when we try to step the time at + * exactly the same time as the other agent. Since we are relatively + * conservative about when to step, this is very unlikely. + * + * When slewing the time we will conflict much more directly with any + * other time sync agent that is trying to slew the time since only one + * slew rate can be active at any given time. To play as nicely as + * possible we only change the slew when necessary: + * + * 1. When starting the timesync loop reset the slew to nominal to clean + * up any odd state left behind a previous time sync agent. For + * example vmware tools could have been running with a slew and then + * crashed. Reseting to nominal gives us a reasonable starting point. + * An additional bonus is that on Windows turning on slewing (even when + * left at nominal) turns off windows' built in time synchronization + * according to MSDN. + * + * 2. When stopping the timesync loop disable slewing. + * + * 3. When we stop slewing (either because we move to a host that doesn't + * support BDOOR_CMD_GETTIMEFULL_WITH_LAG or slew correction was + * disabled), reset the slew rate to nominal. + * + * 4. When stepping the time, reset slewing to nominal if it isn't + * already. + * + * 5. Avoid changing the slew in any other circumstance. This allows a + * another agent to slew the time when we are not actively slewing. */ #include "timeSync.h" @@ -59,6 +122,7 @@ typedef enum TimeSyncState { } TimeSyncState; typedef struct TimeSyncData { + gboolean slewActive; gboolean slewCorrection; uint32 slewPercentCorrection; uint32 timeSyncPeriod; /* In seconds. */ @@ -67,6 +131,8 @@ typedef struct TimeSyncData { } TimeSyncData; +static void TimeSyncSetSlewState(TimeSyncData *data, gboolean active); + /** * Read the time reported by the Host OS. * @@ -259,6 +325,102 @@ TimeSyncReadHostAndGuest(int64 *host, int64 *guest, } +/** + * Set the guest OS time to the host OS time by stepping the time. + * + * @param[in] data Structure tracking time sync state. + * @param[in] adjustment Amount to correct the guest time. + */ + +gboolean +TimeSyncStepTime(TimeSyncData *data, int64 adjustment) +{ + Backdoor_proto bp; + int64 before; + int64 after; + + if (vmx86_debug) { + TimeSync_GetCurrentTime(&before); + } + + /* Stepping invalidates the current slew, reset to nominal. */ + TimeSyncSetSlewState(data, FALSE); + + if (!TimeSync_AddToCurrentTime(adjustment)) { + return FALSE; + } + + /* + * Tell timetracker to stop trying to catch up, since we have corrected + * both the guest OS error and the apparent time error. + */ + bp.in.cx.halfs.low = BDOOR_CMD_STOPCATCHUP; + Backdoor(&bp); + + if (vmx86_debug) { + TimeSync_GetCurrentTime(&after); + + g_debug("Time changed by %"FMT64"dus from %"FMT64"d.%06"FMT64"d -> " + "%"FMT64"d.%06"FMT64"d\n", adjustment, + before / US_PER_SEC, before % US_PER_SEC, + after / US_PER_SEC, after % US_PER_SEC); + } + + return TRUE; +} + + +/** + * Slew the guest OS time advancement to correct the time. Only correct a + * portion of the error to avoid overcorrection. + * + * @param[in] data Structure tracking time sync state. + * @param[in] adjustment Amount to correct the guest time. + */ + +static gboolean +TimeSyncSlewTime(TimeSyncData *data, int64 adjustment) +{ + int64 timeSyncPeriodUS = data->timeSyncPeriod * US_PER_SEC; + int64 slewDiff = (adjustment * data->slewPercentCorrection) / 100; + + return TimeSync_EnableTimeSlew(slewDiff, timeSyncPeriodUS); +} + + +/** + * Reset the slew to nominal. + * + * @param[in] data Structure tracking time sync state. + */ + +static void +TimeSyncResetSlew(TimeSyncData *data) +{ + TimeSyncSlewTime(data, 0); +} + + +/** + * Update whether slewing is used for time correction. + * + * @param[in] data Structure tracking time sync state. + * @param[in] active Is slewing active. + */ + +static void +TimeSyncSetSlewState(TimeSyncData *data, gboolean active) +{ + if (active != data->slewActive) { + g_debug(active ? "Starting slew.\n" : "Stopping slew.\n"); + if (!active) { + TimeSyncResetSlew(data); + } + data->slewActive = active; + } +} + + /** * Set the guest OS time to the host OS time. * @@ -276,18 +438,11 @@ TimeSyncDoSync(Bool slewCorrection, Bool allowBackwardSync, void *_data) { - Backdoor_proto bp; int64 guest, host; int64 gosError, apparentError, maxTimeError; Bool apparentErrorValid; TimeSyncData *data = _data; -#ifdef VMX86_DEBUG - int64 before; - int64 after; - TimeSync_GetCurrentTime(&before); -#endif - g_debug("Synchronizing time: " "syncOnce %d, slewCorrection %d, allowBackwardSync %d.\n", syncOnce, slewCorrection, allowBackwardSync); @@ -300,6 +455,7 @@ TimeSyncDoSync(Bool slewCorrection, gosError = guest - host - apparentError; if (syncOnce) { + /* * Non-loop behavior: * @@ -307,67 +463,41 @@ TimeSyncDoSync(Bool slewCorrection, * 1) The guest OS error is behind by more than maxTimeError. * 2) The guest OS is ahead of the host OS. */ + if (gosError < -maxTimeError || (gosError + apparentError > 0 && allowBackwardSync)) { - TimeSync_DisableTimeSlew(); - if (!TimeSync_AddToCurrentTime(-gosError + -apparentError)) { - g_warning("Unable to set the guest OS time: %s.\n\n", - Msg_ErrString()); + g_debug("One time synchronization: stepping time.\n"); + if (!TimeSyncStepTime(data, -gosError + -apparentError)) { return FALSE; } + } else { + g_debug("One time synchronization: correction not needed.\n"); } } else { /* * Loop behavior: * - * If guest is behind host by more than maxTimeLag + interruptLag - * perform a step correction to the guest clock and ask the monitor - * to drop its accumulated catchup (interruptLag). - * - * Otherwise, perform a slew correction. Adjust the guest's clock - * rate to be either faster or slower than nominal real time, such - * that we expect to correct correctionPercent percent of the error - * during this synchronization cycle. + * If guest error is more than maxTimeError behind perform a step + * correction. Otherwise, if we can distinguish guest error from + * apparent time error perform a slew correction . */ + TimeSyncSetSlewState(data, apparentErrorValid && slewCorrection); + if (gosError < -maxTimeError) { - TimeSync_DisableTimeSlew(); - if (!TimeSync_AddToCurrentTime(-gosError + -apparentError)) { - g_warning("Unable to set the guest OS time: %s.\n\n", Msg_ErrString()); + g_debug("Periodic synchronization: stepping time.\n"); + if (!TimeSyncStepTime(data, -gosError + -apparentError)) { return FALSE; } } else if (slewCorrection && apparentErrorValid) { - int64 timeSyncPeriodUS = data->timeSyncPeriod * US_PER_SEC; - int64 slewDiff = (-gosError * data->slewPercentCorrection) / 100; - - if (!TimeSync_EnableTimeSlew(slewDiff, timeSyncPeriodUS)) { - g_warning("Unable to slew the guest OS time: %s.\n\n", Msg_ErrString()); + g_debug("Periodic synchronization: slewing time.\n"); + if (!TimeSyncSlewTime(data, -gosError)) { return FALSE; } - } else { - TimeSync_DisableTimeSlew(); } } -#ifdef VMX86_DEBUG - TimeSync_GetCurrentTime(&after); - - g_debug("Time changed from %"FMT64"d.%06"FMT64"d -> " - "%"FMT64"d.%06"FMT64"d\n", - before / US_PER_SEC, before % US_PER_SEC, - after / US_PER_SEC, after % US_PER_SEC); -#endif - - /* - * If we have stepped the time, ask TimeTracker to reset to normal the rate - * of timer interrupts it forwards from the host to the guest. - */ - if (!TimeSync_IsTimeSlewEnabled()) { - bp.in.cx.halfs.low = BDOOR_CMD_STOPCATCHUP; - Backdoor(&bp); - } - return TRUE; } @@ -395,78 +525,6 @@ ToolsDaemonTimeSyncLoop(gpointer _data) } -#if defined(_WIN32) -/** - * Try to disable the Windows Time Daemon. - * - * @return TRUE on success. - */ - -static Bool -TimeSyncDisableWinTimeDaemon(void) -{ - DWORD timeAdjustment; - DWORD timeIncrement; - DWORD error; - BOOL timeAdjustmentDisabled; - BOOL success = FALSE; - - /* - * We need the SE_SYSTEMTIME_NAME privilege to make the change; get - * the privilege now (or bail if we can't). - */ - success = System_SetProcessPrivilege(SE_SYSTEMTIME_NAME, TRUE); - if (!success) { - return FALSE; - } - - /* Actually try to stop the time daemon. */ - if (GetSystemTimeAdjustment(&timeAdjustment, &timeIncrement, - &timeAdjustmentDisabled)) { - g_debug("GetSystemTimeAdjustment() succeeded: timeAdjustment %d," - "timeIncrement %d, timeAdjustmentDisabled %s\n", - timeAdjustment, timeIncrement, - timeAdjustmentDisabled ? "TRUE" : "FALSE"); - /* - * timeAdjustmentDisabled means the opposite of what you'd think; - * if it's TRUE, that means the system may be adjusting the time - * on its own using the time daemon. Read MSDN for the details, - * and see Bug 24173 for more discussion on this. - */ - - if (timeAdjustmentDisabled) { - /* - * MSDN is a bit vague on the semantics of this function, but it - * would appear that the timeAdjustment value here is simply the - * total amount that the system will add to the clock on each - * timer tick, i.e. if you set it to zero the system clock will - * not progress at all (and indeed, attempting to set it to zero - * results in an ERROR_INVALID_PARAMETER). In order to have time - * proceed at the normal rate, this needs to be set to the value - * of timeIncrement retrieved from GetSystemTimeAdjustment(). - */ - if (!SetSystemTimeAdjustment(timeIncrement, FALSE)) { - error = GetLastError(); - g_debug("SetSystemTimeAdjustment failed: %d\n", error); - goto exit; - } - } - } else { - error = GetLastError(); - g_debug("GetSystemTimeAdjustment failed: %d\n", error); - goto exit; - } - - success = TRUE; - - exit: - g_debug("Stopping time daemon %s.\n", success ? "succeeded" : "failed"); - System_SetProcessPrivilege(SE_SYSTEMTIME_NAME, FALSE); - return success; -} -#endif - - /** * Start the "time synchronization" loop. * @@ -486,12 +544,10 @@ TimeSyncStartLoop(ToolsAppCtx *ctx, g_debug("Starting time sync loop.\n"); -#if defined(_WIN32) - g_debug("Daemon: Attempting to disable Windows Time daemon\n"); - if (!TimeSyncDisableWinTimeDaemon()) { - g_warning("Daemon: Failed to disable Windows Time daemon\n"); - } -#endif + /* + * Turn slew on and set it to nominal. + */ + TimeSyncResetSlew(data); g_debug("New sync period is %d sec.\n", data->timeSyncPeriod); @@ -524,6 +580,7 @@ TimeSyncStopLoop(ToolsAppCtx *ctx, g_debug("Stopping time sync loop.\n"); + TimeSyncSetSlewState(data, FALSE); TimeSync_DisableTimeSlew(); g_source_destroy(data->timer); @@ -682,7 +739,7 @@ TimeSyncSetOption(gpointer src, } if (doSync && !doneAlready && - !TimeSyncDoSync(data->slewCorrection, TRUE, TRUE, data)) { + !TimeSyncDoSync(data->slewCorrection, TRUE, TRUE, data)) { g_warning("Unable to sync time during startup.\n"); return FALSE; } @@ -756,6 +813,7 @@ ToolsOnLoad(ToolsAppCtx *ctx) { TOOLS_APP_SIGNALS, VMTools_WrapArray(sigs, sizeof *sigs, ARRAYSIZE(sigs)) } }; + data->slewActive = FALSE; data->slewCorrection = FALSE; data->slewPercentCorrection = TIMESYNC_PERCENT_CORRECTION; data->state = TIMESYNC_INITIALIZING;