On Tue, Jan 07, 2025 at 04:50:21PM -0300, Fabiano Rosas wrote:
> The migration stream lacks magic numbers at some key points. It's easy
> to mis-parse data. Unfortunately, the VMS_NULLPTR_MARKER continues
> with the trend. A '0' byte is ambiguous and could be interpreted as a
> valid 0x30.
>
> It is maybe not worth trying to change this while keeping backward
> compatibility, so add some words of documentation to clarify.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> migration/vmstate-types.c | 6 ++++++
> scripts/analyze-migration.py | 9 +++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index e83bfccb9e..08ed059f87 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -339,6 +339,12 @@ static int put_nullptr(QEMUFile *f, void *pv, size_t
> size,
>
> const VMStateInfo vmstate_info_nullptr = {
> .name = "uint64",
Ouch.. So I overlooked this line and this explains why it didn't go via
VMSDFieldGeneric already.
Instead of below comment, do we still have chance to change this to
something like "uint8"? Then I suppose the script will be able to identify
this properly.
> +
> + /*
> + * Ideally these would actually read/write the size of a pointer,
> + * but we're stuck with just a byte now for backward
> + * compatibility.
> + */
> .get = get_nullptr,
> .put = put_nullptr,
> };
> diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> index f2457b1dde..4292fde424 100755
> --- a/scripts/analyze-migration.py
> +++ b/scripts/analyze-migration.py
> @@ -388,12 +388,21 @@ def read(self):
> return self.data
>
> class VMSDFieldUInt(VMSDFieldInt):
> + NULL_PTR_MARKER = 0x30
> +
> def __init__(self, desc, file):
> super(VMSDFieldUInt, self).__init__(desc, file)
>
> def read(self):
> super(VMSDFieldUInt, self).read()
> self.data = self.udata
> +
> + if self.data == self.NULL_PTR_MARKER:
> + # The migration stream encodes NULL pointers as '0' so any
> + # 0x30 in the stream could be a NULL. There's not much we
> + # can do without breaking backward compatibility.
> + pass
So this change doesn't do anything, right?
It'll be weird here having it "uint64" but the super().read() will actually
only read 1 byte.. I assume the oneliner change of s/uint64/uint8/ could
be a replacement of this patch, and I hope that'll work too for the script.
So we will still see a bunch of 0x30s but I assume it's ok.
> +
> return self.data
>
> class VMSDFieldIntLE(VMSDFieldInt):
> --
> 2.35.3
>
--
Peter Xu
