Re: [PATCH v3 07/16] qcow2: Write v3-compliant snapshot list on upgrade

[Eric Blake]

On 10/14/19 3:45 AM, Max Reitz wrote:

> > > +    need_snapshot_update = false;
> > > +    for (i = 0; i < s->nb_snapshots; i++) {
> > > +        if (s->snapshots[i].extra_data_size <
> > > +            sizeof_field(QCowSnapshotExtraData, vm_state_size_large) +
> > > +            sizeof_field(QCowSnapshotExtraData, disk_size))
> >
> > Shorter as:
> > if (s->snapshots[i].extra_data_size < sizeof(QCowSnapshotExtraData))
> >
> > but that's stylistic, so R-b still stands.
>
> Yes, but if we ever add fields to QCowSnapshotExtraData, we shouldn’t
> count them here.  Therefore, I think we need to count exactly the fields
> that the standard says are mandatory in v3.

If we ever add more fields, I'd prefer that we did something like:

struct QCowSnapshotExtraV3Minimum {
    uint64_t vm_state_size_large;
    uint64_t disk_size;
};
struct QCow3SnapshotExtraFull {
    struct QCowSnapshotExtraV3Minimum base;
    new fields...;
};

and use sane naming to get at extra members based on the expected types, 
rather than trying to piecemeal portions of a type based on size.

Until we actually DO add more fields, why do we have to complicate the 
current code?

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