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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list