[RFC,07/17] qcow2: introduce icount field for snapshots

Commit Message

Pavel Dovgalyuk April 25, 2018, 12:46 p.m. UTC

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 <[e-mail]>
---
 block/qcow2-snapshot.c |    9 +++++++++
 block/qcow2.h          |    2 ++
 2 files changed, 11 insertions(+)

Comments

Eric Blake April 25, 2018, 7:01 p.m. UTC | #1

On 04/25/2018 07:46 AM, Pavel Dovgalyuk wrote:
> 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 <[e-mail]>
> ---
>  block/qcow2-snapshot.c |    9 +++++++++
>  block/qcow2.h          |    2 ++
>  2 files changed, 11 insertions(+)

Missing a change to docs/interop/qcow2.txt.  You cannot add new features
into the qcow2 internal snapshot metadata without first documenting them
as part of the qcow2 spec.

> 
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 74293be..4f2357e 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -103,6 +103,12 @@ int qcow2_read_snapshots(BlockDriverState *bs)
>              sn->disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
>          }
>  
> +        if (extra_data_size >= 24) {
> +            sn->icount = be64_to_cpu(extra.icount);

That's VERY dangerous without a spec change.  If someone else adds extra
data in some other format, then you will misinterpret their extra
information as your icount information.

Patch

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 74293be..4f2357e 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -103,6 +103,12 @@  int qcow2_read_snapshots(BlockDriverState *bs)
             sn->disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
         }
 
+        if (extra_data_size >= 24) {
+            sn->icount = be64_to_cpu(extra.icount);
+        } else {
+            sn->icount = 0;
+        }
+
         /* Read snapshot ID */
         sn->id_str = g_malloc(id_str_size + 1);
         ret = bdrv_pread(bs->file, offset, sn->id_str, id_str_size);
@@ -209,6 +215,7 @@  static 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);
@@ -372,6 +379,7 @@  int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
     sn->date_sec = sn_info->date_sec;
     sn->date_nsec = sn_info->date_nsec;
     sn->vm_clock_nsec = sn_info->vm_clock_nsec;
+    sn->icount = sn_info->icount;
 
     /* Allocate the L1 table of the snapshot and copy the current one there. */
     l1_table_offset = qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
@@ -691,6 +699,7 @@  int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
         sn_info->date_sec = sn->date_sec;
         sn_info->date_nsec = sn->date_nsec;
         sn_info->vm_clock_nsec = sn->vm_clock_nsec;
+        sn_info->icount = sn->icount;
     }
     *psn_tab = sn_tab;
     return s->nb_snapshots;
diff --git a/block/qcow2.h b/block/qcow2.h
index adf5c39..8880937 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -151,6 +151,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;
 
 
@@ -164,6 +165,7 @@  typedef struct QCowSnapshot {
     uint32_t date_sec;
     uint32_t date_nsec;
     uint64_t vm_clock_nsec;
+    uint64_t icount;
 } QCowSnapshot;
 
 struct Qcow2Cache;