Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME

[Liran Alon <liran.alon@oracle.com>]

On 22/03/2020 13:22, Liran Alon wrote:
>
> On 15/03/2020 13:56, Liran Alon wrote:
>>
>> On 14/03/2020 22:56, Michael S. Tsirkin wrote:
>>> On Sat, Mar 14, 2020 at 10:05:20PM +0200, Liran Alon wrote:
>>>> Michael, you can also refer to this VMware time-keeping whitepaper:
>>>> https://urldefense.com/v3/__https://www.vmware.com/pdf/vmware_timekeeping.pdf__;!!GqivPVa7Brio!K8sfnfvVgKwrQ4SMwX-K6-S5yR4ln9_qZ6o4GzIpQkohfWtinlplNhXzFlyUgks$ 
>>>> .
>>>> According to section "Initializing and Correcting Wall-Clock Time":
>>>> """
>>>> VMware Tools can also optionally be used to correct long‐term drift 
>>>> and
>>>> errors by periodically
>>>> resynchronizing the virtual machine’s clock to the host’s clock, 
>>>> but the
>>>> current version at this writing is limited.
>>>> In particular, in guest operating systems other than NetWare, it 
>>>> does not
>>>> correct errors in which the guest clock
>>>> is ahead of real time, only those in which the guest clock is behind.
>>>>
>>>> """
>>> This talks about guest time.
>>> What this does not mention is whether hosts need to employ any 
>>> mechanisms
>>> to synchronise wall clock between hosts.
>> The above mentioned whitepaper also discuss how VMware maintains the 
>> wallclock time across migrations (vMotion).
>> See section "Using VMware Tools Clock Synchronization" in whitepaper.
>>
>> Specifically, there is an option in .vmx file named 
>> "time.synchronize.resume.disk" which:
>> """
>> If set to TRUE, the clock syncs after resuming from suspend and after 
>> migrating to a new host using the VMware VMotion feature.
>> """
>>
>> The matching functionality in open-vm-tools can can be seen in 
>> services/plugins/timeSync/timeSync.c where ToolsOnLoad()
>> registers the "Time_Synchronize" RpcCallback, which is 
>> TimeSyncTcloHandler(), that is possibly allowed to sync time 
>> backwards (Note the "backwardSync" var).
>>
>> The current patch-series I have submitted doesn't implement this 
>> RpcCallback functionality.
>> That work can be delayed to a future patch-series that will add this 
>> extra functionality as-well.
>>
>>>> If I understand correctly, this seems to validate my assumption 
>>>> that current
>>>> implementation for CMD_GETTIME is sufficient.
>>> So I am concerned this does not interact well with other time sources
>>> in QEMU. For example, it's very useful to set guest time with -rtc base
>>> flag.
>>>
>>> Can you use qemu_get_timedate?
>>>
>> This is a very good point.
>> VMware also have the ability that allows user to explicitly set guest 
>> time with .vmx "rtc.startTime" option.
>> (The time-zone can also be set by specifying an offset from UTC with 
>> "rtc.diffFromUTC" option)
>>
>> However, if you will read section "Using VMware Tools Clock 
>> Synchronization -> Disabling All Synchronization" in above mentioned 
>> whitepaper,
>> you will notice that in VMware's design, VMPort CMD_GETTIME command 
>> is intentionally not synced with virtual CMOS TOD. i.e. The section 
>> explicitly
>> documents that if a user wants to set guest time to fictitious time, 
>> user must disable VMware Tools time sync functionality by manipulating
>> "tools.syncTime" and "time.synchronize.*" configuration options as 
>> desired.
>>
>> Therefore, I think current patch VMPort CMD_GETTIME command 
>> implementation is correct.
>> What do you think?
>>
>> -Liran
>>
> Gentle ping.
>
> I would like to send the next version of the patch-series.
> But before I do, I would like to know what we have agreed upon 
> regarding this patch.
>
> Thanks,
> -Liran
>
Another gentle ping before I send v4 of patch-series.

Thanks,
-Liran