On 25/02/2025 03:58, Peter Xu wrote:
> On Fri, Feb 21, 2025 at 02:36:09PM +0800, Li Zhijian wrote:
>> Similar to migration_channels_and_transport_compatible(), introduce a
>> new helper migration_capabilities_and_transport_compatible() to check if
>> the capabilites is compatible with the transport.
>>
>> Currently, only move the capabilities vs RDMA transport to this
>> function.
>>
>> Signed-off-by: Li Zhijian <lizhij...@fujitsu.com>
>
> Yeah this is even better, thanks.
>
> Reviewed-by: Peter Xu <pet...@redhat.com>
Hi Peter,
Thinking one step further, this patch looks promising and can check
most situations. However, on the destination side, if the user first
specifies '-incoming' (with the startup parameter -incoming xxx or
migrate_incoming xxx) and then 'migrate_set_capability xxx on',
the function migration_capabilities_and_transport_compatible() will
not be called to check compatibility, which might lead to migration failure.
To address this, without introducing a new member 'transport' into the
MigrationState
structure, the code might need to be adjusted to this:
The question is whether we need to consider it now(in this patch set)?
static bool migration_transport_compatible(MigrationAddress *addr, Error
**errp)
{
return migration_channels_and_transport_compatible(addr, errp) &&
- migration_capabilities_and_transport_compatible(addr, errp);
+ migration_capabilities_and_transport_compatible(addr->transport,
+ migrate_get_current()->capabilities, errp);
}
static gint page_request_addr_cmp(gconstpointer ap, gconstpointer bp)
diff --git a/migration/options.c b/migration/options.c
index bb259d192a9..59f0ed5b528 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -439,6 +439,29 @@ static bool migrate_incoming_started(void)
return !!migration_incoming_get_current()->transport_data;
}
+bool
+migration_capabilities_and_transport_compatible(MigrationAddressType transport,
+ bool *new_caps,
+ Error **errp)
+{
+ if (transport == MIGRATION_ADDRESS_TYPE_RDMA) {
+ if (new_caps[MIGRATION_CAPABILITY_XBZRLE]) {
+ error_setg(errp, "RDMA and XBZRLE can't be used together");
+ return false;
+ }
+ if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
+ error_setg(errp, "RDMA and multifd can't be used together");
+ return false;
+ }
+ if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
+ error_setg(errp, "RDMA and postcopy-ram can't be used together");
+ return false;
+ }
+ }
+
+ return true;
+}
+
/**
* @migration_caps_check - check capability compatibility
*
@@ -602,6 +625,15 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps,
Error **errp)
}
}
+ /*
+ * In destination side, check the cases that capability is being set
+ * after incoming thread has started.
+ */
+ if (migrate_rdma() &&
+ !migration_capabilities_and_transport_compatible(
+ MIGRATION_ADDRESS_TYPE_RDMA, new_caps, errp)) {
+ return false;
+ }
return true;
}
diff --git a/migration/options.h b/migration/options.h
index 762be4e641a..ca6a40e7545 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -58,6 +58,9 @@ bool migrate_tls(void);
/* capabilities helpers */
bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp);
+bool
+migration_capabilities_and_transport_compatible(MigrationAddressType transport,
+ bool *new_caps, Error **errp);
>
