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

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> 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"?
>

Yes, better to not inherit from VMSDFieldInt then.

>> +
>>          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
>>