[libvirt] [PATCH] Do not try to cancel non-existent migration on source

[Jiri Denemark]

On Tue, Aug 16, 2011 at 08:33:04 -0600, Eric Blake wrote:
> On 08/16/2011 04:44 AM, Jiri Denemark wrote:
> > If migration failed on source daemon, the migration is automatically
> > canceled by the daemon itself. Thus we don't need to call
> > virDomainMigrateConfirm3(cancelled=1). Calling it doesn't cause any harm
> > but the resulting error message printed in logs may confuse people.
> > ---
> >   src/libvirt.c |   41 +++++++++++++++++++++++++----------------
> >   1 files changed, 25 insertions(+), 16 deletions(-)
> 
> Shouldn't qemu_migration.c get the same fix, since peer2peer and
> tunneled migration basically re-implement the same migration driver
> function?

It's reimplementing the same thing but in a different context. The code in
libvirt.c runs on a client, while qemu_migration.c runs within the qemu driver
in libvirtd. The virDomainMigratePrform3 API called from libvirt.c cleans up
after itself; it resumes a domain and finishes the migration job in case of
error. On the other hand, everything in doPeer2PeerMigrate3 is run within a
single public API and the function called to do the Perform phase is an
internal function which doesn't clean up after itself in the same way as the
public API has to. Hence, we need to call qemuMigrationConfirm to actually
resume the domain even if the Perform phase failed.

> 
> > +    if (notify_source) {
> > +        VIR_DEBUG("Confirm3 %p ret=%d domain=%p", domain->conn, ret, domain);
> > +        VIR_FREE(cookiein);
> > +        cookiein = cookieout;
> > +        cookieinlen = cookieoutlen;
> > +        cookieout = NULL;
> > +        cookieoutlen = 0;
> > +        ret = domain->conn->driver->domainMigrateConfirm3
> > +            (domain, cookiein, cookieinlen,
> > +             flags | protection, cancelled);
> 
> And this brings us right back to Alex's question about ret being a dead 
> store - do we need to add a VIR_WARN here based on ret, or just drop the 
> assignment?

AFAIK we do not log reported errors until they are dispatched to clients so
the log doesn't contain any error from domainMigrateConfirm3. So IMHO
VIR_WARNing about the failure seems like the way to go.

Jirka