RE: [PATCH v2 3/6] qemu: Add internal support for active disk internal snapshots
-----Original Message-----
From: Peter Krempa <pkrempa(a)redhat.com>
Sent: Monday, 20 February 2023 16:00
To: Or Ozeri <ORO(a)il.ibm.com>
Cc: libvir-list(a)redhat.com; Danny Harnik <DANNYH(a)il.ibm.com>
Subject: [EXTERNAL] Re: [PATCH v2 3/6] qemu: Add internal support for
active disk internal snapshots
This modification is done to a function named
`qemuSnapshotCreateActiveExternal` but clearly enabled creation of internal
snapshots. You'll need to address that one too.
By address, you mean rename?
It treats both external and disks-only (which can be internal / external / mixed)
So maybe qemuSnapshotCreateActiveExternalOrDisksOnly?
I can't think of a neat name for it :\
Since your patches add intermingling (at least) partial of snapshots the code
will need to be split up separately.
1) check that the old-style full internal snapshot is requested and deal with
that as a special case
This check already exists, so I guess no change is required?
2) separate qemuSnapshotCreateActiveExternal into:
- 2.1) - the bit that creates the external memory snapshot
- 2.2) - the bit that creates the external disk snapshot
You mean separate each of the bits mentioned above to a new function?
The bit that creates the memory snapshot also pauses the VM, and unpause
after all is done (including the disks snapshot). I'm guessing you do not want
to include this as part of the 2.1 mentioned above?
- 2.3) - modify the bit that creates external disk snapshot to
also
do the internal disk snapshot for rbd
- 2.4) - call them in proper sequence. I suspect you also want to do
a external memory snapshot including the internal RBD
snapshot as s transaction.
Actually no, there is no current use-case for that (that I know of).
Good bit is that the oldschool internal snapshot is a completely separate case
and can't be intermingled here.
But the existing code must be modified to honour the change of semantics
you are introducing.