9:35 a.m.
Currently, the virVcpuInfo returned by virDomainGetVcpus() will always
report a state of VIR_VCPU_RUNNING for each defined domain vcpu even if
the vcpu is currently in the halted state.
As the monitor interface is in fact reporting the accurate state, it is
rather easy to transport this information with the existing API.
This is done by
- adding a new state of VIR_VCPU_HALTED
- adding a monitor function to pass back the halted state for the vcpus
- adding a new field to the private domain vcpu object reflecting the
halted state for the vcpu
- modifying the driver code to report the vcpu state based on the halted
indicator
- extending virsh vcpuinfo to also display the halted state
The vcpu state is however not recorded in the internal XML format, since
the state can change asynchronously (without notification).
Viktor Mihajlovski (5):
domain: Add new VCPU state "halted"
qemu: Add monitor APIs for the detection of halted VCPUs
tests: Add testcase for qemuMonitorJSONGetCPUState
qemu: Add domain support for VCPU halted state
qemu: Ensure reported VCPU state is current in driver API
include/libvirt/libvirt-domain.h | 1 +
src/qemu/qemu_domain.c | 82 ++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 4 ++
src/qemu/qemu_driver.c | 23 ++++++++-
src/qemu/qemu_monitor.c | 22 ++++++++
src/qemu/qemu_monitor.h | 2 +
src/qemu/qemu_monitor_json.c | 52 +++++++++++++++++--
src/qemu/qemu_monitor_json.h | 2 +
src/qemu/qemu_monitor_text.c | 54 ++++++++++++++++----
src/qemu/qemu_monitor_text.h | 2 +
tests/qemumonitorjsontest.c | 106 ++++++++++++++++++++++++---------------
tools/virsh-domain.c | 3 +-
12 files changed, 295 insertions(+), 58 deletions(-)
--
1.9.1
9:35 a.m.
New subject: [libvirt] [PATCH 1/5] domain: Add new VCPU state "halted"
QEMU virtual CPUs can assume a halted state, which - depending on
the architecture - can indicate whether a CPU is in use by the
guest operating system.
A new VCPU state halted is introduced in preparation of the
support in the QEMU driver (and optionally others where deemed
appropriate).
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
include/libvirt/libvirt-domain.h | 1 +
tools/virsh-domain.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7ea93aa..98b9420 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1656,6 +1656,7 @@ typedef enum {
VIR_VCPU_OFFLINE = 0, /* the virtual CPU is offline */
VIR_VCPU_RUNNING = 1, /* the virtual CPU is running */
VIR_VCPU_BLOCKED = 2, /* the virtual CPU is blocked on resource */
+ VIR_VCPU_HALTED = 3, /* the virtual CPU is halted */
# ifdef VIR_ENUM_SENTINELS
VIR_VCPU_LAST
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index dbdee5b..4650eb3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -181,7 +181,8 @@ VIR_ENUM_IMPL(virshDomainVcpuState,
VIR_VCPU_LAST,
N_("offline"),
N_("running"),
- N_("blocked"))
+ N_("blocked"),
+ N_("halted"))
static const char *
virshDomainVcpuStateToString(int state)
--
1.9.1
9:35 a.m.
New subject: [libvirt] [PATCH 2/5] qemu: Add monitor APIs for the detection of halted VCPUs
New monitor function qemuMonitorGetCPUState added to retrieve the
halted state of VCPUs via the query-cpus command.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++++++++++
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 52 ++++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_monitor_json.h | 2 ++
src/qemu/qemu_monitor_text.c | 54 +++++++++++++++++++++++++++++++++++---------
src/qemu/qemu_monitor_text.h | 2 ++
6 files changed, 119 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 098e654..0fdee29 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1667,6 +1667,28 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
}
+/**
+ * qemuMonitorGetCPUState:
+ * @mon: monitor
+ * @halted: returned array of boolean containing the vCPUs halted state
+ *
+ * Detects whether vCPUs are halted. Returns count of detected vCPUs on success,
+ * 0 if qemu didn't report vcpus (does not report libvirt error),
+ * -1 on error (reports libvirt error).
+ */
+int
+qemuMonitorGetCPUState(qemuMonitorPtr mon,
+ bool **halted)
+{
+ QEMU_CHECK_MONITOR(mon);
+
+ if (mon->json)
+ return qemuMonitorJSONGetCPUState(mon, halted);
+ else
+ return qemuMonitorTextGetCPUState(mon, halted);
+}
+
+
int
qemuMonitorSetLink(qemuMonitorPtr mon,
const char *name,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index cb4cca8..6bbb8a5 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -392,6 +392,8 @@ int qemuMonitorSystemPowerdown(qemuMonitorPtr mon);
int qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
int **pids);
+int qemuMonitorGetCPUState(qemuMonitorPtr mon,
+ bool **halted);
int qemuMonitorGetVirtType(qemuMonitorPtr mon,
virDomainVirtType *virtType);
int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bb426dc..878efaa 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1324,13 +1324,15 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon)
*/
static int
qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply,
- int **pids)
+ int **pids,
+ bool **halted)
{
virJSONValuePtr data;
int ret = -1;
size_t i;
int *threads = NULL;
ssize_t ncpus;
+ bool *haltedcpus = NULL;
if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1347,9 +1349,13 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply,
if (VIR_ALLOC_N(threads, ncpus) < 0)
goto cleanup;
+ if (VIR_ALLOC_N(haltedcpus, ncpus) < 0)
+ goto cleanup;
+
for (i = 0; i < ncpus; i++) {
virJSONValuePtr entry = virJSONValueArrayGet(data, i);
int thread;
+ bool ishalted;
if (!entry) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cpu information was missing an array element"));
@@ -1363,15 +1369,31 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply,
goto cleanup;
}
+ if (virJSONValueObjectGetBoolean(entry, "halted", &ishalted) <
0) {
+ /* Some older qemu versions may not report the halted state,
+ * for backward compatibility we assume it's not halted */
+ ishalted = false;
+ }
+
threads[i] = thread;
+ haltedcpus[i] = ishalted;
}
- *pids = threads;
- threads = NULL;
+ if (pids) {
+ VIR_FREE(*pids);
+ *pids = threads;
+ threads = NULL;
+ }
+ if (halted) {
+ VIR_FREE(*halted);
+ *halted = haltedcpus;
+ haltedcpus = NULL;
+ }
ret = ncpus;
cleanup:
VIR_FREE(threads);
+ VIR_FREE(haltedcpus);
return ret;
}
@@ -1394,8 +1416,30 @@ int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon,
if (qemuMonitorJSONCheckError(cmd, reply) < 0)
goto cleanup;
+ ret = qemuMonitorJSONExtractCPUInfo(reply, pids, NULL);
+ cleanup:
+ virJSONValueFree(cmd);
+ virJSONValueFree(reply);
+ return ret;
+}
+
+int qemuMonitorJSONGetCPUState(qemuMonitorPtr mon,
+ bool **halted)
+{
+ int ret = -1;
+ virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus",
+ NULL);
+ virJSONValuePtr reply = NULL;
+
+ if (!cmd)
+ return -1;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ goto cleanup;
- ret = qemuMonitorJSONExtractCPUInfo(reply, pids);
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ goto cleanup;
+ ret = qemuMonitorJSONExtractCPUInfo(reply, NULL, halted);
cleanup:
virJSONValueFree(cmd);
virJSONValueFree(reply);
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 37a739e..5469079 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -60,6 +60,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon);
int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon,
int **pids);
+int qemuMonitorJSONGetCPUState(qemuMonitorPtr mon,
+ bool **halted);
int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
virDomainVirtType *virtType);
int qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 9295219..bdbf1a3 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -500,13 +500,17 @@ int qemuMonitorTextSystemReset(qemuMonitorPtr mon)
}
-int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
- int **pids)
+static int qemuMonitorTextGetCPUInfoInternal(qemuMonitorPtr mon,
+ int **pids,
+ bool **halted)
{
char *qemucpus = NULL;
char *line;
pid_t *cpupids = NULL;
size_t ncpupids = 0;
+ bool *cpuhalted = NULL;
+ size_t ncpuhalted = 0;
+ int ret = 0;
if (qemuMonitorHMPCommand(mon, "info cpus", &qemucpus) < 0)
return -1;
@@ -517,7 +521,7 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
* (qemu) info cpus
* * CPU #0: pc=0x00000000000f0c4a thread_id=30019
* CPU #1: pc=0x00000000fffffff0 thread_id=30020
- * CPU #2: pc=0x00000000fffffff0 thread_id=30021
+ * CPU #2: pc=0x00000000fffffff0 (halted) thread_id=30021
*
*/
line = qemucpus;
@@ -525,18 +529,26 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
char *offset = NULL;
char *end = NULL;
int tid = 0;
+ bool ishalted = false;
/* Extract host Thread ID */
if ((offset = strstr(line, "thread_id=")) == NULL)
- goto error;
+ goto cleanup;
if (virStrToLong_i(offset + strlen("thread_id="), &end, 10,
&tid) < 0)
- goto error;
+ goto cleanup;
if (end == NULL || !c_isspace(*end))
- goto error;
+ goto cleanup;
if (VIR_APPEND_ELEMENT_COPY(cpupids, ncpupids, tid) < 0)
- goto error;
+ goto cleanup;
+
+ /* Extract halted indicator */
+ if ((offset = strstr(line, "(halted)")) != NULL)
+ ishalted = true;
+
+ if (VIR_APPEND_ELEMENT_COPY(cpuhalted, ncpuhalted, ishalted) < 0)
+ goto cleanup;
VIR_DEBUG("tid=%d", tid);
@@ -548,18 +560,38 @@ int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
/* Validate we got data for all VCPUs we expected */
VIR_FREE(qemucpus);
- *pids = cpupids;
- return ncpupids;
+ if (pids) {
+ VIR_FREE(*pids);
+ *pids = cpupids;
+ }
+ if (halted) {
+ VIR_FREE(*halted);
+ *halted = cpuhalted;
+ }
+ ret = ncpupids;
- error:
+ cleanup:
VIR_FREE(qemucpus);
VIR_FREE(cpupids);
+ VIR_FREE(cpuhalted);
/* Returning 0 to indicate non-fatal failure, since
* older QEMU does not have VCPU<->PID mapping and
* we don't want to fail on that
*/
- return 0;
+ return ret;
+}
+
+int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
+ int **pids)
+{
+ return qemuMonitorTextGetCPUInfoInternal(mon, pids, NULL);
+}
+
+int qemuMonitorTextGetCPUState(qemuMonitorPtr mon,
+ bool **halted)
+{
+ return qemuMonitorTextGetCPUInfoInternal(mon, NULL, halted);
}
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index eeaca52..7c185ff 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -51,6 +51,8 @@ int qemuMonitorTextSystemReset(qemuMonitorPtr mon);
int qemuMonitorTextGetCPUInfo(qemuMonitorPtr mon,
int **pids);
+int qemuMonitorTextGetCPUState(qemuMonitorPtr mon,
+ bool **halted);
int qemuMonitorTextGetVirtType(qemuMonitorPtr mon,
virDomainVirtType *virtType);
int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon,
--
1.9.1
4:40 a.m.
New subject: [libvirt] [PATCH 2/5] qemu: Add monitor APIs for the detection of halted VCPUs
On Thu, Jul 14, 2016 at 16:35:39 +0200, Viktor Mihajlovski wrote:
New monitor function qemuMonitorGetCPUState added to retrieve the
halted state of VCPUs via the query-cpus command.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/qemu/qemu_monitor.c | 22 ++++++++++++++++++
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 52 ++++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_monitor_json.h | 2 ++
src/qemu/qemu_monitor_text.c | 54 +++++++++++++++++++++++++++++++++++---------
src/qemu/qemu_monitor_text.h | 2 ++
6 files changed, 119 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 098e654..0fdee29 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1667,6 +1667,28 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
}
+/**
+ * qemuMonitorGetCPUState:
+ * @mon: monitor
+ * @halted: returned array of boolean containing the vCPUs halted state
+ *
+ * Detects whether vCPUs are halted. Returns count of detected vCPUs on success,
+ * 0 if qemu didn't report vcpus (does not report libvirt error),
+ * -1 on error (reports libvirt error).
+ */
+int
+qemuMonitorGetCPUState(qemuMonitorPtr mon,
+ bool **halted)
Extraction of the data is cheap compared to calling the monitor. Also
this adds a lot of duplicated code. I've merged this to the code
returnign the thread ids and dropped the duplicate monitor helpers.
I've kept the extraction code though.
Peter
9:35 a.m.
New subject: [libvirt] [PATCH 3/5] tests: Add testcase for qemuMonitorJSONGetCPUState
Mix of halted and non-halted VCPUs used for verification.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
tests/qemumonitorjsontest.c | 106 +++++++++++++++++++++++++++-----------------
1 file changed, 66 insertions(+), 40 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index f698c14..555af7f 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1210,57 +1210,64 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUInfo(const void *data)
int ret = -1;
pid_t *cpupids = NULL;
pid_t expected_cpupids[] = {17622, 17624, 17626, 17628};
- int ncpupids;
+ bool *cpuhalted = NULL;
+ bool expected_cpuhalted[] = {true, true, false, false};
+ int ncpusfound;
size_t i;
+ const char *replystring;
if (!test)
return -1;
- if (qemuMonitorTestAddItem(test, "query-cpus",
- "{"
- " \"return\": ["
- " {"
- " \"current\": true,"
- " \"CPU\": 0,"
- " \"pc\": -2130530478,"
- " \"halted\": true,"
- " \"thread_id\": 17622"
- " },"
- " {"
- " \"current\": false,"
- " \"CPU\": 1,"
- " \"pc\": -2130530478,"
- " \"halted\": true,"
- " \"thread_id\": 17624"
- " },"
- " {"
- " \"current\": false,"
- " \"CPU\": 2,"
- " \"pc\": -2130530478,"
- " \"halted\": true,"
- " \"thread_id\": 17626"
- " },"
- " {"
- " \"current\": false,"
- " \"CPU\": 3,"
- " \"pc\": -2130530478,"
- " \"halted\": true,"
- " \"thread_id\": 17628"
- " }"
- " ],"
- " \"id\": \"libvirt-7\""
- "}") < 0)
- goto cleanup;
+ replystring =
+ "{"
+ " \"return\": ["
+ " {"
+ " \"current\": true,"
+ " \"CPU\": 0,"
+ " \"pc\": -2130530478,"
+ " \"halted\": true,"
+ " \"thread_id\": 17622"
+ " },"
+ " {"
+ " \"current\": false,"
+ " \"CPU\": 1,"
+ " \"pc\": -2130530478,"
+ " \"halted\": true,"
+ " \"thread_id\": 17624"
+ " },"
+ " {"
+ " \"current\": false,"
+ " \"CPU\": 2,"
+ " \"pc\": -2130530478,"
+ " \"halted\": false,"
+ " \"thread_id\": 17626"
+ " },"
+ " {"
+ " \"current\": false,"
+ " \"CPU\": 3,"
+ " \"pc\": -2130530478,"
+ " \"halted\": false,"
+ " \"thread_id\": 17628"
+ " }"
+ " ],"
+ " \"id\": \"libvirt-7\""
+ "}";
- ncpupids = qemuMonitorJSONGetCPUInfo(qemuMonitorTestGetMonitor(test), &cpupids);
+ if (qemuMonitorTestAddItem(test, "query-cpus", replystring) < 0)
+ goto cleanup;
+ if (qemuMonitorTestAddItem(test, "query-cpus", replystring) < 0)
+ goto cleanup;
- if (ncpupids != 4) {
+ ncpusfound = qemuMonitorJSONGetCPUInfo(qemuMonitorTestGetMonitor(test),
+ &cpupids);
+ if (ncpusfound != 4) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- "Expecting ncpupids = 4 but got %d", ncpupids);
+ "Expecting ncpusfound = 4 but got %d", ncpusfound);
goto cleanup;
}
- for (i = 0; i < ncpupids; i++) {
+ for (i = 0; i < ncpusfound; i++) {
if (cpupids[i] != expected_cpupids[i]) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"Expecting cpupids[%zu] = %d but got %d",
@@ -1269,10 +1276,29 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUInfo(const void *data)
}
}
+ ncpusfound = qemuMonitorJSONGetCPUState(qemuMonitorTestGetMonitor(test),
+ &cpuhalted);
+
+ if (ncpusfound != 4) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "Expecting ncpusfound = 4 but got %d", ncpusfound);
+ goto cleanup;
+ }
+
+ for (i = 0; i < ncpusfound; i++) {
+ if (cpuhalted[i] != expected_cpuhalted[i]) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "Expecting cpuhalted[%zu] = %d but got %d",
+ i, expected_cpuhalted[i], cpuhalted[i]);
+ goto cleanup;
+ }
+ }
+
ret = 0;
cleanup:
VIR_FREE(cpupids);
+ VIR_FREE(cpuhalted);
qemuMonitorTestFree(test);
return ret;
}
--
1.9.1
9:35 a.m.
New subject: [libvirt] [PATCH 4/5] qemu: Add domain support for VCPU halted state
Adding a field to the domain's private vcpu object to hold the halted
state information.
Adding two functions in support of the halted state:
- qemuDomainGetVcpuHalted: retrieve the halted state from a
private vcpu object
- qemuDomainRefreshVcpuHalted: obtain the per-vcpu halted states
via qemu monitor and store the results in the private vcpu objects
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/qemu/qemu_domain.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 4 +++
2 files changed, 86 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 286f096..945a75d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5650,6 +5650,88 @@ qemuDomainDetectVcpuPids(virQEMUDriverPtr driver,
return ret;
}
+/**
+ * qemuDomainGetVcpuHalted:
+ * @vm: domain object
+ * @vcpu: cpu id
+ *
+ * Returns the vCPU halted state.
+ */
+bool
+qemuDomainGetVcpuHalted(virDomainObjPtr vm,
+ unsigned int vcpuid)
+{
+ virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
+ return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted;
+}
+
+/**
+ * qemuDomainRefreshVcpuHalted:
+ * @driver: qemu driver data
+ * @vm: domain object
+ * @asyncJob: current asynchronous job type
+ *
+ * Updates vCPU halted state in the private data of @vm.
+ *
+ * Returns number of detected vCPUs on success, -1 on error and reports
+ * an appropriate error, -2 if the domain doesn't exist any more.
+ */
+int
+qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ int asyncJob)
+{
+ virDomainVcpuDefPtr vcpu;
+ size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
+ bool *cpuhalted = NULL;
+ int ncpuhalted;
+ size_t i;
+ int ret = -1;
+
+ /* Not supported currently for TCG, see above
+ */
+ if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU)
+ return 0;
+
+ if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+ return -1;
+ ncpuhalted = qemuMonitorGetCPUState(qemuDomainGetMonitor(vm), &cpuhalted);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ ret = -2;
+ goto cleanup;
+ }
+
+ /* Don't fail for older QEMUs
+ */
+ if (ncpuhalted <= 0) {
+ virResetLastError();
+ ret = 0;
+ goto cleanup;
+ }
+
+ for (i = 0; i < maxvcpus; i++) {
+ vcpu = virDomainDefGetVcpu(vm->def, i);
+
+ if (i < ncpuhalted)
+ QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = cpuhalted[i];
+ else
+ QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = false;
+ }
+
+ if (ncpuhalted != virDomainDefGetVcpus(vm->def)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("got wrong number of vCPUs from QEMU monitor. "
+ "got %d, wanted %d"),
+ ncpuhalted, virDomainDefGetVcpus(vm->def));
+ goto cleanup;
+ }
+
+ ret = ncpuhalted;
+
+ cleanup:
+ VIR_FREE(cpuhalted);
+ return ret;
+}
bool
qemuDomainSupportsNicdev(virDomainDefPtr def,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 114db98..44add02 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -316,6 +316,7 @@ struct _qemuDomainVcpuPrivate {
virObject parent;
pid_t tid; /* vcpu thread id */
+ bool halted; /* vcpu halted state */
};
# define QEMU_DOMAIN_VCPU_PRIVATE(vcpu) \
@@ -649,6 +650,9 @@ bool qemuDomainHasVcpuPids(virDomainObjPtr vm);
pid_t qemuDomainGetVcpuPid(virDomainObjPtr vm, unsigned int vcpuid);
int qemuDomainDetectVcpuPids(virQEMUDriverPtr driver, virDomainObjPtr vm,
int asyncJob);
+bool qemuDomainGetVcpuHalted(virDomainObjPtr vm, unsigned int vcpu);
+int qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, virDomainObjPtr vm,
+ int asyncJob);
bool qemuDomainSupportsNicdev(virDomainDefPtr def,
virDomainNetDefPtr net);
--
1.9.1
4:38 a.m.
New subject: [libvirt] [PATCH 4/5] qemu: Add domain support for VCPU halted state
On Thu, Jul 14, 2016 at 16:35:41 +0200, Viktor Mihajlovski wrote:
Adding a field to the domain's private vcpu object to hold the
halted
state information.
Adding two functions in support of the halted state:
- qemuDomainGetVcpuHalted: retrieve the halted state from a
private vcpu object
- qemuDomainRefreshVcpuHalted: obtain the per-vcpu halted states
via qemu monitor and store the results in the private vcpu objects
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/qemu/qemu_domain.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 4 +++
2 files changed, 86 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 286f096..945a75d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5650,6 +5650,88 @@ qemuDomainDetectVcpuPids(virQEMUDriverPtr driver,
return ret;
}
+/**
+ * qemuDomainGetVcpuHalted:
+ * @vm: domain object
+ * @vcpu: cpu id
+ *
+ * Returns the vCPU halted state.
+ */
+bool
+qemuDomainGetVcpuHalted(virDomainObjPtr vm,
+ unsigned int vcpuid)
+{
+ virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
+ return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted;
+}
+
+/**
+ * qemuDomainRefreshVcpuHalted:
+ * @driver: qemu driver data
+ * @vm: domain object
+ * @asyncJob: current asynchronous job type
+ *
+ * Updates vCPU halted state in the private data of @vm.
+ *
+ * Returns number of detected vCPUs on success, -1 on error and reports
+ * an appropriate error, -2 if the domain doesn't exist any more.
+ */
+int
+qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ int asyncJob)
+{
+ virDomainVcpuDefPtr vcpu;
+ size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
+ bool *cpuhalted = NULL;
+ int ncpuhalted;
+ size_t i;
+ int ret = -1;
+
+ /* Not supported currently for TCG, see above
+ */
+ if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU)
+ return 0;
+
+ if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+ return -1;
+ ncpuhalted = qemuMonitorGetCPUState(qemuDomainGetMonitor(vm), &cpuhalted);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ ret = -2;
+ goto cleanup;
+ }
+
+ /* Don't fail for older QEMUs
+ */
+ if (ncpuhalted <= 0) {
+ virResetLastError();
+ ret = 0;
+ goto cleanup;
+ }
+
+ for (i = 0; i < maxvcpus; i++) {
+ vcpu = virDomainDefGetVcpu(vm->def, i);
+
+ if (i < ncpuhalted)
+ QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = cpuhalted[i];
+ else
+ QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = false;
I've merged the 4 lines above to the function refreshing other VCPU
(since I'll be adding a patch that will make it not refresh the pids all
the time) and thus dropped the rest of this function.
I've tweaked the commit message an I'll resend it with the rest of the
patches soon.
Peter
9:35 a.m.
New subject: [libvirt] [PATCH 5/5] qemu: Ensure reported VCPU state is current in driver API
Refresh the VCPU halted states in API functions returning domain
VCPU state information to make sure it's current. This affects
qemuDomainGetVcpus and qemuDomainGetStatsVcpu
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/qemu/qemu_driver.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cda85f6..de9be99 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1477,13 +1477,17 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
for (i = 0; i < virDomainDefGetVcpusMax(vm->def) && ncpuinfo <
maxinfo; i++) {
virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i);
pid_t vcpupid = qemuDomainGetVcpuPid(vm, i);
+ bool vcpuhalted = qemuDomainGetVcpuHalted(vm, i);
if (!vcpu->online)
continue;
if (info) {
info[i].number = i;
- info[i].state = VIR_VCPU_RUNNING;
+ if (vcpuhalted)
+ info[i].state = VIR_VCPU_HALTED;
+ else
+ info[i].state = VIR_VCPU_RUNNING;
if (qemuGetProcessInfo(&(info[i].cpuTime), &(info[i].cpu), NULL,
vm->pid, vcpupid) < 0) {
@@ -5267,6 +5271,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
unsigned char *cpumaps,
int maplen)
{
+ virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
@@ -5283,6 +5288,13 @@ qemuDomainGetVcpus(virDomainPtr dom,
goto cleanup;
}
+ if (qemuDomainRefreshVcpuHalted(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s",
+ _("could not refresh CPU states"));
+ return -1;
+ }
+
ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen);
cleanup:
@@ -18535,7 +18547,7 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver
ATTRIBUTE_UNUSED,
static int
-qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
+qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
@@ -18565,6 +18577,13 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
goto cleanup;
+ if (qemuDomainRefreshVcpuHalted(driver, dom, QEMU_ASYNC_JOB_NONE) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s",
+ _("could not refresh CPU states"));
+ return -1;
+ }
+
if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
virDomainDefGetVcpus(dom->def),
NULL, 0) < 0) {
--
1.9.1
4:35 a.m.
New subject: [libvirt] [PATCH 5/5] qemu: Ensure reported VCPU state is current in driver API
On Thu, Jul 14, 2016 at 16:35:42 +0200, Viktor Mihajlovski wrote:
Refresh the VCPU halted states in API functions returning domain
VCPU state information to make sure it's current. This affects
qemuDomainGetVcpus and qemuDomainGetStatsVcpu
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
src/qemu/qemu_driver.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
[..]
@@ -5283,6 +5288,13 @@ qemuDomainGetVcpus(virDomainPtr dom,
goto cleanup;
}
+ if (qemuDomainRefreshVcpuHalted(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) {
APIs calling the monitor need to enter a job since monitor calls unlock
the domain. There wouldn't be anything guarding the API.
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s",
+ _("could not refresh CPU states"));
+ return -1;
This skips cleanup section where the VM object needs to be unlocked.
+ }
+
ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen);
cleanup:
@@ -18565,6 +18577,13 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr
driver ATTRIBUTE_UNUSED,
VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
goto cleanup;
[1]
>
> + if (qemuDomainRefreshVcpuHalted(driver, dom, QEMU_ASYNC_JOB_NONE) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s",
+ _("could not refresh CPU states"));
+ return -1;
This would leak memory allocated just above this hunk.
+ }
+
I've fixed the problems mentioned above and tweaked the code to comply
with changes done to previous patches. I've also dropped the reviewed-by
tag since I'll be sending this for review again due to the changes
needed.
Peter
12:56 a.m.
On Thu, Jul 14, 2016 at 16:35:37 +0200, Viktor Mihajlovski wrote:
Currently, the virVcpuInfo returned by virDomainGetVcpus() will
always
report a state of VIR_VCPU_RUNNING for each defined domain vcpu even if
the vcpu is currently in the halted state.
As the monitor interface is in fact reporting the accurate state, it is
rather easy to transport this information with the existing API.
This is done by
- adding a new state of VIR_VCPU_HALTED
- adding a monitor function to pass back the halted state for the vcpus
- adding a new field to the private domain vcpu object reflecting the
halted state for the vcpu
- modifying the driver code to report the vcpu state based on the halted
indicator
- extending virsh vcpuinfo to also display the halted state
The vcpu state is however not recorded in the internal XML format, since
the state can change asynchronously (without notification).
I'm going to pick this up in my work-in-progress series of adding vCPU
hot(un)plug using the new APIs in qemu and I'll let you know about the
issues once I see how well it integrates since it is touching the same
places.
Peter
1:30 a.m.
On Mon, Jul 18, 2016 at 07:56:36 +0200, Peter Krempa wrote:
On Thu, Jul 14, 2016 at 16:35:37 +0200, Viktor Mihajlovski wrote:
> Currently, the virVcpuInfo returned by virDomainGetVcpus() will always
> report a state of VIR_VCPU_RUNNING for each defined domain vcpu even if
> the vcpu is currently in the halted state.
>
> As the monitor interface is in fact reporting the accurate state, it is
> rather easy to transport this information with the existing API.
>
> This is done by
> - adding a new state of VIR_VCPU_HALTED
> - adding a monitor function to pass back the halted state for the vcpus
> - adding a new field to the private domain vcpu object reflecting the
> halted state for the vcpu
> - modifying the driver code to report the vcpu state based on the halted
> indicator
> - extending virsh vcpuinfo to also display the halted state
>
> The vcpu state is however not recorded in the internal XML format, since
> the state can change asynchronously (without notification).
I'm going to pick this up in my work-in-progress series of adding vCPU
hot(un)plug using the new APIs in qemu and I'll let you know about the
issues once I see how well it integrates since it is touching the same
places.
So I was testing the data reported from the monitor while doing some
work on othe vCPU hotplug series and found that for x86 the halted state
is reported if the CPU is idle and thus halted. This state it therefore
almost always reported when the VM is idle.
Quoting conversation on previous version [1]:
> Could you please elaborat how you expect to use this info?
Dpending on the architecture, the halted state is an indication that the
virtual CPU is not in use. On s390x for example the halted state is only
reported if the CPU is stopped or in disabled wait, both indicating that
the virtual CPUs are not enabled for the operating system.
While for the s390 platform the state may reasonably express that the
vcpu is unused, other platforms have apparently different meaning.
Addditionally on x86_64 if the cpu is hotplugged but not grabbed by the
guest it is reporting non-halted state.
This information can be used by a libvirt client application for
hotplugging decisions, and this is in fact the intention.
Wouldn't it be possible for you to use the virDomainGetGuestVcpus [2]
API to determine this information more reliably using the guest agent?
I'm also a bit worried that the new state might cause problems with
older apps actually using the value for any reason.
Said that. I still keep the series in a branch and I'll post a updated
version including rebase on top of the oncomming vCPU hotplug series
once I finish it since it completely rewrites the exact part of the
monitor code requesting the information due to changes necessary to
detect hotpluggable cpus in recent qemu.
Peter
[1] https://www.redhat.com/archives/libvir-list/2016-July/msg00234.html
[2] http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetGuestVcpus
4:29 a.m.
On 29.07.2016 08:30, Peter Krempa wrote:
So I was testing the data reported from the monitor while doing some
work on othe vCPU hotplug series and found that for x86 the halted state
is reported if the CPU is idle and thus halted. This state it therefore
almost always reported when the VM is idle.
Quoting conversation on previous version [1]:
>> Could you please elaborat how you expect to use this info?
> Dpending on the architecture, the halted state is an indication that the
> virtual CPU is not in use. On s390x for example the halted state is only
> reported if the CPU is stopped or in disabled wait, both indicating that
> the virtual CPUs are not enabled for the operating system.
While for the s390 platform the state may reasonably express that the
vcpu is unused, other platforms have apparently different meaning.
I agree, however
I haven't claimed that s390x and x86 use the same
mechanism or have the same semantics, even if in both cases the CPUs are
doing "nothing" (in the x86 case in a much more transient way).
Addditionally on x86_64 if the cpu is hotplugged but not grabbed by the
guest it is reporting non-halted state.
I don't have a current x86 QEMU setup to check with, but I am wondering
what the CPU thread is actually doing (actively looping?) if it's not
executing the HLT instruction. Anyway, it may be a correct
representation of the CPU doing "something".
> This information can be used by a libvirt client application for
> hotplugging decisions, and this is in fact the intention.
Wouldn't it be possible for you to use the virDomainGetGuestVcpus [2]
API to determine this information more reliably using the guest agent?
As I said,
on s390x the hypervisor-reported state IS reliable. Requiring
the guest agent may not be acceptable for all use cases.
I'm also a bit worried that the new state might cause problems with
older apps actually using the value for any reason.
That could of course be, but
otoh apps should really not assume that
enumerations like the cpu state will never be extended. virsh is a nice
example showing how to deal with it.
Said that. I still keep the series in a branch and I'll post a updated
version including rebase on top of the oncomming vCPU hotplug series
once I finish it since it completely rewrites the exact part of the
monitor code requesting the information due to changes necessary to
detect hotpluggable cpus in recent qemu.
Great, looking forward to it.
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
3068
days inactive
3083
days old
11 comments
2 participants
participants (2)
-
Peter Krempa -
Viktor Mihajlovski