Re: [PATCH qemu-server v5 3/3] fix #5032: agent: sync guest time via QGA when the clock falls behind

[Fiona Ebner <f.ebner@proxmox.com>]

Am 05.11.26 um 9:16 PM schrieb Thomas Lamprecht:
> Am 13.08.26 um 15:57 schrieb Jakob Klocker:
>> Add a 'sync-time-on-resume' agent config property (default off) that,
>> when enabled, issues the guest-set-time QGA command to set the guest
>> clock to the host's current time after operations that leave the clock
>> behind:
>> - resuming from hibernation (suspend-to-disk)
>> - resuming from pause/suspend
>> - rolling back to a snapshot that includes RAM
>> - live migration
>> - taking a snapshot, during which the guest is briefly frozen
>>
>> The start, resume and rollback API endpoints gain a
>> 'sync-time-on-resume' parameter (e.g. 'qm start <vmid>
>> --sync-time-on-resume 0') that overrides the configured agent value for
>> a single operation, forcing the sync on or off regardless of config.
>>
>> guest_set_time prints a confirmation line to stdout on success. During
>> live migration the resume step runs over the migration tunnel, which
>> also uses stdout, so an unconditional print corrupts the tunnel
>> protocol. vm_resume therefore takes a 'quiet' parameter to suppress the
>> print on the migration path; the confirmation is logged by the migration
>> code instead.
>>
>> A version check was added for migration to display a clear error
>> message in case 'sync-time-on-resume' is enabled and the target doesn't
>> support the 'sync-time-on-resume' the option.
>>
>> Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
>> ---
>>  src/PVE/API2/Qemu.pm           | 75 ++++++++++++++++++++++++++++++++--
>>  src/PVE/CLI/qm.pm              |  2 +-
>>  src/PVE/QemuConfig.pm          | 12 +++++-
>>  src/PVE/QemuMigrate.pm         | 13 ++++++
>>  src/PVE/QemuServer.pm          | 18 +++++++-
>>  src/PVE/QemuServer/Agent.pm    | 61 +++++++++++++++++++++++++++
>>  src/PVE/QemuServer/RunState.pm | 11 ++++-
>>  7 files changed, 183 insertions(+), 9 deletions(-)
> 
>> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
>> index 8da6f15d..ae40195c 100644
>> --- a/src/PVE/QemuMigrate.pm
>> +++ b/src/PVE/QemuMigrate.pm
>> @@ -312,6 +312,16 @@ sub prepare {
>>          }
>>      }
>>  
>> +    my $target_version = PVE::QemuServer::Helpers::get_node_pvecfg_version($self->{node});
> 
> not sure if this is all worth it, we normally require all nodes to be the same level
> and existing VMs do not automatically gain this on upgrade, one has to explicitly
> configure it first. We could also avoid it for now for migration (and probably taking
> a snapshot) those are milliseconds at max anyway (else it would not really be live
> anymore). Could be enabled later for that and then potentially backup snapshots too?
> There's also a brief pause there I think?

Also, checking this will be covered together with all other newly
introduced config settings once my strict config check series is in:

https://lore.proxmox.com/pve-devel/20261103124831.139399-1-f.ebner@proxmox.com/T/

> 
> But if we indeed want to add this check then it needs two changes...
> 
>> +    if (
>> +        $target_version
>> +        && !PVE::QemuServer::Helpers::pvecfg_min_version($target_version, 9, 2, 5)
> 
> ... this new call to PVE::QemuServer::Helpers::pvecfg_min_version(...) could be
> swapped with the new location of that, i.e. PVE::Tools::pvecfg_min_version(...)
> and the version updated (or at least a prominent NOTE in the diffstat section of
> this patch added as reminder to update it on applying.
> 
>> +        && PVE::QemuServer::Agent::get_qga_key($conf, 'sync-time-on-resume')
> 
> This misses one explicitly setting the property to 0, in which case the other
> node still needs to support it. Would need a definedness check here.
> 
>> +    ) {
>> +        die "target node '$self->{node}' is too old to support 'sync-time-on-resume'"
>> +            . " (needs pve-manager >= 9.2.5); disable the option before migrating\n";
>> +    }
>> +
>>      my $vollist = PVE::QemuServer::get_vm_volumes($conf);
>>  
>>      my $storages = {};