public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state
@ 2026-06-22 13:47 Jakob Klocker
  2026-06-22 13:47 ` [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of " Jakob Klocker
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jakob Klocker @ 2026-06-22 13:47 UTC (permalink / raw)
  To: pve-devel; +Cc: Jakob Klocker

This series adds a new agent option set-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 
enabled by default when the QEMU Guest Agent is configured.

When a VM is restored from a saved state (hibernation or snapshot with
RAM), the guest clock continues from the point the state was saved and
no longer matches wall-clock time. The same skew can appear after
creating a snapshot of a running VM. For guests whose OS does not
correct this on its own, the drift persists until the next NTP sync (if
any), which can cause problems for time-sensitive workloads.

This series introduces the set-time-on-resume agent option and triggers
a guest-set-time call:
 * after restoring a VM from a saved state or snapshot with RAM
 * after taking a snapshot

The option is exposed in the VM configuration GUI so it can be toggled
per guest.

This series depends on [0] being applied first.

[0] https://lore.proxmox.com/pve-devel/20260612100758.116697-1-j.klocker@proxmox.com/T/#u

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

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

qemu-server:

Jakob Klocker (1):
  fix #5032: qemu: sync guest time on resume and snapshot of saved state

 src/PVE/QemuConfig.pm       |  7 ++++++
 src/PVE/QemuServer.pm       | 10 ++++++++
 src/PVE/QemuServer/Agent.pm | 47 +++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)


pve-manager:

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

 www/manager6/Utils.js                     |  2 ++
 www/manager6/form/AgentFeatureSelector.js | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)


Summary over all repositories:
  5 files changed, 86 insertions(+), 0 deletions(-)

-- 
Generated by murpp 0.12.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of saved state
  2026-06-22 13:47 [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state Jakob Klocker
@ 2026-06-22 13:47 ` Jakob Klocker
  2026-06-24 11:31   ` Fiona Ebner
  2026-06-22 13:47 ` [PATCH pve-manager v2 2/2] fix #5032: ui: qemu agent: add set-time-on-resume option Jakob Klocker
  2026-06-23 14:10 ` [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state Michael Köppl
  2 siblings, 1 reply; 7+ messages in thread
From: Jakob Klocker @ 2026-06-22 13:47 UTC (permalink / raw)
  To: pve-devel; +Cc: Jakob Klocker

When a VM is resumed from a saved state (hibernation or snapshot with
RAM), the guest clock may be stale. A time skew can occur when
creating a snapshot of a running VM.

Add a new agent option `set-time-on-resume` to automatically
synchronize the guest time with the host using the QEMU Guest Agent.

Trigger time synchronization:
- after restoring a VM from a saved state or snapshot with RAM
- after taking a snapshot

This ensures consistent guest time after rollback, restore, or
snapshot operations when the guest OS clock does not automatically
correct itself.

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

Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---

changes since v1:
- adapt warning message in the resume-from-saved-state path

 src/PVE/QemuConfig.pm       |  7 ++++++
 src/PVE/QemuServer.pm       | 10 ++++++++
 src/PVE/QemuServer/Agent.pm | 47 +++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
index c24eb835..3f0ff663 100644
--- a/src/PVE/QemuConfig.pm
+++ b/src/PVE/QemuConfig.pm
@@ -383,6 +383,13 @@ sub __snapshot_create_vol_snapshots_hook {
                     next;
                 }
             }
+            if ($snap->{vmstate}) {
+                my $conf = $class->load_config($vmid);
+                if (PVE::QemuServer::Agent::should_set_time_on_resume($conf->{agent})) {
+                    eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 }
+                        or warn "could not sync guest time after snapshot - $@";
+                }
+            }
         }
     }
 }
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 55e9f520..daf10904 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5992,6 +5992,16 @@ sub vm_start_nolock {
         );
     }
 
+    my $from_saved_state = $resume || ($statefile && !$migratedfrom);
+
+    if (
+        $from_saved_state
+        && PVE::QemuServer::Agent::should_set_time_on_resume($conf->{agent})
+    ) {
+        eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 }
+            or warn "could not sync guest time after resume from saved state - $@";
+    }
+
     return $res;
 }
 
diff --git a/src/PVE/QemuServer/Agent.pm b/src/PVE/QemuServer/Agent.pm
index be6df443..be8bbae6 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 ();
 
 use PVE::JSONSchema;
 
@@ -18,6 +19,8 @@ our @EXPORT_OK = qw(
     get_qga_key
     parse_guest_agent
     qga_check_running
+    should_set_time_on_resume
+    guest_set_time
 );
 
 our $agent_fmt = {
@@ -52,6 +55,21 @@ our $agent_fmt = {
         optional => 1,
         default => 1,
     },
+    'set-time-on-resume' => {
+        description => "Update the guest clock through QGA after resuming from"
+            . " hibernation or rolling back to a snapshot with RAM.",
+        verbose_description =>
+            "Whether to issue the guest-set-time QEMU guest agent command after the VM"
+            . " resumes with a restored RAM state, that is, when waking from hibernation"
+            . " or after rolling back to a snapshot that includes RAM. In these cases the"
+            . " guest's clock still reflects the time the state was saved. With this"
+            . " option enabled, the clock is synchronized to the host's current time,"
+            . " provided the QEMU Guest Agent option is enabled in the guest's"
+            . " configuration and the agent is running inside of the guest.",
+        type => 'boolean',
+        optional => 1,
+        default => 1,
+    },
     type => {
         description => "Select the agent type",
         type => 'string',
@@ -332,4 +350,33 @@ sub guest_fs_freeze_applicable($agent_str, $vmid, $logfunc = undef) {
     return 1;
 }
 
+=head3 should_set_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. Does B<not> check whether the agent is actually running.
+
+=cut
+
+sub should_set_time_on_resume($agent_str) {
+    my $agent = parse_guest_agent($agent_str);
+    return 0 if !$agent->{enabled};
+    return $agent->{'set-time-on-resume'} // 1;
+}
+
+=head3 guest_set_time
+
+Sets the guest's clock via the QEMU Guest Agent's C<guest-set-time> command. If C<$time_ns>
+(nanoseconds since the UNIX epoch, UTC) is not given, the current host time is used. Passing
+an explicit time is required because the agent's argument-less form reads the guest's RTC,
+which may itself be stale after a vmstate snapshot or resume.
+
+=cut
+
+sub guest_set_time($vmid, $time_ns = undef) {
+    $time_ns //= int(Time::HiRes::time() * 1_000_000_000);
+    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'guest-set-time', time => $time_ns);
+    check_agent_error($res, "unable to set guest time");
+    return;
+}
+
 1;
-- 
2.47.3




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH pve-manager v2 2/2] fix #5032: ui: qemu agent: add set-time-on-resume option
  2026-06-22 13:47 [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state Jakob Klocker
  2026-06-22 13:47 ` [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of " Jakob Klocker
@ 2026-06-22 13:47 ` Jakob Klocker
  2026-06-23 14:10 ` [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state Michael Köppl
  2 siblings, 0 replies; 7+ messages in thread
From: Jakob Klocker @ 2026-06-22 13:47 UTC (permalink / raw)
  To: pve-devel; +Cc: Jakob Klocker

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

This allows enabling/disabling automatic guest clock
synchronization after resume from saved state or snapshot
restore with RAM.

Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5032
Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
---
changes since v1:
- reword the agent option label for clarity

 www/manager6/Utils.js                     |  2 ++
 www/manager6/form/AgentFeatureSelector.js | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index bbf59d8f..20862875 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -523,6 +523,8 @@ Ext.define('PVE.Utils', {
                     continue;
                 } else if (key === 'freeze-fs' && PVE.Parser.parseBoolean(value)) {
                     continue;
+                } else if (key === 'set-time-on-resume' && PVE.Parser.parseBoolean(value)) {
+                    continue;
                 } else if (PVE.Parser.parseBoolean(value)) {
                     displayText = Proxmox.Utils.enabledText;
                 }
diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 3cae4194..a7cec6c2 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: 'set-time-on-resume',
+            reference: 'set_time_on_resume',
+            bind: {
+                disabled: '{!enabled.checked}',
+            },
+            disabled: true,
+            uncheckedValue: '0',
+            defaultValue: '1',
+        },
         {
             xtype: 'displayfield',
             userCls: 'pmx-hint',
@@ -74,6 +88,9 @@ Ext.define('PVE.form.AgentFeatureSelector', {
         if (PVE.Parser.parseBoolean(values['freeze-fs'])) {
             delete values['freeze-fs'];
         }
+        if (PVE.Parser.parseBoolean(values['set-time-on-resume'])) {
+            delete values['set-time-on-resume'];
+        }
 
         const agentstr = PVE.Parser.printPropertyString(values, 'enabled');
         return { agent: agentstr };
@@ -90,6 +107,9 @@ Ext.define('PVE.form.AgentFeatureSelector', {
         if (!Ext.isDefined(res['freeze-fs'])) {
             res['freeze-fs'] = 1;
         }
+        if (!Ext.isDefined(res['set-time-on-resume'])) {
+            res['set-time-on-resume'] = 1;
+        }
 
         this.callParent([res]);
     },
-- 
2.47.3




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state
  2026-06-22 13:47 [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state Jakob Klocker
  2026-06-22 13:47 ` [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of " Jakob Klocker
  2026-06-22 13:47 ` [PATCH pve-manager v2 2/2] fix #5032: ui: qemu agent: add set-time-on-resume option Jakob Klocker
@ 2026-06-23 14:10 ` Michael Köppl
  2 siblings, 0 replies; 7+ messages in thread
From: Michael Köppl @ 2026-06-23 14:10 UTC (permalink / raw)
  To: Jakob Klocker, pve-devel

Tested these patches in combination with proxmox-test-tools.
proxmox-test-instance would regularly run into problems with rollbacks
of entire clusters of test instances when using snapshots with RAM. I
ran automated test runs with rollbacks with RAM in between and checked
that the guest time was the same as on the host for each new test that
was run. Works as expected!

Had a look at the code as well. With the patch mentioned in the cover
letter applied before, the code lgtm as well!

Consider this:
Tested-by: Michael Köppl <m.koeppl@proxmox.com>
Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>

On Mon Jun 22, 2026 at 3:47 PM CEST, Jakob Klocker wrote:
> This series adds a new agent option set-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 
> enabled by default when the QEMU Guest Agent is configured.

[snip]




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of saved state
  2026-06-22 13:47 ` [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of " Jakob Klocker
@ 2026-06-24 11:31   ` Fiona Ebner
  2026-06-25  7:46     ` Jakob Klocker
  0 siblings, 1 reply; 7+ messages in thread
From: Fiona Ebner @ 2026-06-24 11:31 UTC (permalink / raw)
  To: Jakob Klocker, pve-devel

Am 22.06.26 um 3:47 PM schrieb Jakob Klocker:
> When a VM is resumed from a saved state (hibernation or snapshot with
> RAM), the guest clock may be stale. A time skew can occur when
> creating a snapshot of a running VM.
> 
> Add a new agent option `set-time-on-resume` to automatically
> synchronize the guest time with the host using the QEMU Guest Agent.
> 
> Trigger time synchronization:
> - after restoring a VM from a saved state or snapshot with RAM
> - after taking a snapshot
> 
> This ensures consistent guest time after rollback, restore, or
> snapshot operations when the guest OS clock does not automatically
> correct itself.
> 

It's not limited to snapshots with RAM. A snapshot operation without RAM
will also pause the guest. And I think it also makes sense to do it for
resume after a pause.

For rollback, the agent option from the saved snapshot configuration
will be used, right? So if I created the snapshot originally with
'set-time-on-resume', then it will always be applied upon rollback with
no way to opt-out. And vice-versa if disabled. Should we add an option
to the 'qm rollback' command to allow overriding this? Or
unconditionally use the current value for 'set-time-on-resume' instead
upon rollback? Not sure if that would be more in line with expectations
or more surprising?

Not quite the same for resume from hibernation, but it will also be
impossible to modify the setting if already suspended, because of the
config lock.

And for a paused VM, it would be impossible to change because 'agent'
property changes are not (yet) hot-plugged, but that might make sense to
do actually. But not sure if that would be better done as part of a
major release.

On the other hand, resume (from pause and suspend) could also gain
support for an option to override the 'set-time-on-resume' setting.

Maybe the command options could default to the current guest config
setting if not explicitly specified?

What do you think?

> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5032
> 
> Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
> Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
> 
> changes since v1:
> - adapt warning message in the resume-from-saved-state path
> 
>  src/PVE/QemuConfig.pm       |  7 ++++++
>  src/PVE/QemuServer.pm       | 10 ++++++++
>  src/PVE/QemuServer/Agent.pm | 47 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+)
> 
> diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
> index c24eb835..3f0ff663 100644
> --- a/src/PVE/QemuConfig.pm
> +++ b/src/PVE/QemuConfig.pm
> @@ -383,6 +383,13 @@ sub __snapshot_create_vol_snapshots_hook {
>                      next;
>                  }
>              }
> +            if ($snap->{vmstate}) {
> +                my $conf = $class->load_config($vmid);
> +                if (PVE::QemuServer::Agent::should_set_time_on_resume($conf->{agent})) {
> +                    eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 }
> +                        or warn "could not sync guest time after snapshot - $@";

Style nit: our code base uses the following pattern:
eval { func(); };
warn "msg - $@" if $@;

> +                }
> +            }
>          }
>      }
>  }
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index 55e9f520..daf10904 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -5992,6 +5992,16 @@ sub vm_start_nolock {
>          );
>      }
>  
> +    my $from_saved_state = $resume || ($statefile && !$migratedfrom);

Style nit: I'd kinda prefer to avoid this one-off variable. Maybe add a
short comment near the expression instead?

> +
> +    if (
> +        $from_saved_state
> +        && PVE::QemuServer::Agent::should_set_time_on_resume($conf->{agent})
> +    ) {
> +        eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 }
> +            or warn "could not sync guest time after resume from saved state - $@";

Same style nit as above.

> +    }
> +
>      return $res;
>  }
>  
> diff --git a/src/PVE/QemuServer/Agent.pm b/src/PVE/QemuServer/Agent.pm
> index be6df443..be8bbae6 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 ();

