[PATCH guest-common/manager/qemu-server v3 0/3] fix #5032: add guest time sync via QGA

[PATCH guest-common/manager/qemu-server v3 0/3] fix #5032: add guest time sync via QGA

[Jakob Klocker]

This series adds a new agent option 'sync-time-on-resume' to
automatically synchronize the guest clock via the QEMU Guest Agent 
after operations that can leave the guest time stale. The option is 
disabled by default and only takes effect when the QEMU Guest Agent is
enabled.

When a VM resumes with a restored RAM state (waking from hibernation or
rolling back to a snapshot that includes RAM), the guest clock 
continues from the point the state was saved and no longer matches 
wall-clock time. Skews also appears whenever the guest is briefly 
frozen and resumed - after creating a snapshot, an
ordinary pause/resume, or the resume step of a live migration. 

When enabled, the option triggers a guest-set-time call:
 * after resuming from hibernation (suspend-to-disk)
 * after rolling back to a snapshot that includes RAM
 * after a live migration
 * after an ordinary pause/resume
 * after taking a snapshot

The start, resume and rollback API endpoints also accept a
'sync-time-on-resume' parameter that overrides the configured value for
a single operation. 

The persistent config option is exposed in the QEMU Guest Agent
configuration GUI, so it can be toggled per guest.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5032

changes from v1 to v2 (thanks @Arthur)
- qemu: adapt warning message in the resume-from-saved-state path
- ui: reword the agent option label for clarity

changes from v2 to v3 (thanks @Fiona)
- add sync on pause/resume
- add override options
- add print task 
- adapt formating and naming

pve-guest-common:

Jakob Klocker (1):
  AbstractConfig: allow passing options to snapshot_rollback

 src/PVE/AbstractConfig.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


pve-manager:

Jakob Klocker (1):
  fix #5032: ui: qemu agent: add sync-time-on-resume option

 www/manager6/form/AgentFeatureSelector.js | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)


qemu-server:

Jakob Klocker (1):
  fix #5032: agent: sync guest time via QGA when the clock falls behind

 src/PVE/API2/Qemu.pm           | 46 ++++++++++++++++++++++++++---
 src/PVE/CLI/qm.pm              |  2 +-
 src/PVE/QemuConfig.pm          | 12 ++++++--
 src/PVE/QemuMigrate.pm         |  2 ++
 src/PVE/QemuServer.pm          | 18 +++++++++++-
 src/PVE/QemuServer/Agent.pm    | 54 ++++++++++++++++++++++++++++++++++
 src/PVE/QemuServer/RunState.pm | 11 ++++++-
 7 files changed, 136 insertions(+), 9 deletions(-)


Summary over all repositories:
  9 files changed, 156 insertions(+), 12 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH pve-guest-common v3 1/3] AbstractConfig: allow passing options to snapshot_rollback

[Jakob Klocker]

