Re: [PATCH 09/33] qemu_migration.c: forbid powernv domains migration
On Mon, Jan 24, 2022 at 10:09:30AM -0300, Daniel Henrique Barboza wrote: > > > 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. Ok, lets drop this one and rely on QEMU reporting the lack of support. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
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 --- 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
Re: [PATCH 09/33] qemu_migration.c: forbid powernv domains migration
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. It is possible for devices to register migration blockers to prevent it, but libvirt shouldn't try to second guess that. 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. > > Signed-off-by: Daniel Henrique Barboza > --- > 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 -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 09/33] qemu_migration.c: forbid powernv domains migration
On Thu, Jan 20, 2022 at 10:52:12 -0300, Daniel Henrique Barboza wrote: > The PowerNV machine does not implement any form of migration. > > Signed-off-by: Daniel Henrique Barboza > --- > src/qemu/qemu_migration.c | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Peter Krempa
[PATCH 09/33] qemu_migration.c: forbid powernv domains migration
The PowerNV machine does not implement any form of migration. Signed-off-by: Daniel Henrique Barboza --- 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