On 05/26/2011 03:45 AM, Federico Simoncelli wrote:
If this is correct I'll squash it with the v4 patch.
---
src/qemu/qemu_domain.h | 4 ++--
src/qemu/qemu_driver.c | 6 ++----
src/qemu/qemu_migration.c | 8 ++++----
3 files changed, 8 insertions(+), 10 deletions(-)
Almost!
@@ -5745,13 +5744,12 @@ static int
qemuDomainGetBlockInfo(virDomainPtr dom,
priv->jobSignalsData.infoDevName = disk->info.alias;
priv->jobSignalsData.blockInfo = info;
- priv->jobSignalsData.infoRetCode = -1;
+ priv->jobSignalsData.infoRetCode = &ret;
priv->jobSignals |= QEMU_JOB_SIGNAL_BLKINFO;
So far, so good - ret starts life as -1, and we are now off-loading the
responsibility for setting ret to any other value onto
qemuMigrationProcessJobSignals after a successful query.
+++ b/src/qemu/qemu_migration.c
@@ -659,8 +659,8 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver,
job, _("guest unexpectedly quit"));
if (cleanup) {
priv->jobSignals = 0;
- priv->jobSignalsData.statRetCode = -1;
- priv->jobSignalsData.infoRetCode = -1;
+ priv->jobSignalsData.statRetCode = NULL;
+ priv->jobSignalsData.infoRetCode = NULL;
But here, there's no need to set the pointers to NULL; by clearing
jobSignals, we guarantee that qemuDomainGetBlockInfo will eventually
wake up on the condition, at which point it will notice that ret is
still -1 because migration never set it. And since migration is
aborting, we will never again access priv->jobSignalsData.statRetCode to
try and dereference it, so this is a dead assignment.
So ACK with that nit fixed, and I've pushed the modified v4.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org