| Message ID | 20230425180544.1815888-2-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vmstate-static-checker: Fix VMS_ARRAY comparisons | expand |
Peter Xu <peterx@redhat.com> wrote: > For VMS_ARRAY typed vmsd fields, also dump the number of entries in the > array in -vmstate-dump. > > Without such information, vmstate static checker can report false negatives > of incompatible vmsd on VMS_ARRAY typed fields, when the src/dst do not > have the same type of array defined. It's because in the checker we only > check against size of fields within a VMSD field. > > One example: e1000e used to have a field defined as a boolean array with 5 > entries, then removed it and replaced it with UNUSED (in 31e3f318c8b535): > > - VMSTATE_BOOL_ARRAY(core.eitr_intr_pending, E1000EState, > - E1000E_MSIX_VEC_NUM), > + VMSTATE_UNUSED(E1000E_MSIX_VEC_NUM), > > It's a legal replacement but vmstate static checker is not happy with it, > because it checks only against the "size" field between the two > fields (here one is BOOL_ARRAY, the other is UNUSED): > > For BOOL_ARRAY: > > { > "field": "core.eitr_intr_pending", > "version_id": 0, > "field_exists": false, > "size": 1 > }, > > For UNUSED: > > { > "field": "unused", > "version_id": 0, > "field_exists": false, > "size": 5 > }, > > It's not the script to blame because there's just not enough information > dumped to show the total size of the entry for an array. Add it. > > Note that this will not break old vmstate checker because the field will > just be ignored. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> queued.
diff --git a/migration/savevm.c b/migration/savevm.c index 9671211339..550c38f0ef 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -536,6 +536,9 @@ static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field, field->version_id); fprintf(out_file, "%*s\"field_exists\": %s,\n", indent, "", field->field_exists ? "true" : "false"); + if (field->flags & VMS_ARRAY) { + fprintf(out_file, "%*s\"num\": %d,\n", indent, "", field->num); + } fprintf(out_file, "%*s\"size\": %zu", indent, "", field->size); if (field->vmsd != NULL) { fprintf(out_file, ",\n");
For VMS_ARRAY typed vmsd fields, also dump the number of entries in the array in -vmstate-dump. Without such information, vmstate static checker can report false negatives of incompatible vmsd on VMS_ARRAY typed fields, when the src/dst do not have the same type of array defined. It's because in the checker we only check against size of fields within a VMSD field. One example: e1000e used to have a field defined as a boolean array with 5 entries, then removed it and replaced it with UNUSED (in 31e3f318c8b535): - VMSTATE_BOOL_ARRAY(core.eitr_intr_pending, E1000EState, - E1000E_MSIX_VEC_NUM), + VMSTATE_UNUSED(E1000E_MSIX_VEC_NUM), It's a legal replacement but vmstate static checker is not happy with it, because it checks only against the "size" field between the two fields (here one is BOOL_ARRAY, the other is UNUSED): For BOOL_ARRAY: { "field": "core.eitr_intr_pending", "version_id": 0, "field_exists": false, "size": 1 }, For UNUSED: { "field": "unused", "version_id": 0, "field_exists": false, "size": 5 }, It's not the script to blame because there's just not enough information dumped to show the total size of the entry for an array. Add it. Note that this will not break old vmstate checker because the field will just be ignored. Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/savevm.c | 3 +++ 1 file changed, 3 insertions(+)