Re: [PATCH v2 5/9] migration: check required subsections are loaded, once

[Juan Quintela]

marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Check that required subsections have been loaded.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

I will let other people to comment on this before merging.

I can see the (pontential problem) that Peter said: We still don't have
enough state.

But I can also see the problem that you are trying to fix: A needed
subsection didn't came.

> @@ -492,7 +521,7 @@ static int vmstate_subsection_load(QEMUFile *f, const 
> VMStateDescription *vmsd,
>              /* it doesn't have a valid subsection name */
>              return 0;
>          }
> -        sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
> +        sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr, visited);
>          if (sub_vmsd == NULL) {
>              trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(lookup)");
>              return -ENOENT;

I fully agree that a given subsection shouldn't be loaded more than once.
The part needed for this can get in at any point.



> @@ -509,6 +538,13 @@ static int vmstate_subsection_load(QEMUFile *f, const 
> VMStateDescription *vmsd,
>          }
>      }
>  
> +    for (i = 0; i < n; i++) {
> +        if (!visited[i] && vmstate_section_needed(vmsd->subsections[i], 
> opaque)) {
> +            trace_vmstate_subsection_load_bad(vmsd->name, 
> vmsd->subsections[i]->name, "(not visited)");
> +            return -ENOENT;
> +        }
> +    }
> +
>      trace_vmstate_subsection_load_good(vmsd->name);
>      return 0;
>  }

This part is the only one where I can see there could be some
discussion.  So I wil wait to see what other people think.

Later, Juan.