Re: [PATCH v3 6/7] migration: Fix arrays of pointers in JSON writer

[Peter Xu]

On Thu, Jan 09, 2025 at 03:52:48PM -0300, Fabiano Rosas wrote:
> Currently, if an array of pointers contains a NULL pointer, that
> pointer will be encoded as '0' in the stream. Since the JSON writer
> doesn't define a "pointer" type, that '0' will now be an uint8, which
> is different from the original type being pointed to, e.g. struct.
> 
> (we're further calling uint8 "nullptr", but that's irrelevant to the
> issue)
> 
> That mixed-type array shouldn't be compressed, otherwise data is lost
> as the code currently makes the whole array have the type of the first
> element:
> 
> css = {NULL, NULL, ..., 0x5555568a7940, NULL};
> 
> {"name": "s390_css", "instance_id": 0, "vmsd_name": "s390_css",
>  "version": 1, "fields": [
>     ...,
>     {"name": "css", "array_len": 256, "type": "nullptr", "size": 1},
>     ...,
> ]}
> 
> In the above, the valid pointer at position 254 got lost among the
> compressed array of nullptr.
> 
> While we could disable the array compression when a NULL pointer is
> found, the JSON part of the stream still makes part of downtime, so we
> should avoid writing unecessary bytes to it.
> 
> Keep the array compression in place, but if NULL and non-NULL pointers
> are mixed break the array into several type-contiguous pieces :
> 
> css = {NULL, NULL, ..., 0x5555568a7940, NULL};
> 
> {"name": "s390_css", "instance_id": 0, "vmsd_name": "s390_css",
>  "version": 1, "fields": [
>      ...,
>      {"name": "css", "array_len": 254, "type": "nullptr", "size": 1},
>      {"name": "css", "type": "struct", "struct": {"vmsd_name": 
> "s390_css_img", ... }, "size": 768},
>      {"name": "css", "type": "nullptr", "size": 1},
>      ...,
> ]}
> 
> Now each type-discontiguous region will become a new JSON entry. The
> reader should interpret this as a concatenation of values, all part of
> the same field.
> 
> Parsing the JSON with analyze-script.py now shows the proper data
> being pointed to at the places where the pointer is valid and
> "nullptr" where there's NULL:
> 
> "s390_css (14)": {
>     ...
>     "css": [
>         "nullptr",
>         "nullptr",
>         ...
>         "nullptr",
>         {
>             "chpids": [
>             {
>                 "in_use": "0x00",
>                 "type": "0x00",
>                 "is_virtual": "0x00"
>             },
>             ...
>             ]
>         },
>         "nullptr",
>     }
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu