Re: [PATCH 09/33] qemu_migration.c: forbid powernv domains migration
On 1/21/22 11:33, Daniel P. Berrangé wrote:
On Thu, Jan 20, 2022 at 10:52:12AM -0300, Daniel Henrique Barboza
wrote:
> The PowerNV machine does not implement any form of migration.
What do you mean by that ?
Migration is a general feature in QEMU, not typically something
that a machine types opts in/out of.
What I meant with this patch is that migration, albeit possible, will not work.
These emulations doesn't implement vmstate() for their internal states or its
specific devices.
The reason is a more complicated version of "because no one bother to do it".
It's
technically doable of course (as you said, every QEMU machine is migratable per
default) but not something that we have in our long-term scope.
This patch had the intention to tell users to 'don't even bother migrating these
domains, it doesn't work'. We can live without it and let QEMU error out
normally.
It is possible for devices to register migration blockers to
prevent it, but libvirt shouldn't try to second guess that.
I agree that this kind of handling is something that belongs to QEMU and it's
best that Libvirt doesn't speculate about it.
Overall I'd like to see a clear justification for why libvirt
should enforce a policy here, as opposed to letting QEMU
accept or reject the migration.
I'm fine with dropping this patch.
Thanks,
Daniel
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
> ---
> src/qemu/qemu_migration.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 2635ef1162..dc2fe92e9b 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1332,6 +1332,12 @@ qemuMigrationSrcIsAllowed(virQEMUDriver *driver,
> int pauseReason;
> size_t i;
>
> + if (qemuDomainIsPowerNV(vm->def)) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("Powernv domains are not migratable"));
> + return false;
> + }
> +
> /* perform these checks only when migrating to remote hosts */
> if (remote) {
> nsnapshots = virDomainSnapshotObjListNum(vm->snapshots, NULL, 0);
> --
> 2.34.1
>
Regards,
Daniel