Re: [RFC PATCH 3/4] hw/acpi: add VM generation counter field to VMClock

2025-12-01 Thread David Woodhouse
On Mon, 2025-12-01 at 16:01 +0100, Babis Chalios wrote:
> 
> That's a shame :(
> 
> Maybe we could do the same as VMGenID then. Make vm_generation_counter
> something that the user can set when creating the device (same as
> VMGenID's GUID). WDYT David?

Hm, can we store the current vmgenid in the vmlcock serialised state
and then bump the counter if it *changes*? Arguably we shouldn't be
advertising the VMCLOCK_FLAG_VM_GEN_COUNTER_PRESENT flag if there is no
vmgenid anyway, so we should *already* be looking at least for its
presence?

That way, when the external tooling launches the new QEMU for the
snapshot with the new vmgenid, we will spot it and bump the counter
accordingly?


smime.p7s
Description: S/MIME cryptographic signature


Re: [RFC PATCH 3/4] hw/acpi: add VM generation counter field to VMClock

2025-12-01 Thread Babis Chalios
On Mon, 2025-12-01 at 14:41 +, Daniel P. Berrangé wrote:
> On Mon, Dec 01, 2025 at 02:29:57PM +, David Woodhouse wrote:
> > On Mon, 2025-12-01 at 14:12 +, Daniel P. Berrangé wrote:
> > > From QEMU's POV, live migration and snapshots
> > > are indistiguishable operations, both using the same
> > > functionaility.
> > > 
> > > eg
> > >   $ qemu-system-x86_64 -monitor stdio -device vmclock
> > >   (qemu) migrate file:snapshot.img
> > > 
> > > and
> > > 
> > >   $ qemu-system-x86_64 -monitor stdio -device vmclock -incoming
> > > file:snapshot.img
> > > 
> > > 
> > > and we can't check the QEMU migration target being "file:" and
> > > mgmt
> > > apps can use the "fd:" protocol to pass in a pre-opened target
> > > which can
> > > be a socket or pipe or file.
> > 
> > What triggers the vmgenid to actually get updated for a snapshot?
> > That's the condition we're after, isn't it?
> 
> I don't quiet understand the sequences, but libvirt is involved in
> setting
> guid= as an arg to -device vmgenid when it spawns QEMU. This
> means
> libvirt has control over when it is changed or not.
> 

That's a shame :(

Maybe we could do the same as VMGenID then. Make vm_generation_counter
something that the user can set when creating the device (same as
VMGenID's GUID). WDYT David?

Cheers,
Babis





Re: [RFC PATCH 3/4] hw/acpi: add VM generation counter field to VMClock

2025-12-01 Thread Daniel P . Berrangé
On Mon, Dec 01, 2025 at 02:29:57PM +, David Woodhouse wrote:
> On Mon, 2025-12-01 at 14:12 +, Daniel P. Berrangé wrote:
> > From QEMU's POV, live migration and snapshots
> > are indistiguishable operations, both using the same functionaility.
> > 
> > eg
> >   $ qemu-system-x86_64 -monitor stdio -device vmclock
> >   (qemu) migrate file:snapshot.img
> > 
> > and
> > 
> >   $ qemu-system-x86_64 -monitor stdio -device vmclock -incoming 
> > file:snapshot.img
> > 
> > 
> > and we can't check the QEMU migration target being "file:" and mgmt
> > apps can use the "fd:" protocol to pass in a pre-opened target which can
> > be a socket or pipe or file.
> 
> What triggers the vmgenid to actually get updated for a snapshot?
> That's the condition we're after, isn't it?

I don't quiet understand the sequences, but libvirt is involved in setting
guid= as an arg to -device vmgenid when it spawns QEMU. This means
libvirt has control over when it is changed or not.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [RFC PATCH 3/4] hw/acpi: add VM generation counter field to VMClock

2025-12-01 Thread David Woodhouse
On Mon, 2025-12-01 at 14:12 +, Daniel P. Berrangé wrote:
> From QEMU's POV, live migration and snapshots
> are indistiguishable operations, both using the same functionaility.
> 
> eg
>   $ qemu-system-x86_64 -monitor stdio -device vmclock
>   (qemu) migrate file:snapshot.img
> 
> and
> 
>   $ qemu-system-x86_64 -monitor stdio -device vmclock -incoming 
> file:snapshot.img
> 
> 
> and we can't check the QEMU migration target being "file:" and mgmt
> apps can use the "fd:" protocol to pass in a pre-opened target which can
> be a socket or pipe or file.

What triggers the vmgenid to actually get updated for a snapshot?
That's the condition we're after, isn't it?


smime.p7s
Description: S/MIME cryptographic signature


Re: [RFC PATCH 3/4] hw/acpi: add VM generation counter field to VMClock

2025-12-01 Thread Daniel P . Berrangé
On Mon, Dec 01, 2025 at 12:51:10PM +, Chalios, Babis wrote:
> The final published version of the VMClock specification adds support
> for an extra vm_generation_counter field which allows hypervisors to
> differentiate between live migration and snapshot loading events. During
> the latter, apart from adjusting clocks, guests might want to take
> further actions such as resetting network devices, updating UUIDs,
> reseeding entropy pools, etc.
> 
> VM generation counter itself is stored in the guest memory region and
> exposed to guest userspace, so we don't need to serialize it within
> vmstate_vmclock as well.
> 
> Signed-off-by: Babis Chalios 
> ---
>  hw/acpi/vmclock.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/acpi/vmclock.c b/hw/acpi/vmclock.c
> index c582c0c1f8..47cbba4496 100644
> --- a/hw/acpi/vmclock.c
> +++ b/hw/acpi/vmclock.c
> @@ -20,6 +20,7 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/qdev-properties-system.h"
>  #include "migration/vmstate.h"
> +#include "migration/misc.h"
>  #include "system/reset.h"
>  
>  #include "standard-headers/linux/vmclock-abi.h"
> @@ -64,6 +65,7 @@ void vmclock_build_acpi(VmclockState *vms, GArray 
> *table_data,
>  static void vmclock_update_guest(VmclockState *vms)
>  {
>  uint64_t disruption_marker;
> +uint64_t vm_generation_counter;
>  uint32_t seq_count;
>  
>  if (!vms->clk) {
> @@ -79,6 +81,16 @@ static void vmclock_update_guest(VmclockState *vms)
>  disruption_marker++;
>  vms->clk->disruption_marker = cpu_to_le64(disruption_marker);
>  
> +/*
> + * We only increase the vm_generation_counter when loading from a 
> snapshot,
> + * not during live migration
> + */
> +if (!migration_is_running()) {
> +vm_generation_counter = le64_to_cpu(vms->clk->vm_generation_counter);
> +vm_generation_counter++;
> +vms->clk->vm_generation_counter = cpu_to_le64(vm_generation_counter);
> +}

I don't believe this conditional works. Run it with

  $ qemu-system-x86_64 -monitor stdio -device vmclock
  (qemu) migrate tcp:localhost:9000

and

  $ qemu-system-x86_64 -monitor stdio -device vmclock -incoming 
tcp:localhost:9000


and the vm_generation_counter always gets updated on every migrate
operation.

'migration_is_running()' is always returning 'false' when this callback
is triggered on the target.

Even if it were to return 'true' as this code expects, this would not
allow to distinguish between snapshots and live migration. The QEMU
"migrate" / "migrate-incoming" commands are used by mgmt apps to
implement snapshots. From QEMU's POV, live migration and snapshots
are indistiguishable operations, both using the same functionaility.

eg
  $ qemu-system-x86_64 -monitor stdio -device vmclock
  (qemu) migrate file:snapshot.img

and

  $ qemu-system-x86_64 -monitor stdio -device vmclock -incoming 
file:snapshot.img


and we can't check the QEMU migration target being "file:" and mgmt
apps can use the "fd:" protocol to pass in a pre-opened target which can
be a socket or pipe or file.

Only the mgmt app knows if this is for a snapshot or a live migration or
something else.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|