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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org