[libvirt] [PATCH 20/34] qemu: cpu hotplug: Fix error handling logic

[John Ferlan]

On 11/20/2015 10:22 AM, Peter Krempa wrote:
> The cpu hotplug helper functions used negative error handling in a part
> of them, although some code that was added later didn't properly set the
> error codes in some cases. This would cause improper error messages in
> cases where we couldn't modify the numa cpu mask and a few other cases.
> 
> Fix the logic by converting it to the regularly used pattern.
> ---
>  src/qemu/qemu_driver.c | 26 +++++++++-----------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 

ACK (could have removed a couple of open/close {} brackets [1]

One other "could do" thing since I peeked to the next patch -
qemuMonitorSetCPU could lift the comments from qemuMonitorJSONSetCPU or
qemuMonitorTextSetCPU...


John

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a483220..49fdd63 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4721,10 +4721,6 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
>          vcpus++;
>      }
> 
> -    /* hotplug succeeded */
> -
> -    ret = 0;
> -
>      /* After hotplugging the CPUs we need to re-detect threads corresponding
>       * to the virtual CPUs. Some older versions don't provide the thread ID
>       * or don't have the "info cpus" command (and they don't support multiple
> @@ -4732,12 +4728,12 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
>       * fatal */
>      if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) {
>          virResetLastError();
> +        ret = 0;
>          goto exit_monitor;
>      }
> -    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> -        ret = -1;
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>          goto cleanup;
> -    }
> 
>      if (ncpupids != vcpus) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -4745,7 +4741,6 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
>                           "got %d, wanted %d"),
>                         ncpupids, vcpus);
>          vcpus = oldvcpus;
> -        ret = -1;
>          goto cleanup;
>      }
> 
> @@ -4772,12 +4767,10 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
>              if (qemuDomainHotplugAddPin(vm->def->cpumask, i,
>                                          &vm->def->cputune.vcpupin,
>                                          &vm->def->cputune.nvcpupin) < 0) {
> -                ret = -1;
>                  goto cleanup;
>              }

[1] {} not necessary

>              if (qemuDomainHotplugPinThread(vm->def->cpumask, i, cpupids[i],
>                                             cgroup_vcpu) < 0) {
> -                ret = -1;
>                  goto cleanup;
>              }

[1] {} not necessary

>          }
> @@ -4794,6 +4787,8 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
>      priv->vcpupids = cpupids;
>      cpupids = NULL;
> 
> +    ret = 0;
> +
>   cleanup:
>      VIR_FREE(cpupids);
>      VIR_FREE(mem_mask);
> @@ -4841,8 +4836,6 @@ qemuDomainHotunplugVcpus(virQEMUDriverPtr driver,
>          vcpus--;
>      }
> 
> -    ret = 0;
> -
>      /* After hotplugging the CPUs we need to re-detect threads corresponding
>       * to the virtual CPUs. Some older versions don't provide the thread ID
>       * or don't have the "info cpus" command (and they don't support multiple
> @@ -4850,19 +4843,17 @@ qemuDomainHotunplugVcpus(virQEMUDriverPtr driver,
>       * fatal */
>      if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) {
>          virResetLastError();
> +        ret = 0;
>          goto exit_monitor;
>      }
> -    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> -        ret = -1;
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
>          goto cleanup;
> -    }
> 
>      /* check if hotunplug has failed */
>      if (ncpupids == oldvcpus) {
>          virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>                         _("qemu didn't unplug the vCPUs properly"));
>          vcpus = oldvcpus;
> -        ret = -1;
>          goto cleanup;
>      }
> 
> @@ -4872,7 +4863,6 @@ qemuDomainHotunplugVcpus(virQEMUDriverPtr driver,
>                           "got %d, wanted %d"),
>                         ncpupids, vcpus);
>          vcpus = oldvcpus;
> -        ret = -1;
>          goto cleanup;
>      }
> 
> @@ -4892,6 +4882,8 @@ qemuDomainHotunplugVcpus(virQEMUDriverPtr driver,
>      priv->vcpupids = cpupids;
>      cpupids = NULL;
> 
> +    ret = 0;
> +
>   cleanup:
>      VIR_FREE(cpupids);
>      if (virDomainObjIsActive(vm) &&
>