Re: [PATCH v1 0/4] Introduce active disk internal snapshot support
On Thu, Jan 12, 2023 at 02:44:35 -0600, Or Ozeri wrote:
Internal disk snapshots are currently only supported on non-active
VMs.
This patch series extends this support for active VMs running with qemu.
Could you elaborate how this will be useful? Generally disk only
snapshots are of very limited use as when reverting you get to a state
equating of a power loss of a real box.
This means potentially corrupted files, state and other drawbacks.
With external disk only snapshots this was at least somewhat useful as
it made the original image read-only and thus accessible with a
different process.
With internal snapshots you still can't even read that image as it's
still locked in write mode, so the utility value is even less.
This implementation for the qemu driver works even when there are
other
disks which ask for external snapshot.
Thus we remove the restriction disallowing mixing of internal and external
disk snapshots for active VMs.
Note that mixing is still disallowed for non-active VMs, as it require
a bit more work in a different area of the code.
Note that a few days ago Pavel pushed his series adding support for
deletion of external snapshots. That code heavily depends on the fact
that mixing internal/external snapshots was forbidden.
Thus the patch in this series will not be acceptable until you fix the
code that Pavel added.
Also note that Pavel is working on reversion of external snapshots so
that work might also colide.
Additionally we add a new attribute to allow the user specifying a
unique snapshot name for each internal disk snapshot.
In case this optional attribute is not specified, snapshot name
will be taken from the domain snapshot name, as it is currently done today.
This bit is potentially dangerous as based on your code you don't seem
to be checking that the name is unique across snapshots, which can cause
potential problems when names between snapshots collide.
This is also something that I feel requires elaboration of how it will
be useful.