On 29/08/2023 17:53, Peter Xu wrote:
External email: Use caution opening links or attachments On Mon, Aug 28, 2023 at 06:18:41PM +0300, Avihai Horon wrote:diff --git a/migration/options.c b/migration/options.c index 1d1e1321b0..e201053563 100644 --- a/migration/options.c +++ b/migration/options.c @@ -499,6 +499,11 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp) error_setg(errp, "Postcopy is not yet compatible with multifd"); return false; } + + if (migration_vfio_mig_active()) { + error_setg(errp, "Postcopy is not compatible with VFIO migration"); + return false; + }Hmm.. this will add yet another vfio hard-coded line into migration/.. What will happen if the vfio device is hot plugged after enabling postcopy-ram here?
In that case a migration blocker will be added.
Is it possible to do it in a generic way?
What comes to my mind is to let devices register a handler for a "caps change" notification and allow them to object.
But maybe that's a bit of an overkill.
I was thinking the only unified place to do such check is when migration starts, as long as we switch to SETUP all caps are locked and doesn't allow any change until it finishes or fails. So, can we do this check inside vfio_save_setup(), allow vfio_save_setup() to fail the whole migration early? For example, maybe we should have an Error** passed in, then if it fails it calls migrate_set_error, so reflected in query-migrate later too.
Yes, I think this could work and it will simplify things because we could also drop the VFIO migration blockers code. The downside is that the user will know migration is blocked only when he tries to migrate, and migrate_caps_check() will not block setting postcopy when a VFIO device is already attached. I don't have a strong opinion here, so if it's fine by you and everyone else, I could change that to what you suggested.
Thanks.
