On Mon, Jul 18, 2022 at 10:25 AM Jiri Denemark <jdenemar@xxxxxxxxxx> 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!
