| Message ID | 159316679154.10508.16814264064541947914.stgit@pasha-ThinkPad-X280 |
|---|---|
| State | New |
| Headers | show |
| Series | Reverse debugging | expand |
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. > +++ 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?
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
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 2756b37d24..d14e7be1aa 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -164,6 +164,12 @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair, sn->disk_size = bs->total_sectors * BDRV_SECTOR_SIZE; } + if (sn->extra_data_size >= endof(QCowSnapshotExtraData, icount)) { + sn->icount = be64_to_cpu(extra.icount); + } else { + sn->icount = -1ULL; + } + if (sn->extra_data_size > sizeof(extra)) { uint64_t extra_data_end; size_t unknown_extra_data_size; @@ -333,6 +339,7 @@ int qcow2_write_snapshots(BlockDriverState *bs) memset(&extra, 0, sizeof(extra)); extra.vm_state_size_large = cpu_to_be64(sn->vm_state_size); extra.disk_size = cpu_to_be64(sn->disk_size); + extra.icount = cpu_to_be64(sn->icount); id_str_size = strlen(sn->id_str); name_size = strlen(sn->name); diff --git a/block/qcow2.h b/block/qcow2.h index 7ce2c23bdb..9c81086991 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -179,6 +179,7 @@ typedef struct QEMU_PACKED QCowSnapshotHeader { typedef struct QEMU_PACKED QCowSnapshotExtraData { uint64_t vm_state_size_large; uint64_t disk_size; + uint64_t icount; } QCowSnapshotExtraData; @@ -192,6 +193,8 @@ typedef struct QCowSnapshot { uint32_t date_sec; uint32_t date_nsec; uint64_t vm_clock_nsec; + /* icount value for the moment when snapshot was taken */ + uint64_t icount; /* Size of all extra data, including QCowSnapshotExtraData if available */ uint32_t extra_data_size; /* Data beyond QCowSnapshotExtraData, if any */ diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt index cb723463f2..06014c0e90 100644 --- a/docs/interop/qcow2.txt +++ 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.