[PATCH] qemu: don't call qemuMigrationSrcIsAllowedHostdev() from qemuMigrationDstPrepareFresh()

Laine Stump posted 1 patch 1 year, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20220727162823.277305-1-laine@redhat.com
src/qemu/qemu_migration.c | 3 ---
1 file changed, 3 deletions(-)
[PATCH] qemu: don't call qemuMigrationSrcIsAllowedHostdev() from qemuMigrationDstPrepareFresh()
Posted by Laine Stump 1 year, 9 months ago
This call to qemuMigrationSrcIsAllowedHostdev() (which does a
hardcoded fail of the migration if there is any PCI or mdev hostdev
device in the domain) while doing the destination side of migration
prep was found once the call to that same function was removed from
the source side migration prep (commit 25883cd5).

According to jdenemar, for the V2 migration protocol, prep of the
destination is the first step, so this *was* the proper place to do
the check, but for V3 migration this is in a way redundant (since we
will have already done the check on the source side (updated by
25883cd5 to query QEMU rather than do a hardcoded fail)).

Of course it's possible that the source could support migration of a
particular VFIO device, but the destination doesn't. But the current
check on the destination side is worthless even in that case, since it
is just *always* failing rather than querying QEMU; and QEMU can't be
queried at the point where the destination check is happening, since
it isn't yet running.

Anyway QEMU should complain when it's started if it's going to fail,
so removing this check should just move the failure to happen a bit
later. So the best solution to this problem is to simply remove the
hardcoded check/fail from qemuMigrationDstPrepareFresh() and rely on
QEMU to fail if it needs to.

Fixes: 25883cd5f0b188f2417f294b7d219a77b219f7c2
Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/qemu/qemu_migration.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 20dc91f1ce..85b6b12f92 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3379,9 +3379,6 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver,
                       QEMU_MIGRATION_COOKIE_CAPS;
     }
 
-    if (!qemuMigrationSrcIsAllowedHostdev(*def))
-        goto cleanup;
-
     /* Let migration hook filter domain XML */
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
         g_autofree char *xml = NULL;
-- 
2.35.3
Re: [PATCH] qemu: don't call qemuMigrationSrcIsAllowedHostdev() from qemuMigrationDstPrepareFresh()
Posted by Jiri Denemark 1 year, 9 months ago
On Wed, Jul 27, 2022 at 12:28:23 -0400, Laine Stump wrote:
> This call to qemuMigrationSrcIsAllowedHostdev() (which does a
> hardcoded fail of the migration if there is any PCI or mdev hostdev
> device in the domain) while doing the destination side of migration
> prep was found once the call to that same function was removed from
> the source side migration prep (commit 25883cd5).
> 
> According to jdenemar, for the V2 migration protocol, prep of the
> destination is the first step, so this *was* the proper place to do
> the check, but for V3 migration this is in a way redundant (since we
> will have already done the check on the source side (updated by
> 25883cd5 to query QEMU rather than do a hardcoded fail)).
> 
> Of course it's possible that the source could support migration of a
> particular VFIO device, but the destination doesn't. But the current
> check on the destination side is worthless even in that case, since it
> is just *always* failing rather than querying QEMU; and QEMU can't be
> queried at the point where the destination check is happening, since
> it isn't yet running.
> 
> Anyway QEMU should complain when it's started if it's going to fail,
> so removing this check should just move the failure to happen a bit
> later. So the best solution to this problem is to simply remove the
> hardcoded check/fail from qemuMigrationDstPrepareFresh() and rely on
> QEMU to fail if it needs to.
> 
> Fixes: 25883cd5f0b188f2417f294b7d219a77b219f7c2
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  src/qemu/qemu_migration.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 20dc91f1ce..85b6b12f92 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -3379,9 +3379,6 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver,
>                        QEMU_MIGRATION_COOKIE_CAPS;
>      }
>  
> -    if (!qemuMigrationSrcIsAllowedHostdev(*def))
> -        goto cleanup;
> -
>      /* Let migration hook filter domain XML */
>      if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
>          g_autofree char *xml = NULL;

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

And safe for freeze.