4:30 p.m.
The halted state information is useless on most platforms and calling
'query-cpus' is very expensive. Don't report the info on platforms where
it does not make sense.
Peter Krempa (3):
qemu: driver: Extract vcpu halted state directly
qemu: Remove unused 'cpuhalted' argument from qemuDomainHelperGetVcpus
qemu: domain: Store vcpu halted state as a tristate
Viktor Mihajlovski (1):
qemu: Limit refresh of CPU halted state to s390
src/qemu/qemu_domain.c | 8 +++++++-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 36 +++++++++++++++---------------------
3 files changed, 23 insertions(+), 23 deletions(-)
--
2.15.0
4:30 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: driver: Extract vcpu halted state directly
Don't extract the halted state into a separate array, but rater access
the vcpu structures directly. We still need to call the vcpu helper to
retrieve the performance statistics though.
---
src/qemu/qemu_driver.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bff49e7be6..172a8a3f73 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19660,12 +19660,14 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
int *maxparams,
unsigned int privflags)
{
+ virDomainVcpuDefPtr vcpu;
+ qemuDomainVcpuPrivatePtr vcpupriv;
size_t i;
int ret = -1;
char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
virVcpuInfoPtr cpuinfo = NULL;
unsigned long long *cpuwait = NULL;
- bool *cpuhalted = NULL;
+ bool vcpuhalted = false;
if (virTypedParamsAddUInt(&record->params,
&record->nparams,
@@ -19691,14 +19693,14 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
/* it's ok to be silent and go ahead, because halted vcpu info
* wasn't here from the beginning */
virResetLastError();
- } else if (VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0) {
- goto cleanup;
+ } else {
+ vcpuhalted = true;
}
}
if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
virDomainDefGetVcpus(dom->def),
- NULL, 0, cpuhalted) < 0) {
+ NULL, 0, NULL) < 0) {
virResetLastError();
ret = 0; /* it's ok to be silent and go ahead */
goto cleanup;
@@ -19735,14 +19737,20 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
cpuwait[i]) < 0)
goto cleanup;
- if (cpuhalted) {
+ /* state below is extracted from the individual vcpu structs */
+ if (!(vcpu = virDomainDefGetVcpu(dom->def, cpuinfo[i].number)))
+ continue;
+
+ vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu);
+
+ if (vcpuhalted) {
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
"vcpu.%u.halted", cpuinfo[i].number);
if (virTypedParamsAddBoolean(&record->params,
&record->nparams,
maxparams,
param_name,
- cpuhalted[i]) < 0)
+ vcpupriv->halted) < 0)
goto cleanup;
}
}
@@ -19752,7 +19760,6 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
cleanup:
VIR_FREE(cpuinfo);
VIR_FREE(cpuwait);
- VIR_FREE(cpuhalted);
return ret;
}
--
2.15.0
4:30 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: Remove unused 'cpuhalted' argument from qemuDomainHelperGetVcpus
The halted state is no longer extracted using this helper so the
argument can be removed.
---
src/qemu/qemu_driver.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 172a8a3f73..eaa6049fee 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1490,8 +1490,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
unsigned long long *cpuwait,
int maxinfo,
unsigned char *cpumaps,
- int maplen,
- bool *cpuhalted)
+ int maplen)
{
size_t ncpuinfo = 0;
size_t i;
@@ -1511,9 +1510,6 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
if (cpumaps)
memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo);
- if (cpuhalted)
- memset(cpuhalted, 0, sizeof(*cpuhalted) * maxinfo);
-
for (i = 0; i < virDomainDefGetVcpusMax(vm->def) && ncpuinfo <
maxinfo; i++) {
virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i);
pid_t vcpupid = qemuDomainGetVcpuPid(vm, i);
@@ -1551,9 +1547,6 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
return -1;
}
- if (cpuhalted)
- cpuhalted[ncpuinfo] = qemuDomainGetVcpuHalted(vm, ncpuinfo);
-
ncpuinfo++;
}
@@ -5461,8 +5454,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
goto cleanup;
}
- ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen,
- NULL);
+ ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen);
cleanup:
virDomainObjEndAPI(&vm);
@@ -19700,7 +19692,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
virDomainDefGetVcpus(dom->def),
- NULL, 0, NULL) < 0) {
+ NULL, 0) < 0) {
virResetLastError();
ret = 0; /* it's ok to be silent and go ahead */
goto cleanup;
--
2.15.0
4:30 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: domain: Store vcpu halted state as a tristate
Since it may be possible that the state is unknown in some cases we
should store it as a tristate so that other code using it can determine
whether the state was updated.
---
src/qemu/qemu_domain.c | 3 ++-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 13 ++++---------
3 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 429660689b..5395cd093c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8737,7 +8737,8 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
for (i = 0; i < maxvcpus; i++) {
vcpu = virDomainDefGetVcpu(vm->def, i);
vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu);
- vcpupriv->halted = virBitmapIsBitSet(haltedmap, vcpupriv->qemu_id);
+ vcpupriv->halted = virTristateBoolFromBool(virBitmapIsBitSet(haltedmap,
+
vcpupriv->qemu_id));
}
ret = 0;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 25328cd110..f3ec5d8042 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -399,7 +399,7 @@ struct _qemuDomainVcpuPrivate {
int enable_id; /* order in which the vcpus were enabled in qemu */
int qemu_id; /* ID reported by qemu as 'CPU' in query-cpus */
char *alias;
- bool halted;
+ virTristateBool halted;
/* information for hotpluggable cpus */
char *type;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index eaa6049fee..8d77d8913a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19659,7 +19659,6 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
virVcpuInfoPtr cpuinfo = NULL;
unsigned long long *cpuwait = NULL;
- bool vcpuhalted = false;
if (virTypedParamsAddUInt(&record->params,
&record->nparams,
@@ -19679,15 +19678,11 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
goto cleanup;
- if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
- if (qemuDomainRefreshVcpuHalted(driver, dom,
- QEMU_ASYNC_JOB_NONE) < 0) {
+ if (HAVE_JOB(privflags) && virDomainObjIsActive(dom) &&
+ qemuDomainRefreshVcpuHalted(driver, dom, QEMU_ASYNC_JOB_NONE) < 0) {
/* it's ok to be silent and go ahead, because halted vcpu info
* wasn't here from the beginning */
virResetLastError();
- } else {
- vcpuhalted = true;
- }
}
if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
@@ -19735,14 +19730,14 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu);
- if (vcpuhalted) {
+ if (vcpupriv->halted != VIR_TRISTATE_BOOL_ABSENT) {
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
"vcpu.%u.halted", cpuinfo[i].number);
if (virTypedParamsAddBoolean(&record->params,
&record->nparams,
maxparams,
param_name,
- vcpupriv->halted) < 0)
+ vcpupriv->halted == VIR_TRISTATE_BOOL_YES)
< 0)
goto cleanup;
}
}
--
2.15.0
4:30 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: Limit refresh of CPU halted state to s390
From: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Refreshing the halted state can cause VM performance issues. Since
s390 is currently the only architecture with a known interest in
the halted state, we're avoiding to call QEMU on other platforms.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
src/qemu/qemu_domain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5395cd093c..aa652959ed 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8726,6 +8726,11 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU)
return 0;
+ /* The halted state is interresting only on s390(x). On other platforms
+ * the data would be stale at the time when it would be used. */
+ if (!ARCH_IS_S390(vm->def->os.arch))
+ return 0;
+
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
--
2.15.0
8:40 a.m.
On 06.02.2018 16:30, Peter Krempa wrote:
The halted state information is useless on most platforms and
calling
'query-cpus' is very expensive. Don't report the info on platforms where
it does not make sense.
Peter Krempa (3):
qemu: driver: Extract vcpu halted state directly
qemu: Remove unused 'cpuhalted' argument from qemuDomainHelperGetVcpus
qemu: domain: Store vcpu halted state as a tristate
Viktor Mihajlovski (1):
qemu: Limit refresh of CPU halted state to s390
src/qemu/qemu_domain.c | 8 +++++++-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 36 +++++++++++++++---------------------
3 files changed, 23 insertions(+), 23 deletions(-)
Hi Peter,
I was planning to pick up your tri-state proposal but you were quicker.
Series looks good to me. Refactoring the halted state retrieval is
probably the right thing do.
Thanks!
--
Regards,
Viktor Mihajlovski
1:43 p.m.
On Tue, Feb 06, 2018 at 16:30:51 +0100, Peter Krempa wrote:
The halted state information is useless on most platforms and
calling
'query-cpus' is very expensive. Don't report the info on platforms where
it does not make sense.
Peter Krempa (3):
qemu: driver: Extract vcpu halted state directly
qemu: Remove unused 'cpuhalted' argument from qemuDomainHelperGetVcpus
qemu: domain: Store vcpu halted state as a tristate
Viktor Mihajlovski (1):
qemu: Limit refresh of CPU halted state to s390
src/qemu/qemu_domain.c | 8 +++++++-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 36 +++++++++++++++---------------------
3 files changed, 23 insertions(+), 23 deletions(-)
ACK series
Jirka
2418
days inactive
2419
days old
6 comments
3 participants
participants (3)
-
Jiri Denemark -
Peter Krempa -
Viktor Mihajlovski