On 08/03/2016 04:11 AM, Peter Krempa wrote:
> Call the vcpu thread info validation separately to decrease complexity
> of returned values by qemuDomainRefreshVcpuInfo.
>
> This function now returns 0 on success and -1 on error. Certain
> failures of qemu to report data are still considered as success. Any
> error reported now is fatal.
> ---
> src/qemu/qemu_domain.c | 16 +++++-----------
> src/qemu/qemu_driver.c | 20 ++++++++++----------
> src/qemu/qemu_process.c | 6 ++++++
> 3 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f27f2f7..4cdd012 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5671,10 +5671,10 @@ qemuDomainValidateVcpuInfo(virDomainObjPtr vm)
> * @vm: domain object
> * @asyncJob: current asynchronous job type
> *
> - * Updates vCPU information private data of @vm.
> + * Updates vCPU information private data of @vm. Due to historical reasons this
> + * function returns success even if some data were not reported by qemu.
> *
> - * Returns number of detected vCPU threads on success, -1 on error and reports
> - * an appropriate error, -2 if the domain doesn't exist any more.
> + * Returns 0 on success and -1 on fatal error.
> */
> int
> qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
> @@ -5722,10 +5722,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
> if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> return -1;
> ncpupids = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &cpupids);
> - if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> - ret = -2;
> - goto cleanup;
> - }
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
This is missing a "goto cleanup" which is added in the next patch, but
needs to be here!
>
> /* failure to get the VCPU <-> PID mapping or to execute the query
> * command will not be treated fatal as some versions of qemu don't
> @@ -5745,10 +5742,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
> QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid = 0;
> }
>
> - if (qemuDomainValidateVcpuInfo(vm) < 0)
> - goto cleanup;
> -
> - ret = ncpupids;
> + ret = 0;
>
> cleanup:
> VIR_FREE(cpupids);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 453572b..2199258 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4615,6 +4615,7 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver,
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> virDomainVcpuDefPtr vcpuinfo = virDomainDefGetVcpu(vm->def, vcpu);
> + qemuDomainVcpuPrivatePtr vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpuinfo);
> int ret = -1;
> int rc;
> int oldvcpus = virDomainDefGetVcpus(vm->def);
> @@ -4639,15 +4640,14 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver,
>
> vcpuinfo->online = true;
>
> - if ((rc = qemuDomainRefreshVcpuInfo(driver, vm, QEMU_ASYNC_JOB_NONE)) <= 0) {
> - /* vcpu pids were not detected, skip setting of affinity */
> - if (rc == 0)
> - ret = 0;
> + if (qemuDomainRefreshVcpuInfo(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> + goto cleanup;
>
> + if (qemuDomainValidateVcpuInfo(vm) < 0)
> goto cleanup;
> - }
>
> - if (qemuProcessSetupVcpu(vm, vcpu) < 0)
> + if (vcpupriv->tid > 0 &&
> + qemuProcessSetupVcpu(vm, vcpu) < 0)
> goto cleanup;
>
> ret = 0;
> @@ -4689,11 +4689,11 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver,
> goto cleanup;
> }
>
> - if ((rc = qemuDomainRefreshVcpuInfo(driver, vm, QEMU_ASYNC_JOB_NONE)) < 0) {
> - /* rollback only if domain didn't exit */
> - if (rc == -2)
> - goto cleanup;
> + if (qemuDomainRefreshVcpuInfo(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> + goto cleanup;
Typing what I'm thinking - just to be sure it matches your expectations
of this code. So we no longer care to distinguish between failure and
failure due to domain exit. This just seems to mean we'll get that audit
after a possible domain exit and reset the vcpuinfo->online (won't
really matter if the domain exited).
ACK - with the goto cleanup added at least.
John
>
> + if (qemuDomainValidateVcpuInfo(vm) < 0) {
> + /* rollback vcpu count if the setting has failed */
> virDomainAuditVcpu(vm, oldvcpus, oldvcpus - 1, "update", false);
> vcpuinfo->online = true;
> goto cleanup;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 9136ba5..2e405af 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5193,6 +5193,9 @@ qemuProcessLaunch(virConnectPtr conn,
> if (qemuDomainRefreshVcpuInfo(driver, vm, asyncJob) < 0)
> goto cleanup;
>
> + if (qemuDomainValidateVcpuInfo(vm) < 0)
> + goto cleanup;
> +
> VIR_DEBUG("Detecting IOThread PIDs");
> if (qemuProcessDetectIOThreadPIDs(driver, vm, asyncJob) < 0)
> goto cleanup;
> @@ -5985,6 +5988,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
> if (qemuDomainRefreshVcpuInfo(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> goto error;
>
> + if (qemuDomainValidateVcpuInfo(vm) < 0)
> + goto error;
> +
> VIR_DEBUG("Detecting IOThread PIDs");
> if (qemuProcessDetectIOThreadPIDs(driver, vm, QEMU_ASYNC_JOB_NONE) < 0)
> goto error;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list