Re: [PULL 00/17] Migration patches for 2024-12-17

[Peter Xu <peterx@redhat.com>]

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 <farosas@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 <farosas@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 <peterx@redhat.com>

-- 
Peter Xu