[libvirt] [PATCHv2 08/14] Fix vmdef usage while in monitor in qemu process

[John Ferlan]

<no commit message>?

Since it seems there's a second bugfix here (eg, the alias setting),
perhaps one should be added.


On 01/07/2015 10:42 AM, Ján Tomko wrote:
> ---
>  src/qemu/qemu_process.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4528b58..3d383ef 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2584,6 +2584,7 @@ qemuProcessInitPasswords(virConnectPtr conn,
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      size_t i;
> +    char *alias = NULL;
>  
>      for (i = 0; i < vm->def->ngraphics; ++i) {
>          virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
> @@ -2609,7 +2610,6 @@ qemuProcessInitPasswords(virConnectPtr conn,
>          for (i = 0; i < vm->def->ndisks; i++) {
>              char *secret;
>              size_t secretLen;
> -            const char *alias;
>  
>              if (!vm->def->disks[i]->src->encryption ||
>                  !virDomainDiskGetSource(vm->def->disks[i]))
> @@ -2620,20 +2620,26 @@ qemuProcessInitPasswords(virConnectPtr conn,
>                                                     &secret, &secretLen) < 0)
>                  goto cleanup;
>  
> -            alias = vm->def->disks[i]->info.alias;
> +            VIR_FREE(alias);
> +            if (VIR_STRDUP(alias, vm->def->disks[i]->info.alias) < 0)
> +                goto cleanup;

This will leak secret (my coverity checker found this)

>              if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) {
>                  VIR_FREE(secret);
>                  goto cleanup;
>              }
>              ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret);
>              VIR_FREE(secret);
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> +                ret = -1;
> +                goto cleanup;

superfluous considering the following statement

> +            }
>              if (ret < 0)
>                  goto cleanup;
>          }
>      }
>  
>   cleanup:
> +    VIR_FREE(alias);
>      virObjectUnref(cfg);
>      return ret;
>  }
> @@ -4181,6 +4187,7 @@ int qemuProcessStart(virConnectPtr conn,
>      virCommandPtr cmd = NULL;
>      struct qemuProcessHookData hookData;
>      unsigned long cur_balloon;
> +    unsigned int period = 0;
>      size_t i;
>      bool rawio_set = false;
>      char *nodeset = NULL;
> @@ -4793,15 +4800,18 @@ int qemuProcessStart(virConnectPtr conn,
>                         vm->def->mem.cur_balloon);
>          goto cleanup;
>      }
> +    if (vm->def->memballoon && vm->def->memballoon->period)
> +        period = vm->def->memballoon->period;
>      if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>          goto cleanup;
> -    if (vm->def->memballoon && vm->def->memballoon->period)
> -        qemuMonitorSetMemoryStatsPeriod(priv->mon, vm->def->memballoon->period);
> +    if (period)
> +        qemuMonitorSetMemoryStatsPeriod(priv->mon, period);
>      if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) {
>          qemuDomainObjExitMonitor(driver, vm);

Hmm no status check on this one?  Although fixed in 11/14 it seems

ACK with adjustments

John

>          goto cleanup;
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto cleanup;
>  
>      if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) {
>          VIR_DEBUG("Starting domain CPUs");
>