[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv5] qemu: allow blkstat/blkinfo calls during migration



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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]