Re: [PATCH 02/13] qcow2: introduce icount field for snapshots

[Pavel Dovgalyuk]

On 06.07.2020 23:17, Eric Blake wrote:
> On 6/26/20 5:19 AM, Pavel Dovgalyuk wrote:
> > From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> >
> > This patch introduces the icount field for saving within the snapshot.
> > It is required for navigation between the snapshots in record/replay 
> > mode.
> >
> > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> > Acked-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   0 files changed
>
> That's an odd diffstat; you may want to investigate why git isn't 
> showing the usual diffstat that makes it easier to see which files are 
> touched and the relative size of the changes.

Thanks for noticing, that was a stgit bug.

> > +++ b/docs/interop/qcow2.txt
> > @@ -645,6 +645,11 @@ Snapshot table entry:
> >                       Byte 48 - 55:   Virtual disk size of the 
> > snapshot in bytes
> > +                    Byte 56 - 63:   icount value which corresponds to
> > +                                    the record/replay instruction count
> > +                                    when the snapshot was taken. Set 
> > to -1
> > +                                    if icount was disabled
> > +
> >                       Version 3 images must include extra data at 
> > least up to
> >                       byte 55.
>
> Should we have additional text here, similar to what was added to the 
> overall header in 3ae3fcfa, about how to properly add additional 
> optional fields while maintaining back-compat considerations?  Maybe 
> just a one sentence reference that the rules in that section apply here 
> too?

Your proposal is adding more details about header extension in that patch?
But I didn't get what exactly is needed, because there is already
the following text in the beginning of "variable" part:

variable:   Extra data for future extensions. Unknown fields must be
            ignored. Currently defined are (offset relative to snapshot
            table entry):


Pavel Dovgalyuk