Re: [libvirt PATCH] qemu_migration: Delete vDPA check
On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark <jdenemar(a)redhat.com> wrote:
On Fri, Jul 15, 2022 at 11:14:58 +0200, Eugenio Pérez wrote:
> Currently, the migration code denies migration as long as the VM has a
> vhost-vDPA device.
>
> While it's true that vhost-vDPA devices cannot be migrated at the moment, there
are plans to be able to migrate them soon.
>
> Libvirt must treat it equal to vhost-kernel devices: Not all of them can
> be migrated (the ones that do not expose the feature VHOST_F_LOG_ALL
> cannot be migrated). So checks like this one should work for all vhost
> devices equally.
>
> A more accurate solution is to ask qemu if it has an active migration
> blocker at that moment. Hoever, that require synchronization to avoid
> qemu to add one between the query and the actual migration command.
What would cause QEMU to add such a blocker? I believe it wouldn't do it
by itself and I hope even guest action would not cause migration blocker
to be added.
Qemu already adds a blocker in those cases. For example, in
hw/virtio/vhost.c:vhost_dev_init, you can find this conditional:
--
if (hdev->migration_blocker == NULL) {
if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
error_setg(&hdev->migration_blocker,
"Migration disabled: vhost lacks
VHOST_F_LOG_ALL feature.");
} else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_alloc_check()) {
error_setg(&hdev->migration_blocker,
"Migration disabled: failed to allocate shared memory");
}
}
if (hdev->migration_blocker != NULL) {
r = migrate_add_blocker(hdev->migration_blocker, errp);
...
}
--
So "migrate" command fails at monitor with that error message
Which in ideal case would mean only a QMP command (such as
hotplugging a non-migratable device) is the only way to add migration
blocker. If this is true, than we're safe as libvirt does not allow such
commands between qemuMigrationSrcIsAllowed and migration start.
Ok, that rules out a few bad use cases. I can do a fast lookup to
check if blockers can be added without the knowledge of libvirt.
That said, is there a reason for not implementing the correct
solution
right away as a separate patch?
I was not sure if libvirt already had another way to check, for
example, if the vhost device didn't have VHOST_F_LOG_ALL feature. If
not, I think checking for blockers it's the right thing to do.
Thanks!