[libvirt] [PATCH v2 5/8] qemu: Don't report false errors in migration protocol v2

[John Ferlan]

On 09/11/2015 09:26 AM, Jiri Denemark wrote:
> Finish is the final state in v2 of our migration protocol. If something
> fails, we have no option to abort the migration and resume the original
> domain. Non fatal errors (such as failure to start guest CPUs or make
> the domain persistent) has to be treated as success. Keeping the domain
> running while reporting the failure was just asking for trouble.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
> 
> Notes:
>     Version 2:
>     - rebased on top of changed patch 1
> 
>  src/qemu/qemu_migration.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 

Well we didn't 'keep' that long ;-)  (need to look-ahead more often ;-))

Seems reasonable ACK

John
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 53444f7..35ab521 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -5624,7 +5624,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      unsigned short port;
> -    bool keep = false;
>  
>      VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
>                "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d",
> @@ -5700,17 +5699,14 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>                   * other hand, if we report failure, then the management tools
>                   * might try to restart the domain on the source side, even
>                   * though the domain is actually running on the destination.
> -                 * Return a NULL dom pointer, and hope that this is a rare
> -                 * situation and management tools are smart.
> -                 */
> -
> -                /*
> +                 * Pretend success and hope that this is a rare situation and
> +                 * management tools are smart.
> +                 *

Close your eyes and kiss your domain goodbye...

Too bad there wasn't a way to inhibit the mgmt tools from doing that
start if they weren't smart, but yet still fake the success


>                   * However, in v3 protocol, the source VM is still available
>                   * to restart during confirm() step, so we kill it off now.
>                   */
> -                if (!v3proto)
> -                    keep = true;
> -                goto endjob;
> +                if (v3proto)
> +                    goto endjob;
>              }
>          }
>  
> @@ -5738,9 +5734,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>                   * target in paused state, in case admin can fix
>                   * things up
>                   */
> -                if (!v3proto)
> -                    keep = true;
> -                goto endjob;
> +                if (v3proto)
> +                    goto endjob;
>              }
>              if (priv->job.completed) {
>                  qemuDomainJobInfoUpdateTime(priv->job.completed);
> @@ -5781,7 +5776,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>      }
>  
>   endjob:
> -    if (!dom && !keep &&
> +    if (!dom &&
>          !(flags & VIR_MIGRATE_OFFLINE) &&
>          virDomainObjIsActive(vm)) {
>          qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
>