block/qapi.c | 10 ++-- tests/qemu-iotests/267.out | 48 +++++++++---------- .../tests/qcow2-internal-snapshots.out | 10 ++-- 3 files changed, 35 insertions(+), 33 deletions(-)
When running the command `qemu-img snapshot -l SNAPSHOT` the output of
VM_CLOCK (measures the offset between host and VM clock) cannot to
accommodate values in the order of thousands (4-digit).
This line [1] hints on the problem. Additionally, the column width for
the VM_CLOCK field was reduced from 15 to 13 spaces in commit b39847a5
in line [2], resulting in a shortage of space.
[1]: https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L753
[2]: https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L763
This patch restores the column width to 15 spaces and makes adjustments
to the affected iotests accordingly. Furthermore, addresses a potential source
of confusion by removing whitespace in column headers. Example, VM CLOCK
is modified to VM_CLOCK. Additionally a '--' symbol is introduced when
ICOUNT returns no output for clarity.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2062
Fixes: b39847a50553 (migration: introduce icount field for snapshots )
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
---
v3:
* Make a patch by avoid changing the .patch file
v2:
* Change email provider to 'gmail' to avoid auto-wrapping patches
* Modify iotests for file 'qcow2-internal-snapshots.out'
block/qapi.c | 10 ++--
tests/qemu-iotests/267.out | 48 +++++++++----------
.../tests/qcow2-internal-snapshots.out | 10 ++--
3 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 9e806fa230..ee066ee53c 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -742,15 +742,15 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
char *sizing = NULL;
if (!sn) {
- qemu_printf("%-10s%-17s%8s%20s%13s%11s",
- "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
+ qemu_printf("%-10s%-17s%8s%20s%15s%11s",
+ "ID", "TAG", "VM_SIZE", "DATE", "VM_CLOCK", "ICOUNT");
} else {
g_autoptr(GDateTime) date = g_date_time_new_from_unix_local(sn->date_sec);
g_autofree char *date_buf = g_date_time_format(date, "%Y-%m-%d %H:%M:%S");
secs = sn->vm_clock_nsec / 1000000000;
snprintf(clock_buf, sizeof(clock_buf),
- "%02d:%02d:%02d.%03d",
+ "%04d:%02d:%02d.%03d",
(int)(secs / 3600),
(int)((secs / 60) % 60),
(int)(secs % 60),
@@ -759,8 +759,10 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
if (sn->icount != -1ULL) {
snprintf(icount_buf, sizeof(icount_buf),
"%"PRId64, sn->icount);
+ } else {
+ snprintf(icount_buf, sizeof(icount_buf), "--");
}
- qemu_printf("%-9s %-16s %8s%20s%13s%11s",
+ qemu_printf("%-9s %-16s %8s%20s%15s%11s",
sn->id_str, sn->name,
sizing,
date_buf,
diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
index 7176e376e1..21339e67ad 100644
--- a/tests/qemu-iotests/267.out
+++ b/tests/qemu-iotests/267.out
@@ -33,8 +33,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
@@ -44,8 +44,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
@@ -69,8 +69,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
@@ -94,8 +94,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
@@ -105,8 +105,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
@@ -119,8 +119,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
@@ -134,8 +134,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
@@ -145,15 +145,15 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
Internal snapshots on overlay:
Snapshot list:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
-1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
Internal snapshots on backing file:
=== -blockdev with NBD server on the backing file ===
@@ -166,17 +166,17 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm snap0
(qemu) info snapshots
List of snapshots present on all disks:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
--- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
(qemu) loadvm snap0
(qemu) quit
Internal snapshots on overlay:
Snapshot list:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
-1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
Internal snapshots on backing file:
Snapshot list:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
-1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
*** done
diff --git a/tests/qemu-iotests/tests/qcow2-internal-snapshots.out b/tests/qemu-iotests/tests/qcow2-internal-snapshots.out
index 438f535e6a..c324131561 100644
--- a/tests/qemu-iotests/tests/qcow2-internal-snapshots.out
+++ b/tests/qemu-iotests/tests/qcow2-internal-snapshots.out
@@ -14,8 +14,8 @@ wrote 524288/524288 bytes at offset 0
(qemu) quit
Snapshot list:
-ID TAG VM SIZE DATE VM CLOCK ICOUNT
-1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+ID TAG VM_SIZE DATE VM_CLOCK ICOUNT
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
No errors were found on the image.
=== Verify that loading the snapshot reverts to the old content ===
@@ -48,8 +48,8 @@ read 64512/64512 bytes at offset 66560
Snapshot list:
ID TAG VM SIZE DATE VM CLOCK ICOUNT
-1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
-2 snap1 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
+2 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
No errors were found on the image.
=== qemu-img snapshot can revert to snapshots ===
@@ -80,7 +80,7 @@ read 64512/64512 bytes at offset 66560
Snapshot list:
ID TAG VM SIZE DATE VM CLOCK ICOUNT
-1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 0000:00:00.000 --
No errors were found on the image.
=== Error cases ===
--
2.40.1Am 11.01.2024 um 18:25 hat Abhiram Tilak geschrieben: > When running the command `qemu-img snapshot -l SNAPSHOT` the output of > VM_CLOCK (measures the offset between host and VM clock) cannot to > accommodate values in the order of thousands (4-digit). > > This line [1] hints on the problem. Additionally, the column width for > the VM_CLOCK field was reduced from 15 to 13 spaces in commit b39847a5 > in line [2], resulting in a shortage of space. > > [1]: https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L753 > [2]: https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L763 > > This patch restores the column width to 15 spaces and makes adjustments > to the affected iotests accordingly. Furthermore, addresses a potential source > of confusion by removing whitespace in column headers. Example, VM CLOCK > is modified to VM_CLOCK. Additionally a '--' symbol is introduced when > ICOUNT returns no output for clarity. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2062 > Fixes: b39847a50553 (migration: introduce icount field for snapshots ) > Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> We can tweak the format a bit, but then we need to save those two characters somewhere else so that the output still fits in 80 characters. We can probably reduce the size of the ID column. Maybe what we should also do is decreasing the width of each field by one and instead writing a space character into the format string. So if we ever use up the space for one of the fields again, we would lose the nice column alignment, but you could still recognise the individual fields. Kevin
Alright, right now there are 10 digits for ID, don't think a billion snapshots are feasible anyway. > Maybe what we should also do is decreasing the width of each field by > one and instead writing a space character into the format string. I'm assuming you are talking about adding spaces between DATE, VM_CLOCK and ICOUNT, because spaces already exist for ID and TAGS. Will send a patch soon. Abhiram On Fri, 19 Jan 2024 at 01:13, Kevin Wolf <kwolf@redhat.com> wrote: > Am 11.01.2024 um 18:25 hat Abhiram Tilak geschrieben: > > When running the command `qemu-img snapshot -l SNAPSHOT` the output of > > VM_CLOCK (measures the offset between host and VM clock) cannot to > > accommodate values in the order of thousands (4-digit). > > > > This line [1] hints on the problem. Additionally, the column width for > > the VM_CLOCK field was reduced from 15 to 13 spaces in commit b39847a5 > > in line [2], resulting in a shortage of space. > > > > [1]: > https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L753 > > [2]: > https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L763 > > > > This patch restores the column width to 15 spaces and makes adjustments > > to the affected iotests accordingly. Furthermore, addresses a potential > source > > of confusion by removing whitespace in column headers. Example, VM CLOCK > > is modified to VM_CLOCK. Additionally a '--' symbol is introduced when > > ICOUNT returns no output for clarity. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2062 > > Fixes: b39847a50553 (migration: introduce icount field for snapshots ) > > Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> > > We can tweak the format a bit, but then we need to save those two > characters somewhere else so that the output still fits in 80 > characters. We can probably reduce the size of the ID column. > > Maybe what we should also do is decreasing the width of each field by > one and instead writing a space character into the format string. So if > we ever use up the space for one of the fields again, we would lose the > nice column alignment, but you could still recognise the individual > fields. > > Kevin > >
Am 22.01.2024 um 14:26 hat atp exp geschrieben: > Alright, right now there are 10 digits for ID, don't think a billion > snapshots are feasible anyway. Indeed. > > Maybe what we should also do is decreasing the width of each field by > > one and instead writing a space character into the format string. > > I'm assuming you are talking about adding spaces between DATE, VM_CLOCK > and ICOUNT, because spaces already exist for ID and TAGS. Yes, exactly. Kevin > On Fri, 19 Jan 2024 at 01:13, Kevin Wolf <kwolf@redhat.com> wrote: > > > Am 11.01.2024 um 18:25 hat Abhiram Tilak geschrieben: > > > When running the command `qemu-img snapshot -l SNAPSHOT` the output of > > > VM_CLOCK (measures the offset between host and VM clock) cannot to > > > accommodate values in the order of thousands (4-digit). > > > > > > This line [1] hints on the problem. Additionally, the column width for > > > the VM_CLOCK field was reduced from 15 to 13 spaces in commit b39847a5 > > > in line [2], resulting in a shortage of space. > > > > > > [1]: > > https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L753 > > > [2]: > > https://gitlab.com/qemu-project/qemu/-/blob/master/block/qapi.c?ref_type=heads#L763 > > > > > > This patch restores the column width to 15 spaces and makes adjustments > > > to the affected iotests accordingly. Furthermore, addresses a potential > > source > > > of confusion by removing whitespace in column headers. Example, VM CLOCK > > > is modified to VM_CLOCK. Additionally a '--' symbol is introduced when > > > ICOUNT returns no output for clarity. > > > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2062 > > > Fixes: b39847a50553 (migration: introduce icount field for snapshots ) > > > Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> > > > > We can tweak the format a bit, but then we need to save those two > > characters somewhere else so that the output still fits in 80 > > characters. We can probably reduce the size of the ID column. > > > > Maybe what we should also do is decreasing the width of each field by > > one and instead writing a space character into the format string. So if > > we ever use up the space for one of the fields again, we would lose the > > nice column alignment, but you could still recognise the individual > > fields. > > > > Kevin > > > >
© 2016 - 2024 Red Hat, Inc.