Re: [Qemu-devel] [PATCH 02/10] RFC: Implement qcow2's snapshot dependent saving function.

[Eric Blake <eblake@redhat.com>]

On 03/13/2018 03:33 AM, junyan.he@intel.com wrote:
> From: Junyan He <junyan.he@intel.com>
> 
> For qcow2 format, we can increase the cluster's reference count of
> dependent snapshot content and link the offset to the L2 table of
> the new snapshot point. This way can avoid obvious snapshot's dependent
> relationship, so when we delete some snapshot point, just decrease the
> cluster count and no need to check further.
> 
> Signed-off-by: Junyan He <junyan.he@intel.com>
> ---
>   block/qcow2-snapshot.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++
>   block/qcow2.c          |   2 +
>   block/qcow2.h          |   7 +++
>   3 files changed, 163 insertions(+)

It sounds like you are trying to modify the qcow2 spec to store more 
information into the internal snapshot table (and if you aren't, why 
not? If an internal snapshot depends on another one and we don't record 
that information in the qcow2 metadata, then we are pushing the burden 
of tracking inter-relationships onto management apps, and risk 
corruption if you load a snapshot without also visiting its 
dependencies).  It is absolutely essential that such modifications be 
reflected in docs/interop/qcow2.txt first, to make sure we agree on the 
implementation. What's more, there is already another parallel proposal 
that is also wanting to tweak qcow2 files:

https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg05231.html

We need to make sure both additions are coordinated.


> +
> +    if (!QEMU_IS_ALIGNED(depend_offset,  s->cluster_size)) {

Why two spaces here and elsewhere?


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org