Stlye nit: I'd use qw() for consistency with the previous line.

>  
>  use PVE::JSONSchema;
>  
> @@ -18,6 +19,8 @@ our @EXPORT_OK = qw(
>      get_qga_key
>      parse_guest_agent
>      qga_check_running
> +    should_set_time_on_resume
> +    guest_set_time

I don't think these are worth exporting. You already call them with the
module prefix and I do think it's helpful to be explicit like that.

>  );
>  
>  our $agent_fmt = {
> @@ -52,6 +55,21 @@ our $agent_fmt = {
>          optional => 1,
>          default => 1,
>      },
> +    'set-time-on-resume' => {

I wonder if 'set' is the best? I'm thinking about 'update' or
'synchronize', but maybe those make it too long? 'sync'? Not sure. What
do you think?

> +        description => "Update the guest clock through QGA after resuming from"
> +            . " hibernation or rolling back to a snapshot with RAM.",
> +        verbose_description =>
> +            "Whether to issue the guest-set-time QEMU guest agent command after the VM"
> +            . " resumes with a restored RAM state, that is, when waking from hibernation"
> +            . " or after rolling back to a snapshot that includes RAM. In these cases the"
> +            . " guest's clock still reflects the time the state was saved. With this"

Nit: s/was saved/was saved at/

The description does not mention that it also happens after a snapshot
create operation.

> +            . " option enabled, the clock is synchronized to the host's current time,"
> +            . " provided the QEMU Guest Agent option is enabled in the guest's"
> +            . " configuration and the agent is running inside of the guest.",
> +        type => 'boolean',
> +        optional => 1,
> +        default => 1,

I don't think we should enable this by default, at least not outside of
a major release. What if something relies on the current behavior and
expects a rolled-back state to have the old time?

> +    },
>      type => {
>          description => "Select the agent type",
>          type => 'string',
> @@ -332,4 +350,33 @@ sub guest_fs_freeze_applicable($agent_str, $vmid, $logfunc = undef) {
>      return 1;
>  }
>  
> +=head3 should_set_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. Does B<not> check whether the agent is actually running.
> +
> +=cut
> +
> +sub should_set_time_on_resume($agent_str) {
> +    my $agent = parse_guest_agent($agent_str);
> +    return 0 if !$agent->{enabled};
> +    return $agent->{'set-time-on-resume'} // 1;
> +}
> +
> +=head3 guest_set_time
> +
> +Sets the guest's clock via the QEMU Guest Agent's C<guest-set-time> command. If C<$time_ns>
> +(nanoseconds since the UNIX epoch, UTC) is not given, the current host time is used. Passing
> +an explicit time is required because the agent's argument-less form reads the guest's RTC,
> +which may itself be stale after a vmstate snapshot or resume.
> +
> +=cut
> +
> +sub guest_set_time($vmid, $time_ns = undef) {

Not sure if adding the time parameter is worth it if it's not used. We
can still add it later if a caller that needs it pops up.

> +    $time_ns //= int(Time::HiRes::time() * 1_000_000_000);
> +    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'guest-set-time', time => $time_ns);

Nit: I'd prefer the coercing to int() to be done here when passing the
param to mon_cmd()

> +    check_agent_error($res, "unable to set guest time");
> +    return;
> +}
> +
>  1;





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of saved state
  2026-06-24 11:31   ` Fiona Ebner
@ 2026-06-25  7:46     ` Jakob Klocker
  2026-06-25  8:10       ` Fiona Ebner
  0 siblings, 1 reply; 7+ messages in thread
From: Jakob Klocker @ 2026-06-25  7:46 UTC (permalink / raw)
  To: Fiona Ebner, pve-devel

Thanks for the thorough feedback!
Comments are inline.

On 6/24/26 1:31 PM, Fiona Ebner wrote:
> Am 22.06.26 um 3:47 PM schrieb Jakob Klocker:
>> When a VM is resumed from a saved state (hibernation or snapshot with
>> RAM), the guest clock may be stale. A time skew can occur when
>> creating a snapshot of a running VM.
>>
>> Add a new agent option `set-time-on-resume` to automatically
>> synchronize the guest time with the host using the QEMU Guest Agent.
>>
>> Trigger time synchronization:
>> - after restoring a VM from a saved state or snapshot with RAM
>> - after taking a snapshot
>>
>> This ensures consistent guest time after rollback, restore, or
>> snapshot operations when the guest OS clock does not automatically
>> correct itself.
>>
> 
> It's not limited to snapshots with RAM. A snapshot operation without RAM
> will also pause the guest. And I think it also makes sense to do it for
> resume after a pause.
>
Thanks for pointing that out, I missed that. I'll make sure to sync
the time for snapshots without RAM as well.

I will look into time sync for resume and add it in this series. I
question whether we want one agent option for all the time syncs (on
snapshot, rollback, pause/start) or make it more fine-grained. I'd
lean toward keeping a single option.
> For rollback, the agent option from the saved snapshot configuration
> will be used, right? So if I created the snapshot originally with
> 'set-time-on-resume', then it will always be applied upon rollback with
> no way to opt-out. And vice-versa if disabled. Should we add an option
> to the 'qm rollback' command to allow overriding this? Or
> unconditionally use the current value for 'set-time-on-resume' instead
> upon rollback? Not sure if that would be more in line with expectations
> or more surprising?
> 
> Not quite the same for resume from hibernation, but it will also be
> impossible to modify the setting if already suspended, because of the
> config lock.
> 
> And for a paused VM, it would be impossible to change because 'agent'
> property changes are not (yet) hot-plugged, but that might make sense to
> do actually. But not sure if that would be better done as part of a
> major release.
> 
> On the other hand, resume (from pause and suspend) could also gain
> support for an option to override the 'set-time-on-resume' setting.
> 
> Maybe the command options could default to the current guest config
> setting if not explicitly specified?
> 
> What do you think?
Exactly, the config from the snapshot will be used. I've discussed
this offlist with colleagues before sending the patch, since I also
disliked that there is no way to disable the time sync for a snapshot
once it was taken. We didn't come up with any real use cases where
someone would want this behavior and therefore I didn't look into this
further. I forgot that someone might want to enable it for older
snapshots, so the override would definitely make sense!

I do agree that it feels counterintuitive, I'd expect that if the
current config has sync time disabled, the rollback wouldn't use this
option. I'd opt for using the current guest config, this would feel
most intuitive to me, but am open to suggestions.
I'll gladly add it in a v3.
> 
>> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5032
>>
>> Signed-off-by: Jakob Klocker <j.klocker@proxmox.com>
>> Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
>> Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
>> ---
>>
>> changes since v1:
>> - adapt warning message in the resume-from-saved-state path
>>
>>  src/PVE/QemuConfig.pm       |  7 ++++++
>>  src/PVE/QemuServer.pm       | 10 ++++++++
>>  src/PVE/QemuServer/Agent.pm | 47 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 64 insertions(+)
>>
>> diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
>> index c24eb835..3f0ff663 100644
>> --- a/src/PVE/QemuConfig.pm
>> +++ b/src/PVE/QemuConfig.pm
>> @@ -383,6 +383,13 @@ sub __snapshot_create_vol_snapshots_hook {
>>                      next;
>>                  }
>>              }
>> +            if ($snap->{vmstate}) {
>> +                my $conf = $class->load_config($vmid);
>> +                if (PVE::QemuServer::Agent::should_set_time_on_resume($conf->{agent})) {
>> +                    eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 }
>> +                        or warn "could not sync guest time after snapshot - $@";
> 
> Style nit: our code base uses the following pattern:
> eval { func(); };
> warn "msg - $@" if $@;
I missed that, thanks.>
>> +                }
>> +            }
>>          }
>>      }
>>  }
>> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
>> index 55e9f520..daf10904 100644
>> --- a/src/PVE/QemuServer.pm
>> +++ b/src/PVE/QemuServer.pm
>> @@ -5992,6 +5992,16 @@ sub vm_start_nolock {
>>          );
>>      }
>>  
>> +    my $from_saved_state = $resume || ($statefile && !$migratedfrom);
> 
> Style nit: I'd kinda prefer to avoid this one-off variable. Maybe add a
> short comment near the expression instead?
> 
I'll add a comment and get rid of the variable.>> +
>> +    if (
>> +        $from_saved_state
>> +        && PVE::QemuServer::Agent::should_set_time_on_resume($conf->{agent})
>> +    ) {
>> +        eval { PVE::QemuServer::Agent::guest_set_time($vmid); 1 }
>> +            or warn "could not sync guest time after resume from saved state - $@";
> 
> Same style nit as above.
> 
>> +    }
>> +
>>      return $res;
>>  }
>>  
>> diff --git a/src/PVE/QemuServer/Agent.pm b/src/PVE/QemuServer/Agent.pm
>> index be6df443..be8bbae6 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 ();
> 
> Stlye nit: I'd use qw() for consistency with the previous line.
> 
>>  
>>  use PVE::JSONSchema;
>>  
>> @@ -18,6 +19,8 @@ our @EXPORT_OK = qw(
>>      get_qga_key
>>      parse_guest_agent
>>      qga_check_running
>> +    should_set_time_on_resume
>> +    guest_set_time
> 
> I don't think these are worth exporting. You already call them with the
> module prefix and I do think it's helpful to be explicit like that.
> 
Acknowledged
>>  );
>>  
>>  our $agent_fmt = {
>> @@ -52,6 +55,21 @@ our $agent_fmt = {
>>          optional => 1,
>>          default => 1,
>>      },
>> +    'set-time-on-resume' => {
> 
> I wonder if 'set' is the best? I'm thinking about 'update' or
> 'synchronize', but maybe those make it too long? 'sync'? Not sure. What
> do you think?
I'd opt for sync-time-on-resume - clearer than set.>
>> +        description => "Update the guest clock through QGA after resuming from"
>> +            . " hibernation or rolling back to a snapshot with RAM.",
>> +        verbose_description =>
>> +            "Whether to issue the guest-set-time QEMU guest agent command after the VM"
>> +            . " resumes with a restored RAM state, that is, when waking from hibernation"
>> +            . " or after rolling back to a snapshot that includes RAM. In these cases the"
>> +            . " guest's clock still reflects the time the state was saved. With this"
> 
> Nit: s/was saved/was saved at/
> 
> The description does not mention that it also happens after a snapshot
> create operation.
> 
>> +            . " option enabled, the clock is synchronized to the host's current time,"
>> +            . " provided the QEMU Guest Agent option is enabled in the guest's"
>> +            . " configuration and the agent is running inside of the guest.",
>> +        type => 'boolean',
>> +        optional => 1,
>> +        default => 1,
> 
> I don't think we should enable this by default, at least not outside of
> a major release. What if something relies on the current behavior and
> expects a rolled-back state to have the old time?
> 
I'll change the default to disabled, but would like this reconsidered
before a major release. A VM whose clock is always in sync feels more
intuitive to me.>> +    },
>>      type => {
>>          description => "Select the agent type",
>>          type => 'string',
>> @@ -332,4 +350,33 @@ sub guest_fs_freeze_applicable($agent_str, $vmid, $logfunc = undef) {
>>      return 1;
>>  }
>>  
>> +=head3 should_set_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. Does B<not> check whether the agent is actually running.
>> +
>> +=cut
>> +
>> +sub should_set_time_on_resume($agent_str) {
>> +    my $agent = parse_guest_agent($agent_str);
>> +    return 0 if !$agent->{enabled};
>> +    return $agent->{'set-time-on-resume'} // 1;
>> +}
>> +
>> +=head3 guest_set_time
>> +
>> +Sets the guest's clock via the QEMU Guest Agent's C<guest-set-time> command. If C<$time_ns>
>> +(nanoseconds since the UNIX epoch, UTC) is not given, the current host time is used. Passing
>> +an explicit time is required because the agent's argument-less form reads the guest's RTC,
>> +which may itself be stale after a vmstate snapshot or resume.
>> +
>> +=cut
>> +
>> +sub guest_set_time($vmid, $time_ns = undef) {
> 
> Not sure if adding the time parameter is worth it if it's not used. We
> can still add it later if a caller that needs it pops up.
> 
Will be changed in v3.>> +    $time_ns //= int(Time::HiRes::time() *
1_000_000_000);
>> +    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'guest-set-time', time => $time_ns);
> 
> Nit: I'd prefer the coercing to int() to be done here when passing the
> param to mon_cmd()
Acknowledged>
>> +    check_agent_error($res, "unable to set guest time");
>> +    return;
>> +}
>> +
>>  1;
> 





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of saved state
  2026-06-25  7:46     ` Jakob Klocker
@ 2026-06-25  8:10       ` Fiona Ebner
  0 siblings, 0 replies; 7+ messages in thread
From: Fiona Ebner @ 2026-06-25  8:10 UTC (permalink / raw)
  To: Jakob Klocker, pve-devel

Am 25.06.26 um 9:45 AM schrieb Jakob Klocker:
> On 6/24/26 1:31 PM, Fiona Ebner wrote:
>> Am 22.06.26 um 3:47 PM schrieb Jakob Klocker:
>>> When a VM is resumed from a saved state (hibernation or snapshot with
>>> RAM), the guest clock may be stale. A time skew can occur when
>>> creating a snapshot of a running VM.
>>>
>>> Add a new agent option `set-time-on-resume` to automatically
>>> synchronize the guest time with the host using the QEMU Guest Agent.
>>>
>>> Trigger time synchronization:
>>> - after restoring a VM from a saved state or snapshot with RAM
>>> - after taking a snapshot
>>>
>>> This ensures consistent guest time after rollback, restore, or
>>> snapshot operations when the guest OS clock does not automatically
>>> correct itself.
>>>
>>
>> It's not limited to snapshots with RAM. A snapshot operation without RAM
>> will also pause the guest. And I think it also makes sense to do it for
>> resume after a pause.
>>
> Thanks for pointing that out, I missed that. I'll make sure to sync
> the time for snapshots without RAM as well.
> 
> I will look into time sync for resume and add it in this series. I
> question whether we want one agent option for all the time syncs (on
> snapshot, rollback, pause/start) or make it more fine-grained. I'd
> lean toward keeping a single option.
>> For rollback, the agent option from the saved snapshot configuration
>> will be used, right? So if I created the snapshot originally with
>> 'set-time-on-resume', then it will always be applied upon rollback with
>> no way to opt-out. And vice-versa if disabled. Should we add an option
>> to the 'qm rollback' command to allow overriding this? Or
>> unconditionally use the current value for 'set-time-on-resume' instead
>> upon rollback? Not sure if that would be more in line with expectations
>> or more surprising?
>>
>> Not quite the same for resume from hibernation, but it will also be
>> impossible to modify the setting if already suspended, because of the
>> config lock.
>>
>> And for a paused VM, it would be impossible to change because 'agent'
>> property changes are not (yet) hot-plugged, but that might make sense to
>> do actually. But not sure if that would be better done as part of a
>> major release.
>>
>> On the other hand, resume (from pause and suspend) could also gain
>> support for an option to override the 'set-time-on-resume' setting.
>>
>> Maybe the command options could default to the current guest config
>> setting if not explicitly specified?
>>
>> What do you think?
> Exactly, the config from the snapshot will be used. I've discussed
> this offlist with colleagues before sending the patch, since I also
> disliked that there is no way to disable the time sync for a snapshot
> once it was taken. We didn't come up with any real use cases where
> someone would want this behavior and therefore I didn't look into this
> further. I forgot that someone might want to enable it for older
> snapshots, so the override would definitely make sense!
> 
> I do agree that it feels counterintuitive, I'd expect that if the
> current config has sync time disabled, the rollback wouldn't use this
> option. I'd opt for using the current guest config, this would feel
> most intuitive to me, but am open to suggestions.
> I'll gladly add it in a v3.

As discussed off-list today, we could add a new 'sync-time-on-resume'
option to the resume command (for resume from disk as well as from
paused) and the snapshot rollback command. The default value for these
would be the current configuration setting. And if the option is
explicitly set for the operation, the explicitly set value is used. This
should give users all the necessary flexibility and like that, it also
seems natural that the current configuration value serves as a fallback
(rather than the value in the snapshot config).

As for snapshot create, I would use the current configuration value to
decide whether the time should be synced after the operation and not add
a command option. If somebody has a use case where that would actually
be required, we could still add it later, but it's a bit hard to imagine
right now.

>>> +            . " option enabled, the clock is synchronized to the host's current time,"
>>> +            . " provided the QEMU Guest Agent option is enabled in the guest's"
>>> +            . " configuration and the agent is running inside of the guest.",
>>> +        type => 'boolean',
>>> +        optional => 1,
>>> +        default => 1,
>>
>> I don't think we should enable this by default, at least not outside of
>> a major release. What if something relies on the current behavior and
>> expects a rolled-back state to have the old time?
>>
> I'll change the default to disabled, but would like this reconsidered
> before a major release. A VM whose clock is always in sync feels more
> intuitive to me.>> +    },

Feel free to add a reminder comment that this should be considered with
the usual '# FIXME: MAJOR VERSION: xyz' comment template.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-25  8:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22 13:47 [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state Jakob Klocker
2026-06-22 13:47 ` [PATCH qemu-server v2 1/2] fix #5032: qemu: sync guest time on resume and snapshot of " Jakob Klocker
2026-06-24 11:31   ` Fiona Ebner
2026-06-25  7:46     ` Jakob Klocker
2026-06-25  8:10       ` Fiona Ebner
2026-06-22 13:47 ` [PATCH pve-manager v2 2/2] fix #5032: ui: qemu agent: add set-time-on-resume option Jakob Klocker
2026-06-23 14:10 ` [PATCH manager/qemu-server v2 0/2] fix #5032: sync guest time after restore from saved state Michael Köppl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal