[libvirt] [PATCH] snapshot: improve qemu handling of reused snapshot targets

[Jiri Denemark]

On Tue, Mar 20, 2012 at 15:29:37 -0600, Eric Blake wrote:
> The oVirt developers have stated that the real reasons they want
> to have qemu reuse existing volumes when creating a snapshot are:
> 1. the management framework is set up so that creation has to be
> done from a central node for proper resource tracking, and having
> libvirt and/or qemu create things violates the framework, and
> 2. qemu defaults to creating snapshots with an absolute path to
> the backing file, but oVirt wants to manage a backing chain that
> uses just relative names, to allow for easier migration of a chain
> across storage locations.
> 
> When 0.9.10 added VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT (commit
> 4e9953a4), it only addressed point 1, but libvirt was still using
> O_TRUNC which violates point 2.  Meanwhile, the new qemu
> 'transaction' monitor command includes a new optional mode argument
> that will force qemu to reuse the metadata of the file it just
> opened (with the burden on the caller to have valid metadata there
> in the first place).  So, this tweaks the meaning of the flag to
> cover both points as intended for use by oVirt.  It is not strictly
> backward-compatible to 0.9.10 behavior, but it can be argued that
> the O_TRUNC of 0.9.10 was a bug.

I would agree. It makes more sense if REUSE reuses the whole file including
its content instead of reusing just the file name and some metadata.

> Note that this flag is all-or-nothing, and only selects between
> 'existing' and the default 'absolute-paths'.  A more flexible
> approach that would allow per-disk selections, as well as adding
> support for the 'no-backing-file' mode, would be possible by
> extending the <domainsnapshot> xml to have a per-disk mode, but
> until we have a management application expressing a need for that
> additional complexity, it is not worth doing.

Every opportunity to not increase the complexity of snapshot API even moreis
always welcome :-)

The patch looks like it's doing what you say it's doing, so ACK (with the
additional patch you plan to squash in).

Jirka