Re: [PATCH v2 4/7] migration: Rename vmstate_info_nullptr

[Peter Xu <peterx@redhat.com>]

On Thu, Jan 09, 2025 at 11:09:56AM -0300, Fabiano Rosas wrote:
> Rename vmstate_info_nullptr from "uint64_t" to "nullptr". This vmstate
> actually reads and writes just a byte, so the proper name would be
> uint8. However, since this is a marker for a NULL pointer, it's
> convenient to have a more explicit name that can be identified by the
> consumers of the JSON part of the stream.
> 
> Change the name to "nullptr" and add support for it in the
> analyze-migration.py script. Arbitrarily use the name of the type as
> the value of the field to avoid the script showing 0x30 or '0', which
> could be confusing for readers.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/vmstate-types.c    |  2 +-
>  scripts/analyze-migration.py | 22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index e83bfccb9e..d70d573dbd 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -338,7 +338,7 @@ static int put_nullptr(QEMUFile *f, void *pv, size_t size,
>  }
>  
>  const VMStateInfo vmstate_info_nullptr = {
> -    .name = "uint64",
> +    .name = "nullptr",
>      .get  = get_nullptr,
>      .put  = put_nullptr,
>  };
> diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
> index fcda11f31d..134c25f20a 100755
> --- a/scripts/analyze-migration.py
> +++ b/scripts/analyze-migration.py
> @@ -377,6 +377,8 @@ def read(self):
>  
>  
>  class VMSDFieldInt(VMSDFieldGeneric):
> +    NULL_PTR_MARKER = 0x30
> +
>      def __init__(self, desc, file):
>          super(VMSDFieldInt, self).__init__(desc, file)
>          self.size = int(desc['size'])
> @@ -385,6 +387,16 @@ def __init__(self, desc, file):
>          self.udtype = '>u%d' % self.size
>  
>      def __repr__(self):
> +
> +        # A NULL pointer is encoded in the stream as a '0' to
> +        # disambiguate from a mere 0x0 value and avoid consumers
> +        # trying to follow the NULL pointer. Displaying '0', 0x30 or
> +        # 0x0 when analyzing the JSON debug stream could become
> +        # confusing, so use an explicit term instead. The actual value
> +        # in the stream was already validated by VMSDFieldNull.
> +        if self.data == self.NULL_PTR_MARKER:
> +            return "nullptr"

What happens if a real int field has value 0x30 which is not a marker?
Would it be wrongly represented as "nullptr"?

> +
>          if self.data < 0:
>              return ('%s (%d)' % ((self.format % self.udata), self.data))
>          else:
> @@ -417,6 +429,15 @@ def __init__(self, desc, file):
>          super(VMSDFieldIntLE, self).__init__(desc, file)
>          self.dtype = '<i%d' % self.size
>  
> +class VMSDFieldNull(VMSDFieldUInt):
> +    def __init__(self, desc, file):
> +        super(VMSDFieldUInt, self).__init__(desc, file)
> +
> +    def read(self):
> +        super(VMSDFieldUInt, self).read()
> +        assert(self.data == self.NULL_PTR_MARKER)
> +        return self.data
> +
>  class VMSDFieldBool(VMSDFieldGeneric):
>      def __init__(self, desc, file):
>          super(VMSDFieldBool, self).__init__(desc, file)
> @@ -558,6 +579,7 @@ def getDict(self):
>      "bitmap" : VMSDFieldGeneric,
>      "struct" : VMSDFieldStruct,
>      "capability": VMSDFieldCap,
> +    "nullptr": VMSDFieldNull,
>      "unknown" : VMSDFieldGeneric,
>  }
>  
> -- 
> 2.35.3
> 

-- 
Peter Xu