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

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

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