Thread an optional $opts hashref through snapshot_rollback to
__snapshot_rollback_vm_start, so callers can pass per-rollback options
down to the VM start. Used by qemu-server to let the API override the
guest time sync for a single rollback; other options can be added
without further signature changes.

Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
 src/PVE/AbstractConfig.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index 7bcae19..76ea97a 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -1125,7 +1125,8 @@ my $rollback_remove_replication_snapshots = sub {
 
 # Rolls back to a given snapshot.
 sub snapshot_rollback {
-    my ($class, $vmid, $snapname) = @_;
+    my ($class, $vmid, $snapname, $opts) = @_;
+    $opts //= {};
 
     my $prepare = 1;
 
@@ -1197,7 +1198,7 @@ sub snapshot_rollback {
         $class->write_config($vmid, $conf);
 
         if (!$prepare && $snap->{vmstate}) {
-            $class->__snapshot_rollback_vm_start($vmid, $snap->{vmstate}, $data);
+            $class->__snapshot_rollback_vm_start($vmid, $snap->{vmstate}, $data, $opts);
         }
     };
 
-- 
2.47.3

[PATCH pve-manager v3 2/3] fix #5032: ui: qemu agent: add sync-time-on-resume option

[Jakob Klocker]

Expose the new agent configuration option `sync-time-on-resume`
in the VM configuration GUI.

This allows enabling/disabling automatic guest clock
synchronization after clock-stalling operations.

Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
changes since v1:
- reword the agent option label for clarity
changes since v2:
- set default value to disabled

 www/manager6/form/AgentFeatureSelector.js | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 3cae4194..10bb3eeb 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -45,6 +45,20 @@ Ext.define('PVE.form.AgentFeatureSelector', {
                 hidden: '{freeze_fs.checked}',
             },
         },
+        {
+            xtype: 'proxmoxcheckbox',
+            boxLabel: gettext(
+                "Synchronize guest clock with the host's after resume from saved state",
+            ),
+            name: 'sync-time-on-resume',
+            reference: 'sync_time_on_resume',
+            bind: {
+                disabled: '{!enabled.checked}',
+            },
+            disabled: true,
+            uncheckedValue: '0',
+            defaultValue: '0',
+        },
         {
             xtype: 'displayfield',
             userCls: 'pmx-hint',
@@ -74,7 +88,9 @@ Ext.define('PVE.form.AgentFeatureSelector', {
         if (PVE.Parser.parseBoolean(values['freeze-fs'])) {
             delete values['freeze-fs'];
         }
-
+        if (!PVE.Parser.parseBoolean(values['sync-time-on-resume'])) {
+            delete values['sync-time-on-resume'];
+        }
         const agentstr = PVE.Parser.printPropertyString(values, 'enabled');
         return { agent: agentstr };
     },
-- 
2.47.3

Re: [PATCH pve-manager v3 2/3] fix #5032: ui: qemu agent: add sync-time-on-resume option

[Maximiliano Sandoval]

Jakob Klocker <j.klocker@proxmox.com> writes:

> Expose the new agent configuration option `sync-time-on-resume`
> in the VM configuration GUI.
>
> This allows enabling/disabling automatic guest clock
> synchronization after clock-stalling operations.
>
> Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
> ---
> changes since v1:
> - reword the agent option label for clarity
> changes since v2:
> - set default value to disabled
>
>  www/manager6/form/AgentFeatureSelector.js | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
> index 3cae4194..10bb3eeb 100644
> --- a/www/manager6/form/AgentFeatureSelector.js
> +++ b/www/manager6/form/AgentFeatureSelector.js
> @@ -45,6 +45,20 @@ Ext.define('PVE.form.AgentFeatureSelector', {
>                  hidden: '{freeze_fs.checked}',
>              },
>          },
> +        {
> +            xtype: 'proxmoxcheckbox',
> +            boxLabel: gettext(
> +                "Synchronize guest clock with the host's after resume from saved state",

I would say "...after resuming from a previous state".

> +            ),
> +            name: 'sync-time-on-resume',
> +            reference: 'sync_time_on_resume',
> +            bind: {
> +                disabled: '{!enabled.checked}',
> +            },
> +            disabled: true,
> +            uncheckedValue: '0',
> +            defaultValue: '0',
> +        },
>          {
>              xtype: 'displayfield',
>              userCls: 'pmx-hint',
> @@ -74,7 +88,9 @@ Ext.define('PVE.form.AgentFeatureSelector', {
>          if (PVE.Parser.parseBoolean(values['freeze-fs'])) {
>              delete values['freeze-fs'];
>          }
> -
> +        if (!PVE.Parser.parseBoolean(values['sync-time-on-resume'])) {
> +            delete values['sync-time-on-resume'];
> +        }
>          const agentstr = PVE.Parser.printPropertyString(values, 'enabled');
>          return { agent: agentstr };
>      },

-- 
Maximiliano

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

[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.

Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
---
changes since v1:
- adapt warning message in the resume-from-saved-state path
changes since v2:
- add sync on pause/resume
- add override options
- add print task 
- adapt formating and naming

 src/PVE/API2/Qemu.pm           | 46 ++++++++++++++++++++++++++---
 src/PVE/CLI/qm.pm              |  2 +-
 src/PVE/QemuConfig.pm          | 12 ++++++--
 src/PVE/QemuMigrate.pm         |  2 ++
 src/PVE/QemuServer.pm          | 18 +++++++++++-
 src/PVE/QemuServer/Agent.pm    | 54 ++++++++++++++++++++++++++++++++++
 src/PVE/QemuServer/RunState.pm | 11 ++++++-
 7 files changed, 136 insertions(+), 9 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 9023c344..f58151e8 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -3510,6 +3510,14 @@ __PACKAGE__->register_method({
                     . ' the migration. A value of 0 means that the host_mtu parameter is to be'
                     . ' avoided for the corresponding device.',
             },
+            'sync-time-on-resume' => {
+                type => 'boolean',
+                description =>
+                    "Override the VM's configured 'sync-time-on-resume' agent setting for this"
+                    . " start only. Controls whether the guest clock is synchronized after the"
+                    . " RAM state is restored. The guest agent must be enabled.",
+                optional => 1,
+            },
         },
     },
     returns => {
@@ -3525,6 +3533,7 @@ __PACKAGE__->register_method({
         my $vmid = extract_param($param, 'vmid');
         my $timeout = extract_param($param, 'timeout');
         my $machine = extract_param($param, 'machine');
+        my $sync_time = extract_param($param, 'sync-time-on-resume');
 
         my $get_root_param = sub {
             my $value = extract_param($param, $_[0]);
@@ -3631,6 +3640,7 @@ __PACKAGE__->register_method({
                     timeout => $timeout,
                     forcecpu => $force_cpu,
                     'nets-host-mtu' => $nets_host_mtu,
+                    'sync-time-on-resume' => $sync_time,
                 };
 
                 PVE::QemuServer::vm_start($storecfg, $vmid, $params, $migrate_opts);
@@ -4115,6 +4125,14 @@ __PACKAGE__->register_method({
                 description => "Do not check whether the VM is running, used"
                     . " internally during migration. Only root may use this option.",
             },
+            'sync-time-on-resume' => {
+                type => 'boolean',
+                description =>
+                    "Override the VM's configured 'sync-time-on-resume' agent setting for this"
+                    . " resume only. Controls whether the guest clock is synchronized after the"
+                    . " RAM state is restored. The guest agent must be enabled.",
+                optional => 1,
+            },
         },
     },
     returns => {
@@ -4131,6 +4149,8 @@ __PACKAGE__->register_method({
 
         my $vmid = extract_param($param, 'vmid');
 
+        my $sync_time = extract_param($param, 'sync-time-on-resume');
+
         my $skiplock = extract_param($param, 'skiplock');
         raise_param_exc({ skiplock => "Only root may use this option." })
             if $skiplock && $authuser ne 'root@pam';
@@ -4161,10 +4181,14 @@ __PACKAGE__->register_method({
             syslog('info', "resume VM $vmid: $upid\n");
 
             if (!$to_disk_suspended) {
-                PVE::QemuServer::RunState::vm_resume($vmid, $skiplock, $nocheck);
+                PVE::QemuServer::RunState::vm_resume($vmid, $skiplock, $nocheck, $sync_time);
             } else {
                 my $storecfg = PVE::Storage::config();
-                PVE::QemuServer::vm_start($storecfg, $vmid, { skiplock => $skiplock });
+                PVE::QemuServer::vm_start(
+                    $storecfg,
+                    $vmid,
+                    { skiplock => $skiplock, 'sync-time-on-resume' => $sync_time },
+                );
             }
 
             return;
@@ -6323,6 +6347,14 @@ __PACKAGE__->register_method({
                 optional => 1,
                 default => 0,
             },
+            'sync-time-on-resume' => {
+                type => 'boolean',
+                description =>
+                    "Override the VM's configured sync-time-on-resume agent setting for this"
+                    . " rollback only. Controls whether the guest clock is synchronized"
+                    . " after the RAM state is restored. The guest agent must be enabled.",
+                optional => 1,
+            },
         },
     },
     returns => {
@@ -6342,6 +6374,8 @@ __PACKAGE__->register_method({
 
         my $snapname = extract_param($param, 'snapname');
 
+        my $sync_time = extract_param($param, 'sync-time-on-resume');
+
         # vm_start is invoked directly from the worker, so its own permissions
         # predicate doesn't fire here - check VM.PowerMgmt up front so a user
         # with only VM.Snapshot.Rollback can't power the VM on as a side effect.
@@ -6350,7 +6384,11 @@ __PACKAGE__->register_method({
 
         my $realcmd = sub {
             PVE::Cluster::log_msg('info', $authuser, "rollback snapshot VM $vmid: $snapname");
-            PVE::QemuConfig->snapshot_rollback($vmid, $snapname);
+            PVE::QemuConfig->snapshot_rollback(
+                $vmid,
+                $snapname,
+                { 'sync-time-on-resume' => $sync_time },
+            );
 
             if ($param->{start} && !PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
                 PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node });
@@ -6915,7 +6953,7 @@ __PACKAGE__->register_method({
                 },
                 'resume' => sub {
                     if (PVE::QemuServer::Helpers::vm_running_locally($state->{vmid})) {
-                        PVE::QemuServer::RunState::vm_resume($state->{vmid}, 1, 1);
+                        PVE::QemuServer::RunState::vm_resume($state->{vmid}, 1, 1, undef, 1);
                     } else {
                         die "VM $state->{vmid} not running\n";
                     }
diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
index b903c1f1..25351f64 100755
--- a/src/PVE/CLI/qm.pm
+++ b/src/PVE/CLI/qm.pm
@@ -473,7 +473,7 @@ __PACKAGE__->register_method({
                 if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
                     # vm_resume with nocheck, since local node might not have processed config
                     # move/rename yet
-                    eval { PVE::QemuServer::RunState::vm_resume($vmid, 1, 1); };
+                    eval { PVE::QemuServer::RunState::vm_resume($vmid, 1, 1, undef, 1); };
                     if ($@) {
                         $tunnel_write->("ERR: resume failed - $@");
                     } else {
diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
index 26f0fda2..e95a8c61 100644
--- a/src/PVE/QemuConfig.pm
+++ b/src/PVE/QemuConfig.pm
@@ -383,6 +383,11 @@ sub __snapshot_create_vol_snapshots_hook {
                     next;
                 }
             }
+            my $conf = $class->load_config($vmid);
+            if (PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent})) {
+                eval { PVE::QemuServer::Agent::guest_set_time($vmid); };
+                warn "could not sync guest time after snapshot - $@" if $@;
+            }
         }
     }
 }
@@ -444,8 +449,9 @@ sub __snapshot_rollback_hook {
     my ($class, $vmid, $conf, $snap, $prepare, $data) = @_;
 
     if ($prepare) {
-        # we save the machine of the current config
+        # we save the machine and agent of the current config
         $data->{oldmachine} = $conf->{machine};
+        $data->{agent} = $conf->{agent};
     } else {
         # if we have a 'runningmachine' entry in the snapshot we use that
         # for the forcemachine parameter, else we use the old logic
@@ -509,7 +515,7 @@ sub __snapshot_rollback_vm_stop {
 }
 
 sub __snapshot_rollback_vm_start {
-    my ($class, $vmid, $vmstate, $data) = @_;
+    my ($class, $vmid, $vmstate, $data, $opts) = @_;
 
     my $storecfg = PVE::Storage::config();
     my $params = {
@@ -517,6 +523,8 @@ sub __snapshot_rollback_vm_start {
         forcemachine => $data->{forcemachine},
         forcecpu => $data->{forcecpu},
         'nets-host-mtu' => $data->{'nets-host-mtu'},
+        agent => $data->{agent},
+        'sync-time-on-resume' => $opts->{'sync-time-on-resume'},
     };
     PVE::QemuServer::vm_start($storecfg, $vmid, $params);
 }
diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 8da6f15d..5846578d 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -1770,6 +1770,8 @@ sub phase3_cleanup {
                     $self->{errors} = 1;
                 }
             }
+            $self->log('info', "synced guest clock via guest agent")
+                if PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent});
         }
 
         if (
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index cdf66e89..e30e89e7 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5179,7 +5179,10 @@ sub vmconfig_update_agent {
 
     die "skip\n" if !$conf->{$opt};
 
-    my $hotplug_options = { fstrim_cloned_disks => 1 };
+    my $hotplug_options = {
+        fstrim_cloned_disks => 1,
+        'sync-time-on-resume' => 1,
+    };
 
     my $old_agent = parse_guest_agent($conf->{agent});
     my $agent = parse_guest_agent($value);
@@ -6006,6 +6009,19 @@ sub vm_start_nolock {
         );
     }
 
+    # $resume = suspend-to-disk, $statefile = rollback with RAM. Rollback carries the snapshot's
+    # agent setting in params; hibernate resume falls back to the live config
+    if ($resume || ($statefile && !$migratedfrom)) {
+        my $agent = $params->{agent} // $conf->{agent};
+        if (PVE::QemuServer::Agent::should_sync_time_on_resume(
+            $agent,
+            $params->{'sync-time-on-resume'},
+        )) {
+            eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 };
+            warn "could not sync guest time after resume from saved state - $@" if $@;
+        }
+    }
+
     return $res;
 }
 
diff --git a/src/PVE/QemuServer/Agent.pm b/src/PVE/QemuServer/Agent.pm
index be6df443..77cd2bd6 100644
--- a/src/PVE/QemuServer/Agent.pm
+++ b/src/PVE/QemuServer/Agent.pm
@@ -4,6 +4,7 @@ use v5.36;
 
 use JSON;
 use MIME::Base64 qw(decode_base64 encode_base64);
+use Time::HiRes qw(time);
 
 use PVE::JSONSchema;
 
@@ -52,6 +53,24 @@ our $agent_fmt = {
         optional => 1,
         default => 1,
     },
+    # FIXME: MAJOR VERSION: 10.0.0 - reconsider enabling this option as default
+    'sync-time-on-resume' => {
+        description => "Synchronize the guest clock through QGA after operations that can leave"
+            . " the guest clock behind.",
+        verbose_description =>
+            "Whether to issue the guest-set-time QEMU guest agent command to synchronize the"
+            . " guest clock to the host's current time after operations that can leave the guest"
+            . " clock behind. This happens when resuming from hibernation, resuming from a paused"
+            . " state, taking a snapshot, after a migration, and after rolling back to a snapshot"
+            . " that includes RAM. In these cases the guest's clock may still reflect an earlier"
+            . " time. The time is only synchronized when the QEMU Guest Agent option is enabled"
+            . " in the guest's configuration and the agent is running inside of the guest. For"
+            . " resume, hibernation resume and snapshot rollback, the synchronization can be"
+            . " overridden per operation.",
+        type => 'boolean',
+        optional => 1,
+        default => 0,
+    },
     type => {
         description => "Select the agent type",
         type => 'string',
@@ -332,4 +351,39 @@ sub guest_fs_freeze_applicable($agent_str, $vmid, $logfunc = undef) {
     return 1;
 }
 
+=head3 should_sync_time_on_resume
+
+Returns whether the guest's clock should be synchronized to the host's via the QEMU Guest Agent
+when the VM is resumed from saved state. Requires the guest agent to be enabled. An explicit
+C<$override> (from an API parameter) takes precedence over the C<sync-time-on-resume> agent
+config property when defined; otherwise the config value is used (defaulting to off). Does B<not>
+check whether the agent is actually running.
+
+=cut
+
+sub should_sync_time_on_resume($agent_str, $override = undef) {
+    my $agent = parse_guest_agent($agent_str);
+    return 0 if !$agent->{enabled};
+    return $override ? 1 : 0 if defined($override);
+    return $agent->{'sync-time-on-resume'} // 0;
+}
+
+=head3 guest_set_time
+
+Sets the guest's clock to the current host time via the QEMU Guest Agent's
+C<guest-set-time> command. The host time is passed explicitly (as nanoseconds
+since the UNIX epoch) because the argument-less form reads the time from the
+guest's RTC, which isn't supported on all platforms (e.g. Windows); passing
+the value directly works regardless of guest OS.
+
+=cut
+
+sub guest_set_time($vmid, $quiet = undef) {
+    my $time_ns = Time::HiRes::time() * 1_000_000_000;
+    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'guest-set-time', time => int($time_ns));
+    check_agent_error($res, "unable to set guest time");
+    print "synced guest clock via guest agent\n" if !$quiet;
+    return;
+}
+
 1;
diff --git a/src/PVE/QemuServer/RunState.pm b/src/PVE/QemuServer/RunState.pm
index bbbcc88e..a72aed4d 100644
--- a/src/PVE/QemuServer/RunState.pm
+++ b/src/PVE/QemuServer/RunState.pm
@@ -128,7 +128,7 @@ sub vm_suspend {
 # location of the config file (source or target node) is not deterministic,
 # since migration cannot wait for pmxcfs to process the rename
 sub vm_resume {
-    my ($vmid, $skiplock, $nocheck) = @_;
+    my ($vmid, $skiplock, $nocheck, $sync_time, $quiet) = @_;
 
     PVE::QemuConfig->lock_config(
         $vmid,
@@ -180,6 +180,15 @@ sub vm_resume {
                 if $resume_cmd eq 'cont';
 
             mon_cmd($vmid, $resume_cmd);
+
+            if (
+                $resume_cmd eq 'cont'
+                && PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent},
+                    $sync_time)
+            ) {
+                eval { PVE::QemuServer::Agent::guest_set_time($vmid, $quiet); };
+                warn "could not sync guest time after resume - $@" if $@;
+            }
         },
     );
 }
-- 
2.47.3

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

[Maximiliano Sandoval]

Jakob Klocker <j.klocker@proxmox.com> writes:

Some comments below.

One design comment: I am not sure if the functions should have an
$override parameter, it could be argued than it is the job of the
call-site to determine whether how to proceeded based on API+config
parameters.

disclaimer: I did not check v1 and v2's discussions.

> 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.
>
> Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
> ---
> changes since v1:
> - adapt warning message in the resume-from-saved-state path
> changes since v2:
> - add sync on pause/resume
> - add override options
> - add print task 
> - adapt formating and naming
>
>  src/PVE/API2/Qemu.pm           | 46 ++++++++++++++++++++++++++---
>  src/PVE/CLI/qm.pm              |  2 +-
>  src/PVE/QemuConfig.pm          | 12 ++++++--
>  src/PVE/QemuMigrate.pm         |  2 ++
>  src/PVE/QemuServer.pm          | 18 +++++++++++-
>  src/PVE/QemuServer/Agent.pm    | 54 ++++++++++++++++++++++++++++++++++
>  src/PVE/QemuServer/RunState.pm | 11 ++++++-
>  7 files changed, 136 insertions(+), 9 deletions(-)
>
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index 9023c344..f58151e8 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm
> @@ -3510,6 +3510,14 @@ __PACKAGE__->register_method({
>                      . ' the migration. A value of 0 means that the host_mtu parameter is to be'
>                      . ' avoided for the corresponding device.',
>              },
> +            'sync-time-on-resume' => {
> +                type => 'boolean',
> +                description =>

Please have both a `description` and a `verbose_description`. The
verbose description should mention all cases where this can happen and
if possible mention the exact QGA command used (guest-set-time).

There is a more complete verbose_description at Agent.pm below, but imho
all doc strings should be complete enough to be read on their own,
unless they reference another doc.

> +                    "Override the VM's configured 'sync-time-on-resume' agent setting for this"

This is an agent config so: "The VM QGA's configured..." or similar.

> +                    . " start only. Controls whether the guest clock is synchronized after the"
> +                    . " RAM state is restored. The guest agent must be enabled.",
> +                optional => 1,
> +            },
>          },
>      },
>      returns => {
> @@ -3525,6 +3533,7 @@ __PACKAGE__->register_method({
>          my $vmid = extract_param($param, 'vmid');
>          my $timeout = extract_param($param, 'timeout');
>          my $machine = extract_param($param, 'machine');
> +        my $sync_time = extract_param($param, 'sync-time-on-resume');
>  
>          my $get_root_param = sub {
>              my $value = extract_param($param, $_[0]);
> @@ -3631,6 +3640,7 @@ __PACKAGE__->register_method({
>                      timeout => $timeout,
>                      forcecpu => $force_cpu,
>                      'nets-host-mtu' => $nets_host_mtu,
> +                    'sync-time-on-resume' => $sync_time,
>                  };
>  
>                  PVE::QemuServer::vm_start($storecfg, $vmid, $params, $migrate_opts);
> @@ -4115,6 +4125,14 @@ __PACKAGE__->register_method({
>                  description => "Do not check whether the VM is running, used"
>                      . " internally during migration. Only root may use this option.",
>              },
> +            'sync-time-on-resume' => {
> +                type => 'boolean',
> +                description =>
> +                    "Override the VM's configured 'sync-time-on-resume' agent setting for this"
> +                    . " resume only. Controls whether the guest clock is synchronized after the"
> +                    . " RAM state is restored. The guest agent must be enabled.",

Same considerations here.

> +                optional => 1,
> +            },
>          },
>      },
>      returns => {
> @@ -4131,6 +4149,8 @@ __PACKAGE__->register_method({
>  
>          my $vmid = extract_param($param, 'vmid');
>  
> +        my $sync_time = extract_param($param, 'sync-time-on-resume');
> +
>          my $skiplock = extract_param($param, 'skiplock');
>          raise_param_exc({ skiplock => "Only root may use this option." })
>              if $skiplock && $authuser ne 'root@pam';
> @@ -4161,10 +4181,14 @@ __PACKAGE__->register_method({
>              syslog('info', "resume VM $vmid: $upid\n");
>  
>              if (!$to_disk_suspended) {
> -                PVE::QemuServer::RunState::vm_resume($vmid, $skiplock, $nocheck);
> +                PVE::QemuServer::RunState::vm_resume($vmid, $skiplock, $nocheck, $sync_time);
>              } else {
>                  my $storecfg = PVE::Storage::config();
> -                PVE::QemuServer::vm_start($storecfg, $vmid, { skiplock => $skiplock });
> +                PVE::QemuServer::vm_start(
> +                    $storecfg,
> +                    $vmid,
> +                    { skiplock => $skiplock, 'sync-time-on-resume' => $sync_time },
> +                );
>              }
>  
>              return;
> @@ -6323,6 +6347,14 @@ __PACKAGE__->register_method({
>                  optional => 1,
>                  default => 0,
>              },
> +            'sync-time-on-resume' => {
> +                type => 'boolean',
> +                description =>
> +                    "Override the VM's configured sync-time-on-resume agent setting for this"
> +                    . " rollback only. Controls whether the guest clock is synchronized"
> +                    . " after the RAM state is restored. The guest agent must be enabled.",
> +                optional => 1,

Ditto.

> +            },
>          },
>      },
>      returns => {
> @@ -6342,6 +6374,8 @@ __PACKAGE__->register_method({
>  
>          my $snapname = extract_param($param, 'snapname');
>  
> +        my $sync_time = extract_param($param, 'sync-time-on-resume');
> +
>          # vm_start is invoked directly from the worker, so its own permissions
>          # predicate doesn't fire here - check VM.PowerMgmt up front so a user
>          # with only VM.Snapshot.Rollback can't power the VM on as a side effect.
> @@ -6350,7 +6384,11 @@ __PACKAGE__->register_method({
>  
>          my $realcmd = sub {
>              PVE::Cluster::log_msg('info', $authuser, "rollback snapshot VM $vmid: $snapname");
> -            PVE::QemuConfig->snapshot_rollback($vmid, $snapname);
> +            PVE::QemuConfig->snapshot_rollback(
> +                $vmid,
> +                $snapname,
> +                { 'sync-time-on-resume' => $sync_time },
> +            );
>  
>              if ($param->{start} && !PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
>                  PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node });
> @@ -6915,7 +6953,7 @@ __PACKAGE__->register_method({
>                  },
>                  'resume' => sub {
>                      if (PVE::QemuServer::Helpers::vm_running_locally($state->{vmid})) {
> -                        PVE::QemuServer::RunState::vm_resume($state->{vmid}, 1, 1);
> +                        PVE::QemuServer::RunState::vm_resume($state->{vmid}, 1, 1, undef, 1);
>                      } else {
>                          die "VM $state->{vmid} not running\n";
>                      }
> diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
> index b903c1f1..25351f64 100755
> --- a/src/PVE/CLI/qm.pm
> +++ b/src/PVE/CLI/qm.pm
> @@ -473,7 +473,7 @@ __PACKAGE__->register_method({
>                  if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
>                      # vm_resume with nocheck, since local node might not have processed config
>                      # move/rename yet
> -                    eval { PVE::QemuServer::RunState::vm_resume($vmid, 1, 1); };
> +                    eval { PVE::QemuServer::RunState::vm_resume($vmid, 1, 1, undef, 1); };
>                      if ($@) {
>                          $tunnel_write->("ERR: resume failed - $@");
>                      } else {
> diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
> index 26f0fda2..e95a8c61 100644
> --- a/src/PVE/QemuConfig.pm
> +++ b/src/PVE/QemuConfig.pm
> @@ -383,6 +383,11 @@ sub __snapshot_create_vol_snapshots_hook {
>                      next;
>                  }
>              }
> +            my $conf = $class->load_config($vmid);
> +            if (PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent})) {
> +                eval { PVE::QemuServer::Agent::guest_set_time($vmid); };
> +                warn "could not sync guest time after snapshot - $@" if $@;
> +            }
>          }
>      }
>  }
> @@ -444,8 +449,9 @@ sub __snapshot_rollback_hook {
>      my ($class, $vmid, $conf, $snap, $prepare, $data) = @_;
>  
>      if ($prepare) {
> -        # we save the machine of the current config
> +        # we save the machine and agent of the current config
>          $data->{oldmachine} = $conf->{machine};
> +        $data->{agent} = $conf->{agent};
>      } else {
>          # if we have a 'runningmachine' entry in the snapshot we use that
>          # for the forcemachine parameter, else we use the old logic
> @@ -509,7 +515,7 @@ sub __snapshot_rollback_vm_stop {
>  }
>  
>  sub __snapshot_rollback_vm_start {
> -    my ($class, $vmid, $vmstate, $data) = @_;
> +    my ($class, $vmid, $vmstate, $data, $opts) = @_;
>  
>      my $storecfg = PVE::Storage::config();
>      my $params = {
> @@ -517,6 +523,8 @@ sub __snapshot_rollback_vm_start {
>          forcemachine => $data->{forcemachine},
>          forcecpu => $data->{forcecpu},
>          'nets-host-mtu' => $data->{'nets-host-mtu'},
> +        agent => $data->{agent},
> +        'sync-time-on-resume' => $opts->{'sync-time-on-resume'},
>      };
>      PVE::QemuServer::vm_start($storecfg, $vmid, $params);
>  }
> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
> index 8da6f15d..5846578d 100644
> --- a/src/PVE/QemuMigrate.pm
> +++ b/src/PVE/QemuMigrate.pm
> @@ -1770,6 +1770,8 @@ sub phase3_cleanup {
>                      $self->{errors} = 1;
>                  }
>              }
> +            $self->log('info', "synced guest clock via guest agent")
> +                if PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent});
>          }
>  
>          if (
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index cdf66e89..e30e89e7 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -5179,7 +5179,10 @@ sub vmconfig_update_agent {
>  
>      die "skip\n" if !$conf->{$opt};
>  
> -    my $hotplug_options = { fstrim_cloned_disks => 1 };
> +    my $hotplug_options = {
> +        fstrim_cloned_disks => 1,
> +        'sync-time-on-resume' => 1,
> +    };
>  
>      my $old_agent = parse_guest_agent($conf->{agent});
>      my $agent = parse_guest_agent($value);
> @@ -6006,6 +6009,19 @@ sub vm_start_nolock {
>          );
>      }
>  
> +    # $resume = suspend-to-disk, $statefile = rollback with RAM. Rollback carries the snapshot's
> +    # agent setting in params; hibernate resume falls back to the live config
> +    if ($resume || ($statefile && !$migratedfrom)) {
> +        my $agent = $params->{agent} // $conf->{agent};
> +        if (PVE::QemuServer::Agent::should_sync_time_on_resume(
> +            $agent,
> +            $params->{'sync-time-on-resume'},
> +        )) {
> +            eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 };
> +            warn "could not sync guest time after resume from saved state - $@" if $@;
> +        }
> +    }
> +
>      return $res;
>  }
>  
> diff --git a/src/PVE/QemuServer/Agent.pm b/src/PVE/QemuServer/Agent.pm
> index be6df443..77cd2bd6 100644
> --- a/src/PVE/QemuServer/Agent.pm
> +++ b/src/PVE/QemuServer/Agent.pm
> @@ -4,6 +4,7 @@ use v5.36;
>  
>  use JSON;
>  use MIME::Base64 qw(decode_base64 encode_base64);
> +use Time::HiRes qw(time);
>  
>  use PVE::JSONSchema;
>  
> @@ -52,6 +53,24 @@ our $agent_fmt = {
>          optional => 1,
>          default => 1,
>      },
> +    # FIXME: MAJOR VERSION: 10.0.0 - reconsider enabling this option as default

Unless it was already discussed that this should be done, I would say
this is a "TODO".

> +    'sync-time-on-resume' => {
> +        description => "Synchronize the guest clock through QGA after operations that can leave"

I think this should be "the guest's clock". Same in other places.

> +            . " the guest clock behind.",
> +        verbose_description =>
> +            "Whether to issue the guest-set-time QEMU guest agent command to synchronize the"
s/guest-set-time/'guest-set-time'/.
> +            . " guest clock to the host's current time after operations that can leave the guest"
> +            . " clock behind. This happens when resuming from hibernation, resuming from a paused"
> +            . " state, taking a snapshot, after a migration, and after rolling back to a snapshot"
> +            . " that includes RAM. In these cases the guest's clock may still reflect an earlier"
> +            . " time. The time is only synchronized when the QEMU Guest Agent option is enabled"
> +            . " in the guest's configuration and the agent is running inside of the guest. For"
> +            . " resume, hibernation resume and snapshot rollback, the synchronization can be"
> +            . " overridden per operation.",
> +        type => 'boolean',
> +        optional => 1,
> +        default => 0,
> +    },
>      type => {
>          description => "Select the agent type",
>          type => 'string',
> @@ -332,4 +351,39 @@ sub guest_fs_freeze_applicable($agent_str, $vmid, $logfunc = undef) {
>      return 1;
>  }
>  
> +=head3 should_sync_time_on_resume
> +
> +Returns whether the guest's clock should be synchronized to the host's via the QEMU Guest Agent
> +when the VM is resumed from saved state. Requires the guest agent to be enabled. An explicit
> +C<$override> (from an API parameter) takes precedence over the C<sync-time-on-resume> agent
> +config property when defined; otherwise the config value is used (defaulting to off). Does B<not>
> +check whether the agent is actually running.
> +
> +=cut
> +
> +sub should_sync_time_on_resume($agent_str, $override = undef) {
> +    my $agent = parse_guest_agent($agent_str);
> +    return 0 if !$agent->{enabled};
> +    return $override ? 1 : 0 if defined($override);
> +    return $agent->{'sync-time-on-resume'} // 0;
> +}
> +
> +=head3 guest_set_time
> +
> +Sets the guest's clock to the current host time via the QEMU Guest Agent's
> +C<guest-set-time> command. The host time is passed explicitly (as nanoseconds
> +since the UNIX epoch) because the argument-less form reads the time from the
> +guest's RTC, which isn't supported on all platforms (e.g. Windows); passing
> +the value directly works regardless of guest OS.
> +
> +=cut
> +
> +sub guest_set_time($vmid, $quiet = undef) {
> +    my $time_ns = Time::HiRes::time() * 1_000_000_000;
> +    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'guest-set-time', time => int($time_ns));
> +    check_agent_error($res, "unable to set guest time");
> +    print "synced guest clock via guest agent\n" if !$quiet;
> +    return;

Just an observation, this `return` seems unnecessary to me, however the
thaw fn also has one.

> +}
> +
>  1;
> diff --git a/src/PVE/QemuServer/RunState.pm b/src/PVE/QemuServer/RunState.pm
> index bbbcc88e..a72aed4d 100644
> --- a/src/PVE/QemuServer/RunState.pm
> +++ b/src/PVE/QemuServer/RunState.pm
> @@ -128,7 +128,7 @@ sub vm_suspend {
>  # location of the config file (source or target node) is not deterministic,
>  # since migration cannot wait for pmxcfs to process the rename
>  sub vm_resume {
> -    my ($vmid, $skiplock, $nocheck) = @_;
> +    my ($vmid, $skiplock, $nocheck, $sync_time, $quiet) = @_;
>  
>      PVE::QemuConfig->lock_config(
>          $vmid,
> @@ -180,6 +180,15 @@ sub vm_resume {
>                  if $resume_cmd eq 'cont';
>  
>              mon_cmd($vmid, $resume_cmd);
> +
> +            if (
> +                $resume_cmd eq 'cont'
> +                && PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent},
> +                    $sync_time)
> +            ) {
> +                eval { PVE::QemuServer::Agent::guest_set_time($vmid, $quiet); };
> +                warn "could not sync guest time after resume - $@" if $@;
> +            }
>          },
>      );
>  }

-- 
Maximiliano

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

[Jakob Klocker]

The initial idea for the override was that you can't opt out of the
time sync when resuming from hibernate, since the config is locked. To
let users stay flexible, Fiona and I decided an override is a good fit
here. If there's an override for hibernate, it feels natural that
there would be one for rollback and resume as well.

Some comments inline.

On 7/2/26 12:56 PM, Maximiliano Sandoval wrote:
> Jakob Klocker <j.klocker@proxmox.com> writes:
[snip]
>> +            'sync-time-on-resume' => {
>> +                type => 'boolean',
>> +                description =>
> 
> Please have both a `description` and a `verbose_description`. The
> verbose description should mention all cases where this can happen and
> if possible mention the exact QGA command used (guest-set-time).
> 
> There is a more complete verbose_description at Agent.pm below, but imho
> all doc strings should be complete enough to be read on their own,
> unless they reference another doc.
> 

I noticed no API property in `Qemu.pm` has a `verbose_description`,
therefore I didn't add one either. Is it actually desired to have
verbose descriptions for properties here?

>> +                    "Override the VM's configured 'sync-time-on-resume' agent setting for this"
> 
> This is an agent config so: "The VM QGA's configured..." or similar.
> 
>> +                    . " start only. Controls whether the guest clock is synchronized after the"
>> +                    . " RAM state is restored. The guest agent must be enabled.",
>> +                optional => 1,
>> +            },
>>          },
>>      },
>>      returns => {
>> @@ -3525,6 +3533,7 @@ __PACKAGE__->register_method({
>>          my $vmid = extract_param($param, 'vmid');
>>          my $timeout = extract_param($param, 'timeout');
>>          my $machine = extract_param($param, 'machine');
>> +        my $sync_time = extract_param($param, 'sync-time-on-resume');
>>  
>>          my $get_root_param = sub {
>>              my $value = extract_param($param, $_[0]);
>> @@ -3631,6 +3640,7 @@ __PACKAGE__->register_method({
>>                      timeout => $timeout,
>>                      forcecpu => $force_cpu,
>>                      'nets-host-mtu' => $nets_host_mtu,
>> +                    'sync-time-on-resume' => $sync_time,
>>                  };
>>  
>>                  PVE::QemuServer::vm_start($storecfg, $vmid, $params, $migrate_opts);
>> @@ -4115,6 +4125,14 @@ __PACKAGE__->register_method({
>>                  description => "Do not check whether the VM is running, used"
>>                      . " internally during migration. Only root may use this option.",
>>              },
>> +            'sync-time-on-resume' => {
>> +                type => 'boolean',
>> +                description =>
>> +                    "Override the VM's configured 'sync-time-on-resume' agent setting for this"
>> +                    . " resume only. Controls whether the guest clock is synchronized after the"
>> +                    . " RAM state is restored. The guest agent must be enabled.",
> 
> Same considerations here.
> 
>> +                optional => 1,
>> +            },
>>          },
>>      },
>>      returns => {
>> @@ -4131,6 +4149,8 @@ __PACKAGE__->register_method({
>>  
>>          my $vmid = extract_param($param, 'vmid');
>>  
>> +        my $sync_time = extract_param($param, 'sync-time-on-resume');
>> +
>>          my $skiplock = extract_param($param, 'skiplock');
>>          raise_param_exc({ skiplock => "Only root may use this option." })
>>              if $skiplock && $authuser ne 'root@pam';
>> @@ -4161,10 +4181,14 @@ __PACKAGE__->register_method({
>>              syslog('info', "resume VM $vmid: $upid\n");
>>  
>>              if (!$to_disk_suspended) {
>> -                PVE::QemuServer::RunState::vm_resume($vmid, $skiplock, $nocheck);
>> +                PVE::QemuServer::RunState::vm_resume($vmid, $skiplock, $nocheck, $sync_time);
>>              } else {
>>                  my $storecfg = PVE::Storage::config();
>> -                PVE::QemuServer::vm_start($storecfg, $vmid, { skiplock => $skiplock });
>> +                PVE::QemuServer::vm_start(
>> +                    $storecfg,
>> +                    $vmid,
>> +                    { skiplock => $skiplock, 'sync-time-on-resume' => $sync_time },
>> +                );
>>              }
>>  
>>              return;
>> @@ -6323,6 +6347,14 @@ __PACKAGE__->register_method({
>>                  optional => 1,
>>                  default => 0,
>>              },
>> +            'sync-time-on-resume' => {
>> +                type => 'boolean',
>> +                description =>
>> +                    "Override the VM's configured sync-time-on-resume agent setting for this"
>> +                    . " rollback only. Controls whether the guest clock is synchronized"
>> +                    . " after the RAM state is restored. The guest agent must be enabled.",
>> +                optional => 1,
> 
> Ditto.
> 
>> +            },
>>          },
>>      },
>>      returns => {
>> @@ -6342,6 +6374,8 @@ __PACKAGE__->register_method({
>>  
>>          my $snapname = extract_param($param, 'snapname');
>>  
>> +        my $sync_time = extract_param($param, 'sync-time-on-resume');
>> +
>>          # vm_start is invoked directly from the worker, so its own permissions
>>          # predicate doesn't fire here - check VM.PowerMgmt up front so a user
>>          # with only VM.Snapshot.Rollback can't power the VM on as a side effect.
>> @@ -6350,7 +6384,11 @@ __PACKAGE__->register_method({
>>  
>>          my $realcmd = sub {
>>              PVE::Cluster::log_msg('info', $authuser, "rollback snapshot VM $vmid: $snapname");
>> -            PVE::QemuConfig->snapshot_rollback($vmid, $snapname);
>> +            PVE::QemuConfig->snapshot_rollback(
>> +                $vmid,
>> +                $snapname,
>> +                { 'sync-time-on-resume' => $sync_time },
>> +            );
>>  
>>              if ($param->{start} && !PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
>>                  PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node });
>> @@ -6915,7 +6953,7 @@ __PACKAGE__->register_method({
>>                  },
>>                  'resume' => sub {
>>                      if (PVE::QemuServer::Helpers::vm_running_locally($state->{vmid})) {
>> -                        PVE::QemuServer::RunState::vm_resume($state->{vmid}, 1, 1);
>> +                        PVE::QemuServer::RunState::vm_resume($state->{vmid}, 1, 1, undef, 1);
>>                      } else {
>>                          die "VM $state->{vmid} not running\n";
>>                      }
>> diff --git a/src/PVE/CLI/qm.pm b/src/PVE/CLI/qm.pm
>> index b903c1f1..25351f64 100755
>> --- a/src/PVE/CLI/qm.pm
>> +++ b/src/PVE/CLI/qm.pm
>> @@ -473,7 +473,7 @@ __PACKAGE__->register_method({
>>                  if (PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
>>                      # vm_resume with nocheck, since local node might not have processed config
>>                      # move/rename yet
>> -                    eval { PVE::QemuServer::RunState::vm_resume($vmid, 1, 1); };
>> +                    eval { PVE::QemuServer::RunState::vm_resume($vmid, 1, 1, undef, 1); };
>>                      if ($@) {
>>                          $tunnel_write->("ERR: resume failed - $@");
>>                      } else {
>> diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
>> index 26f0fda2..e95a8c61 100644
>> --- a/src/PVE/QemuConfig.pm
>> +++ b/src/PVE/QemuConfig.pm
>> @@ -383,6 +383,11 @@ sub __snapshot_create_vol_snapshots_hook {
>>                      next;
>>                  }
>>              }
>> +            my $conf = $class->load_config($vmid);
>> +            if (PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent})) {
>> +                eval { PVE::QemuServer::Agent::guest_set_time($vmid); };
>> +                warn "could not sync guest time after snapshot - $@" if $@;
>> +            }
>>          }
>>      }
>>  }
>> @@ -444,8 +449,9 @@ sub __snapshot_rollback_hook {
>>      my ($class, $vmid, $conf, $snap, $prepare, $data) = @_;
>>  
>>      if ($prepare) {
>> -        # we save the machine of the current config
>> +        # we save the machine and agent of the current config
>>          $data->{oldmachine} = $conf->{machine};
>> +        $data->{agent} = $conf->{agent};
>>      } else {
>>          # if we have a 'runningmachine' entry in the snapshot we use that
>>          # for the forcemachine parameter, else we use the old logic
>> @@ -509,7 +515,7 @@ sub __snapshot_rollback_vm_stop {
>>  }
>>  
>>  sub __snapshot_rollback_vm_start {
>> -    my ($class, $vmid, $vmstate, $data) = @_;
>> +    my ($class, $vmid, $vmstate, $data, $opts) = @_;
>>  
>>      my $storecfg = PVE::Storage::config();
>>      my $params = {
>> @@ -517,6 +523,8 @@ sub __snapshot_rollback_vm_start {
>>          forcemachine => $data->{forcemachine},
>>          forcecpu => $data->{forcecpu},
>>          'nets-host-mtu' => $data->{'nets-host-mtu'},
>> +        agent => $data->{agent},
>> +        'sync-time-on-resume' => $opts->{'sync-time-on-resume'},
>>      };
>>      PVE::QemuServer::vm_start($storecfg, $vmid, $params);
>>  }
>> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
>> index 8da6f15d..5846578d 100644
>> --- a/src/PVE/QemuMigrate.pm
>> +++ b/src/PVE/QemuMigrate.pm
>> @@ -1770,6 +1770,8 @@ sub phase3_cleanup {
>>                      $self->{errors} = 1;
>>                  }
>>              }
>> +            $self->log('info', "synced guest clock via guest agent")
>> +                if PVE::QemuServer::Agent::should_sync_time_on_resume($conf->{agent});
>>          }
>>  
>>          if (
>> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
>> index cdf66e89..e30e89e7 100644
>> --- a/src/PVE/QemuServer.pm
>> +++ b/src/PVE/QemuServer.pm
>> @@ -5179,7 +5179,10 @@ sub vmconfig_update_agent {
>>  
>>      die "skip\n" if !$conf->{$opt};
>>  
>> -    my $hotplug_options = { fstrim_cloned_disks => 1 };
>> +    my $hotplug_options = {
>> +        fstrim_cloned_disks => 1,
>> +        'sync-time-on-resume' => 1,
>> +    };
>>  
>>      my $old_agent = parse_guest_agent($conf->{agent});
>>      my $agent = parse_guest_agent($value);
>> @@ -6006,6 +6009,19 @@ sub vm_start_nolock {
>>          );
>>      }
>>  
>> +    # $resume = suspend-to-disk, $statefile = rollback with RAM. Rollback carries the snapshot's
>> +    # agent setting in params; hibernate resume falls back to the live config
>> +    if ($resume || ($statefile && !$migratedfrom)) {
>> +        my $agent = $params->{agent} // $conf->{agent};
>> +        if (PVE::QemuServer::Agent::should_sync_time_on_resume(
>> +            $agent,
>> +            $params->{'sync-time-on-resume'},
>> +        )) {
>> +            eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 };
>> +            warn "could not sync guest time after resume from saved state - $@" if $@;
>> +        }
>> +    }
>> +
>>      return $res;
>>  }
>>  
>> diff --git a/src/PVE/QemuServer/Agent.pm b/src/PVE/QemuServer/Agent.pm
>> index be6df443..77cd2bd6 100644
>> --- a/src/PVE/QemuServer/Agent.pm
>> +++ b/src/PVE/QemuServer/Agent.pm
>> @@ -4,6 +4,7 @@ use v5.36;
>>  
>>  use JSON;
>>  use MIME::Base64 qw(decode_base64 encode_base64);
>> +use Time::HiRes qw(time);
>>  
>>  use PVE::JSONSchema;
>>  
>> @@ -52,6 +53,24 @@ our $agent_fmt = {
>>          optional => 1,
>>          default => 1,
>>      },
>> +    # FIXME: MAJOR VERSION: 10.0.0 - reconsider enabling this option as default
> 
> Unless it was already discussed that this should be done, I would say
> this is a "TODO".
> 

>From what I've seen TODO and FIXME is used in the codebase, since
Fiona suggested FIXME in v2 I've sticked to it.

>> +    'sync-time-on-resume' => {
>> +        description => "Synchronize the guest clock through QGA after operations that can leave"
> 
> I think this should be "the guest's clock". Same in other places.

This reads nicer, I'll make sure to change it.

> 
>> +            . " the guest clock behind.",
>> +        verbose_description =>
>> +            "Whether to issue the guest-set-time QEMU guest agent command to synchronize the"
> s/guest-set-time/'guest-set-time'/.
>> +            . " guest clock to the host's current time after operations that can leave the guest"
>> +            . " clock behind. This happens when resuming from hibernation, resuming from a paused"
>> +            . " state, taking a snapshot, after a migration, and after rolling back to a snapshot"
>> +            . " that includes RAM. In these cases the guest's clock may still reflect an earlier"
>> +            . " time. The time is only synchronized when the QEMU Guest Agent option is enabled"
>> +            . " in the guest's configuration and the agent is running inside of the guest. For"
>> +            . " resume, hibernation resume and snapshot rollback, the synchronization can be"
>> +            . " overridden per operation.",
>> +        type => 'boolean',
>> +        optional => 1,
>> +        default => 0,
>> +    },
>>      type => {
>>          description => "Select the agent type",
>>          type => 'string',
>> @@ -332,4 +351,39 @@ sub guest_fs_freeze_applicable($agent_str, $vmid, $logfunc = undef) {
>>      return 1;
>>  }
>>  
>> +=head3 should_sync_time_on_resume
>> +
>> +Returns whether the guest's clock should be synchronized to the host's via the QEMU Guest Agent
>> +when the VM is resumed from saved state. Requires the guest agent to be enabled. An explicit
>> +C<$override> (from an API parameter) takes precedence over the C<sync-time-on-resume> agent
>> +config property when defined; otherwise the config value is used (defaulting to off). Does B<not>
>> +check whether the agent is actually running.
>> +
>> +=cut
>> +
>> +sub should_sync_time_on_resume($agent_str, $override = undef) {
>> +    my $agent = parse_guest_agent($agent_str);
>> +    return 0 if !$agent->{enabled};
>> +    return $override ? 1 : 0 if defined($override);
>> +    return $agent->{'sync-time-on-resume'} // 0;
>> +}
>> +
>> +=head3 guest_set_time
>> +
>> +Sets the guest's clock to the current host time via the QEMU Guest Agent's
>> +C<guest-set-time> command. The host time is passed explicitly (as nanoseconds
>> +since the UNIX epoch) because the argument-less form reads the time from the
>> +guest's RTC, which isn't supported on all platforms (e.g. Windows); passing
>> +the value directly works regardless of guest OS.
>> +
>> +=cut
>> +
>> +sub guest_set_time($vmid, $quiet = undef) {
>> +    my $time_ns = Time::HiRes::time() * 1_000_000_000;
>> +    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'guest-set-time', time => int($time_ns));
>> +    check_agent_error($res, "unable to set guest time");
>> +    print "synced guest clock via guest agent\n" if !$quiet;
>> +    return;
> 
> Just an observation, this `return` seems unnecessary to me, however the
> thaw fn also has one.

I've added it to stay consistent with the function above, as you
mentioned thaw also has one.
> 
[snip]

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

[Jakob Klocker]

On 7/2/26 12:57 PM, Maximiliano Sandoval wrote:
> Jakob Klocker <j.klocker@proxmox.com> writes:
> 
> Some comments below.
> 
> One design comment: I am not sure if the functions should have an
> $override parameter, it could be argued than it is the job of the
> call-site to determine whether how to proceeded based on API+config
> parameters.
> 
> disclaimer: I did not check v1 and v2's discussions.
Ignore my previous message regarding the override, I misread your
message. As discussed off-list, I will keep the $override parameter in
the `should_sync_time_on_resume` function for now, since the
responsibility of the function is to check whether
`sync-time-on-resume` should be invoked.

Thanks again for the feedback.

Re: [PATCH guest-common/manager/qemu-server v3 0/3] fix #5032: add guest time sync via QGA

[Maximiliano Sandoval]

Jakob Klocker <j.klocker@proxmox.com> writes:

> This series adds a new agent option 'sync-time-on-resume' to
> automatically synchronize the guest clock via the QEMU Guest Agent 
> after operations that can leave the guest time stale. The option is 
> disabled by default and only takes effect when the QEMU Guest Agent is
> enabled.
>
> When a VM resumes with a restored RAM state (waking from hibernation or
> rolling back to a snapshot that includes RAM), the guest clock 
> continues from the point the state was saved and no longer matches 
> wall-clock time. Skews also appears whenever the guest is briefly 
> frozen and resumed - after creating a snapshot, an
> ordinary pause/resume, or the resume step of a live migration. 
>
> When enabled, the option triggers a guest-set-time call:
>  * after resuming from hibernation (suspend-to-disk)
>  * after rolling back to a snapshot that includes RAM
>  * after a live migration
>  * after an ordinary pause/resume
>  * after taking a snapshot
>
> The start, resume and rollback API endpoints also accept a
> 'sync-time-on-resume' parameter that overrides the configured value for
> a single operation. 
>
> The persistent config option is exposed in the QEMU Guest Agent
> configuration GUI, so it can be toggled per guest.
>
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5032
>
> changes from v1 to v2 (thanks @Arthur)
> - qemu: adapt warning message in the resume-from-saved-state path
> - ui: reword the agent option label for clarity
>
> changes from v2 to v3 (thanks @Fiona)
> - add sync on pause/resume
> - add override options
> - add print task 
> - adapt formating and naming

Tested:

- Create a new VM (100) without installing/enabling the QGA yet
- Ran: qm set 100 --agent enabled=1,sync-time-on-resume=1
- Took a snapshot with ram: The following line appears in the task logs:

could not sync guest time after snapshot - VM 100 qga command 'guest-set-time' failed - got timeout

- Rollback to the new snapshot, The following line appears in the task journal:

  could not sync guest time after resume from saved state - VM 100 qga command 'guest-set-time' failed - got timeout

- Enabled the guest agent inside the guest
- Did a new snapshot with RAM, the following line appears in the task log:

synced guest clock via guest agent

- Performed a rollback to the latest snapshot. The same line appears in the task log

If one additionally adds the following qemu-guest-agent config:

```
# cat /etc/qemu/qemu-ga.conf
[general]
verbose = 1
```
One can see the following when taking the snapshot:

```
Jul 02 16:56:57 pve qemu-ga[1660]: 1783004217.298332: debug: read data, count: 132, data: {"execute":"guest-sync-delimited","arguments":{"id":2108601}}
Jul 02 16:56:57 pve qemu-ga[1660]: {"arguments":{"time":1783004217540955904},"execute":"guest-set-time"}
Jul 02 16:56:57 pve qemu-ga[1660]: 1783004217.298563: debug: process_event: called
Jul 02 16:56:57 pve qemu-ga[1660]: 1783004217.298572: debug: processing command
Jul 02 16:56:57 pve qemu-ga[1660]: 1783004217.298595: debug: sending data, count: 21
Jul 02 16:56:57 pve qemu-ga[1660]: 1783004217.298644: debug: process_event: called
Jul 02 16:56:57 pve qemu-ga[1660]: 1783004217.298649: debug: processing command
Jul 02 16:56:57 pve qemu-ga[1660]: 1783004217.541002: debug: g_unix_open_pipe() called with FD_CLOEXEC; please migrate to using O_CLOEXEC instead
Jul 02 16:56:58 pve qemu-ga[1660]: 1783004218.501531: debug: sending data, count: 15
```

I cannot not check whether the date was actually set (since the snapshot
didn't take more than a second), but the passed value matches.

```
$ date -d @1783004217  # 1_783_004_217_540_955_904 from above "divided" by 10^9
Thu Jul  2 04:56:57 PM CEST 2026
```

@jakob: One thing that would be nice to improve is the error message
when the guest agent is not enabled inside the guest. Perhaps it is
possible to print a message saying that we skipped the guest-set-time
command because the guest agent was not running, similarly to what we do
for the guest-fsfreeze-freeze calls. However, it might be undesirable to
add yet another guest agent call just to check if it is there though.

Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

-- 
Maximiliano

Re: [PATCH guest-common/manager/qemu-server v3 0/3] fix #5032: add guest time sync via QGA

[Jakob Klocker]

Thanks for the feedback!
I'll adjust the error message to match`guest_fs_freeze_applicable` in
a v4.

On 7/2/26 5:14 PM, Maximiliano Sandoval wrote:
> [snip]
> @jakob: One thing that would be nice to improve is the error message
> when the guest agent is not enabled inside the guest. Perhaps it is
> possible to print a message saying that we skipped the guest-set-time
> command because the guest agent was not running, similarly to what we do
> for the guest-fsfreeze-freeze calls. However, it might be undesirable to
> add yet another guest agent call just to check if it is there though.
> 
> Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>