| Message ID | 20250109140959.19464-5-farosas@suse.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Fix s390 regressions + migration script | expand |
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 <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 >>
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" + 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, }
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(-)