[libvirt] [PATCH v2.1 11/11] qemu_migration: Stop NBD server at Finish phase

[Jiri Denemark]

On Fri, Jan 11, 2013 at 17:52:23 +0100, Michal Privoznik wrote:
> At the end of migration, it is important to stop NBD
> server and thus release all allocated resources.
> ---
>  src/qemu/qemu_migration.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 86eb4c8..a1372ed 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1501,6 +1501,32 @@ cleanup:
>      return ret;
>  }
>  
> +static void
> +qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
> +                           virDomainObjPtr vm,
> +                           qemuMigrationCookiePtr mig)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +    if (!mig->nbd)
> +        return;
> +
> +    if (mig->nbd->port != -1)
> +        VIR_WARN("This is strange. NBD port was not -1 "
> +                 "when shutting NDB server down");

It's so strange we should not even pass the -1 port to finish phase. It
serves no purpose. However, we should remember that nbd server was
running and stop it in that case; relying on source daemon to tell us
that nbd server was running looks quite fragile.

> +
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm,
> +                                       QEMU_ASYNC_JOB_MIGRATION_IN) < 0) {
> +        VIR_WARN("Unable to enter monitor");

The function already reports an error or logs a warning, this extra
warning is useless.

> +        return;
> +    }
> +
> +    if (qemuMonitorNBDServerStop(priv->mon) < 0)
> +        VIR_WARN("Unable to stop NBD server");

Shouldn't this be fatal to the migration process?

> +
> +    qemuDomainObjExitMonitorWithDriver(driver, vm);
> +}
> +
>  /* Validate whether the domain is safe to migrate.  If vm is NULL,
>   * then this is being run in the v2 Prepare stage on the destination
>   * (where we only have the target xml); if vm is provided, then this
> @@ -3891,6 +3917,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>                      VIR_WARN("unable to provide network data for relocation");
>          }
>  
> +        qemuMigrationStopNBDServer(driver, vm, mig);
> +
>          if (flags & VIR_MIGRATE_PERSIST_DEST) {
>              virDomainDefPtr vmdef;
>              if (vm->persistent)

BTW, Is the NBD server running as a thread within qemu? That is, when we
kill qemu process in case of failed or cancelled migration, we don't
need to explicitly stop the NBD server, do we?

Jirka