<no commit message>
On 01/07/2015 10:42 AM, Ján Tomko wrote:
> ---
> src/qemu/qemu_migration.c | 41 ++++++++++++++++++++++++++---------------
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 77e0b35..31494c8 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1663,7 +1663,8 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
> qemuDomainObjExitMonitor(driver, vm);
Missing the error check here (yes, it's in 11/14)
> goto cleanup;
> }
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
> }
>
> priv->nbdPort = port;
> @@ -1860,7 +1861,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
> }
>
>
> -static void
> +static int
> qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> qemuMigrationCookiePtr mig)
> @@ -1868,22 +1869,23 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
> qemuDomainObjPrivatePtr priv = vm->privateData;
>
> if (!mig->nbd)
> - return;
> + return 0;
>
> if (qemuDomainObjEnterMonitorAsync(driver, vm,
> QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
> - return;
> + return -1;
>
> if (qemuMonitorNBDServerStop(priv->mon) < 0)
> VIR_WARN("Unable to stop NBD server");
>
> - qemuDomainObjExitMonitor(driver, vm);
> -
Why not ret = qemuDomainObjExitMonitor(), then return ret later on...
Holding the lock while virPortAllocatorRelease executes seems unnecessary...
> virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort);
> priv->nbdPort = 0;
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + return -1;
> + return 0;
> }
>
> -static void
> +static int
> qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig,
> virQEMUDriverPtr driver,
> virDomainObjPtr vm)
> @@ -1891,6 +1893,7 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr
> mig,
> qemuDomainObjPrivatePtr priv = vm->privateData;
> size_t i;
> char *diskAlias = NULL;
> + int ret = 0;
>
> VIR_DEBUG("mig=%p nbdPort=%d", mig->nbd, priv->nbdPort);
>
> @@ -1914,12 +1917,15 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr
> mig,
> if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, NULL, 0,
> BLOCK_JOB_ABORT, true) < 0)
> VIR_WARN("Unable to stop block job on %s", diskAlias);
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> }
>
> cleanup:
> VIR_FREE(diskAlias);
> - return;
> + return ret;
> }
>
> /* Validate whether the domain is safe to migrate. If vm is NULL,
> @@ -2125,7 +2131,8 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
> state);
>
> cleanup:
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + ret = -1;
or ret = qemuDomainObjExitMonitor();
return ret;
(your choice here and the next few)
The rest seems OK... Might be nice to mention in a commit message why
failure to cancel drive mirror is ignorable while failure to stop nbd
server is not.
ACK, but it might be nice to pull in part of 11/14 since you're changing
ExitMonitor checks anyway
John
> return ret;
> }
>
> @@ -2164,7 +2171,8 @@ qemuMigrationSetAutoConverge(virQEMUDriverPtr driver,
> state);
>
> cleanup:
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + ret = -1;
> return ret;
> }
>
> @@ -2210,7 +2218,8 @@ qemuMigrationSetPinAll(virQEMUDriverPtr driver,
> state);
>
> cleanup:
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + ret = -1;
> return ret;
> }
>
> @@ -2476,7 +2485,8 @@ qemuDomainMigrateGraphicsRelocate(virQEMUDriverPtr
> driver,
> QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) {
> ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress,
> port, tlsPort, tlsSubject);
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + ret = -1;
> }
>
> cleanup:
> @@ -4000,7 +4010,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>
> /* cancel any outstanding NBD jobs */
> if (mig)
> - qemuMigrationCancelDriveMirror(mig, driver, vm);
> + ignore_value(qemuMigrationCancelDriveMirror(mig, driver, vm));
>
> if (spec->fwdType != MIGRATION_FWD_DIRECT) {
> if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
> @@ -5142,7 +5152,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
> VIR_WARN("unable to provide network data for
> relocation");
> }
>
> - qemuMigrationStopNBDServer(driver, vm, mig);
> + if (qemuMigrationStopNBDServer(driver, vm, mig) < 0)
> + goto endjob;
>
> if (flags & VIR_MIGRATE_PERSIST_DEST) {
> virDomainDefPtr vmdef;
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
