[libvirt] [PATCH] virsh: For virsh migrate --tunnelled, use the virDomainMigrateToURI3 API.

Using virsh migrate + the --tunnelled flag causes virsh to use the wrong API. This gives the error: error: use virDomainMigrateToURI3 for peer-to-peer migration As the error message is wrong, this patch also corrects the error message. https://bugzilla.redhat.com/show_bug.cgi?id=1095924 Signed-off-by: Richard W.M. Jones <rjones@redhat.com> Cc: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt.c | 2 +- tools/virsh-domain.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 2cd793c..6a361f6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5726,7 +5726,7 @@ virDomainMigrate3(virDomainPtr domain, if (flags & (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) { virReportInvalidArg(flags, "%s", _("use virDomainMigrateToURI3 for peer-to-peer " - "migration")); + "or tunnelled migration")); goto error; } diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3a7c260..5a5ce9d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8959,6 +8959,7 @@ doMigrate(void *opaque) flags |= VIR_MIGRATE_ABORT_ON_ERROR; if ((flags & VIR_MIGRATE_PEER2PEER) || + (flags & VIR_MIGRATE_TUNNELLED) || vshCommandOptBool(cmd, "direct")) { /* migrateuri doesn't make sense for tunnelled migration */ -- 1.9.0

On Thu, May 08, 2014 at 21:49:40 +0100, Richard W.M. Jones wrote:
Using virsh migrate + the --tunnelled flag causes virsh to use the wrong API. This gives the error:
error: use virDomainMigrateToURI3 for peer-to-peer migration
As the error message is wrong, this patch also corrects the error message.
Right, the error message is wrong but the suggested doesn't seem right either. The error you should see for virsh migrate --tunnelled is "cannot perform tunnelled migration without using peer2peer flag". Virsh should not try to guess that tunnelled implies peer2peer because there's a possibility someone implements tunnelled migration that transfers data through the client which controls migration and thus p2p flag would not be required and virDomainMigrateToURI3 would be a wrong API. Not that the APIs are designed for that but if I correctly remember discussions we had about this topic in the past, the goal was virsh should not be enforcing (or even automatically adding) p2p flag for tunnelled migrations for better compatibility with future.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com> Cc: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt.c | 2 +- tools/virsh-domain.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 2cd793c..6a361f6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5726,7 +5726,7 @@ virDomainMigrate3(virDomainPtr domain, if (flags & (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) { virReportInvalidArg(flags, "%s", _("use virDomainMigrateToURI3 for peer-to-peer " - "migration")); + "or tunnelled migration")); goto error; }
In other words, I think we actually want VIR_MIGRATE_TUNNELLED to be removed from the condition above and just add another condition similar to what we have in the other migration APIs: if (flags & VIR_MIGRATE_TUNNELLED) { virReportInvalidArg(flags, "%s", _("cannot perform tunnelled migration " "without using peer2peer flag")); goto error; } Jirka

On Thu, May 15, 2014 at 12:30:24PM +0200, Jiri Denemark wrote:
On Thu, May 08, 2014 at 21:49:40 +0100, Richard W.M. Jones wrote:
Using virsh migrate + the --tunnelled flag causes virsh to use the wrong API. This gives the error:
error: use virDomainMigrateToURI3 for peer-to-peer migration
As the error message is wrong, this patch also corrects the error message.
Right, the error message is wrong but the suggested doesn't seem right either. The error you should see for virsh migrate --tunnelled is "cannot perform tunnelled migration without using peer2peer flag". Virsh should not try to guess that tunnelled implies peer2peer because there's a possibility someone implements tunnelled migration that transfers data through the client which controls migration and thus p2p flag would not be required and virDomainMigrateToURI3 would be a wrong API. Not that the APIs are designed for that but if I correctly remember discussions we had about this topic in the past, the goal was virsh should not be enforcing (or even automatically adding) p2p flag for tunnelled migrations for better compatibility with future.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com> Cc: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt.c | 2 +- tools/virsh-domain.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 2cd793c..6a361f6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5726,7 +5726,7 @@ virDomainMigrate3(virDomainPtr domain, if (flags & (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) { virReportInvalidArg(flags, "%s", _("use virDomainMigrateToURI3 for peer-to-peer " - "migration")); + "or tunnelled migration")); goto error; }
In other words, I think we actually want VIR_MIGRATE_TUNNELLED to be removed from the condition above and just add another condition similar to what we have in the other migration APIs:
if (flags & VIR_MIGRATE_TUNNELLED) { virReportInvalidArg(flags, "%s", _("cannot perform tunnelled migration " "without using peer2peer flag")); goto error; }
I'll defer to whatever you want to do. When I originally submitted the patch, I believe that tunnelled !p2p migration was possible, based on this diagram: http://libvirt.org/migration.html#flowmanageddirect I think the diagram and error message are both misleading. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
participants (2)
-
Jiri Denemark
-
Richard W.M. Jones