Re: [libvirt] [PATCH 22/34] qemu: monitor: Remove weird return values from qemuMonitorSetCPU

Let the function report errors internally and change it to return standard return codes. --- src/qemu/qemu_driver.c | 22 ++++------------------ src/qemu/qemu_monitor_json.c | 4 ---- src/qemu/qemu_monitor_text.c | 22 +++++++++++----------- 3 files changed, 15 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 49fdd63..9011b2d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4698,7 +4698,6 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; - int rc = 1; int ret = -1; int oldvcpus = virDomainDefGetVCpus(vm->def); int vcpus = oldvcpus; @@ -4712,10 +4711,7 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, for (i = vcpus; i < nvcpus; i++) { /* Online new CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, true); - if (rc == 0) - goto unsupported; - if (rc < 0) + if (qemuMonitorSetCPU(priv->mon, i, true) < 0) goto exit_monitor; vcpus++; @@ -4795,14 +4791,11 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (virDomainObjIsActive(vm) && virDomainDefSetVCpus(vm->def, vcpus) < 0) ret = -1; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); + virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", ret == 0); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); return ret; - unsupported: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot change vcpu count of this domain")); exit_monitor: ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; @@ -4816,7 +4809,6 @@ qemuDomainHotunplugVcpus(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; - int rc = 1; int ret = -1; int oldvcpus = virDomainDefGetVCpus(vm->def); int vcpus = oldvcpus; @@ -4827,10 +4819,7 @@ qemuDomainHotunplugVcpus(virQEMUDriverPtr driver, for (i = vcpus - 1; i >= nvcpus; i--) { /* Offline old CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, false); - if (rc == 0) - goto unsupported; - if (rc < 0) + if (qemuMonitorSetCPU(priv->mon, i, false) < 0) goto exit_monitor; vcpus--; @@ -4889,12 +4878,9 @@ qemuDomainHotunplugVcpus(virQEMUDriverPtr driver, if (virDomainObjIsActive(vm) && virDomainDefSetVCpus(vm->def, vcpus) < 0) ret = -1; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); + virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", ret == 0); return ret; - unsupported: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot change vcpu count of this domain")); exit_monitor: ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 86b8c7b..50d6f62 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c

@@ -2188,10 +2188,6 @@ int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, else ret = qemuMonitorJSONCheckError(cmd, reply); - /* this function has non-standard return values, so adapt it */ - if (ret == 0) - ret = 1; - cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index f44da20..fd38d01 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1137,8 +1137,7 @@ qemuMonitorTextSetBalloon(qemuMonitorPtr mon, /* - * Returns: 0 if CPU hotplug not supported, +1 if CPU hotplug worked - * or -1 on failure + * Returns: 0 if CPU modification was successful or -1 on failure */

int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, bool online) { @@ -1149,22 +1148,23 @@ int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, bool online) if (virAsprintf(&cmd, "cpu_set %d %s", cpu, online ? "online" : "offline") < 0) return -1; - if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { - VIR_FREE(cmd); - return -1; - } - VIR_FREE(cmd); + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) + goto cleanup; /* If the command failed qemu prints: 'unknown command' * No message is printed on success it seems */ if (strstr(reply, "unknown command:")) { - /* Don't set error - it is expected CPU onlining fails on many qemu - caller will handle */ - ret = 0; - } else { - ret = 1; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot change vcpu count of this domain")); + goto cleanup; } + ret = 0; + + cleanup: VIR_FREE(reply); + VIR_FREE(cmd); + return ret; }