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

[Richard W.M. Jones]

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.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1095924
> 
> 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 at redhat.com>
> > Cc: Jiri Denemark <jdenemar at 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