Re: [PATCH 1/2] vfio: Don't be a iterable and legacy device at the same time

[Peter Xu <peterx@redhat.com>]

On Thu, Jun 22, 2023 at 01:22:26PM +0200, Lukas Straub wrote:
> Legacy savevm devices only implement save_state() and load_state().
> Iterable devices shouldn't implement save_state() or else they are
> handled both as an iterable and legacy device in the savevm code.
> 
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
> 
> Note: this patch is completely untested.

PS: if you're not confident on the change will always work, better mark as
rfc to show a proposal of such change.

Comparing to the "legacy" vs "modern" migration, IIUC it was about whether
to use vmsd, so it's "save_state()" vs "vmsd" in that regard.

Personally, I don't immediately see a direct conflict / issue with device
providing both save_state() and save_setup().  It means the device declares
both (1) iterable data, and (2) non-iterable data (which can be either vmsd
or save_state()).

I do think vmsd is still preferred here for (2), e.g., I quickly looked at
vmstate_vfio_pci_config which seems fine to be implemented as a vmsd, with
a post_load() perhaps.  But that's another story.  It just all looks still
fine.

Do we get any benefit from having that restriction?

-- 
Peter Xu