Re: [libvirt] [PATCH 1/2] qemu: avoid dead store in doPeer2PeerMigrate3
On 08/16/2011 04:02 AM, Osier Yang wrote:
> +++ b/src/qemu/qemu_migration.c
> @@ -2050,6 +2050,9 @@ finish:
> * do, but fortunately worst case is that there is a
> * domain left in 'paused' state on source.
> */
> + if (ret< 0)
> + VIR_WARN("Guest %s probably left in 'paused' state on source",
> + vm->def->name);
Error is already reported by qemuMigrationConfirm if it fails
on resuming the guest on source host. Think it's not worth
to introduce a warning just to avoid the clang warning.
The issue at hand here is that the overall migration is returning status
0 (success), even if confirm failed. Are we sure that the
qemuReportError in qemuMigrationConfirm always shows up in the logs? We
don't need a double log entry; and if the confirm failure is already
being logged, then the appropriate fix here is to just nuke the dead
assignment (that is, call the confirm command without capturing its
status into ret). But if the qemuReportError is not showing up in the
log because the overall migration command is succeeding, then adding a
VIR_WARN here to guarantee a log message is appropriate.
I haven't researched the code well enough to know which solution is
needed, but it should be the same solution for both libvirt.c and
qemu_migration.c, since both files reported the same dead store under clang.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org