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) &&
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list