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