On Mon, Jan 06, 2025 at 04:24:53PM -0300, Fabiano Rosas wrote:
> Here's the fix for the pre-existing issue in the script:
For this patch:
>
> -- 8< --
> From 5bcad03aad85556a7b72f79d3574e246a99432c3 Mon Sep 17 00:00:00 2001
> From: Fabiano Rosas <faro...@suse.de>
> Date: Mon, 6 Jan 2025 15:05:31 -0300
> Subject: [PATCH 1/2] migration: Fix parsing of s390 stream
>
> The parsing for the S390StorageAttributes section is currently leaving
> an unconsumed token that is later interpreted by the generic code as
> QEMU_VM_EOF, cutting the parsing short.
Better mention why it can be intepreted as QEMU_VM_EOF, especially s390's
tag is 8 bytes while QEMU's EOF is 1 byte.. that confused me. :)
>
> The migration will issue a STATTR_FLAG_DONE between iterations, but
> there's a final STATTR_FLAG_EOS at .save_complete.
>
> Fixes: 81c2c9dd5d ("tests/qtest/migration-test: Fix analyze-migration.py for
> s390x")
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> scripts/analyze-migration.py | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> index f2457b1dde..2a2160cbf7 100755
> --- a/scripts/analyze-migration.py
> +++ b/scripts/analyze-migration.py
> @@ -65,6 +65,9 @@ def readvar(self, size = None):
> def tell(self):
> return self.file.tell()
>
> + def seek(self, a, b):
> + return self.file.seek(a, b)
> +
> # The VMSD description is at the end of the file, after EOF. Look for
> # the last NULL byte, then for the beginning brace of JSON.
> def read_migration_debug_json(self):
> @@ -272,11 +275,24 @@ def __init__(self, file, version_id, device,
> section_key):
> self.section_key = section_key
>
> def read(self):
> + pos = 0
> while True:
> addr_flags = self.file.read64()
> flags = addr_flags & 0xfff
> - if (flags & (self.STATTR_FLAG_DONE | self.STATTR_FLAG_EOS)):
> +
> + if flags & self.STATTR_FLAG_DONE:
> + pos = self.file.tell()
> + continue
> + elif flags & self.STATTR_FLAG_EOS:
> return
> + else:
> + # No EOS came after DONE, that's OK, but rewind the
> + # stream because this is not our data.
> + if pos:
> + self.file.seek(pos, 0)
Nit: use io.SEEK_SET.
> + return
> + raise Exception("Unknown flags %x", flags)
> +
> if (flags & self.STATTR_FLAG_ERROR):
> raise Exception("Error in migration stream")
> count = self.file.read64()
With above:
Reviewed-by: Peter Xu <pet...@redhat.com>
--
Peter Xu
