On Fri, Mar 02, 2018 at 10:29:09 +0100, Viktor Mihajlovski wrote:
> Extract architecture specific data from query-cpus[-fast] if
> available. A new function qemuMonitorJSONExtractCPUArchInfo()
> uses a call-back table to find and call architecture-specific
> extraction handlers.
>
> Initially, there's a handler for s390 cpu info to
> set the halted property depending on the s390 cpu state
> returned by QEMU. With this it's still possible to report
> the halted condition even when using query-cpus-fast.
>
> Signed-off-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_monitor.c | 6 ++-
> src/qemu/qemu_monitor_json.c | 123 ++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 126 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index dad1383..5a0e8a7 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2083,12 +2083,16 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon,
> size_t i;
> int rc;
> virBitmapPtr ret = NULL;
> + bool fast;
>
> QEMU_CHECK_MONITOR_NULL(mon);
>
> + fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
As said, mon->vm should not be used here.
> + QEMU_CAPS_QUERY_CPUS_FAST);
> +
> if (mon->json)
> rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false,
> - false);
> + fast);
> else
> rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 2ecdf0a..a408cfd 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1451,15 +1451,128 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon)
> }
>
>
> -/*
> +typedef struct {
> + const char *state;
> + bool halted;
> +} s390CpuStateMap;
> +
> +/**
> + * qemuMonitorJSONExtractCPUS390Info:
> + * @jsoncpu: pointer to a single JSON cpu entry
> + * @cpu: pointer to a single cpu entry
> + *
> + * Derive the legacy cpu info 'halted' information
> + * from the more accurate s390 cpu state. @cpu is
> + * modified only on success.
> + *
> + * Note: the 'uninitialized' s390 cpu state can't be
> + * mapped to halted yes/no.
> + *
> + * A s390 cpu entry could look like this
> + * { "arch": "s390",
> + * "cpu-index": 0,
> + * "qom-path": "/machine/unattached/device[0]",
> + * "thread_id": 3081,
> + * "cpu-state": "operating" }
> + *
> + * Returns 0 on success, and -2 if no cpu-state field was
> + * found or the state value is unknown.
> + */
> +static int
> +qemuMonitorJSONExtractCPUS390Info(virJSONValuePtr jsoncpu,
> + struct qemuMonitorQueryCpusEntry *cpu)
> +{
> + const char *cpu_state;
> + s390CpuStateMap states[] = {
> + { "operating", false},
> + { "load", false},
> + { "stopped", true},
> + { "check-stop", true},
> + };
> + size_t i;
> +
> + if ((cpu_state = virJSONValueObjectGetString(jsoncpu, "cpu-state"))) {
> + for (i = 0; i < ARRAY_CARDINALITY(states); i++) {
> + if (STREQ(states[i].state, cpu_state)) {
> + cpu->halted = states[i].halted;
> + return 0;
> + }
> + }
> + }
> +
> + return -2;
> +}
> +
> +typedef struct {
> + const char *arch;
> + int (*extract)(virJSONValuePtr jsoncpu,
> + struct qemuMonitorQueryCpusEntry *cpu);
> +} qemuCpuArchInfoHandler;
> +
> +
> +/**
> + * qemuMonitorJSONExtractCPUArchInfo:
> + * @arch: virtual CPU's architecture
> + * @jsoncpu: pointer to a single JSON cpu entry
> + * @cpu: pointer to a single cpu entry
> + *
> + * Extracts architecure specific virtual CPU data for a single
> + * CPU from the QAPI response using an architecture specific
> + * callback.
> *
> + * To add a support for a new architecture, extend the array
> + * 'handlers' with a line like
> + * ...
> + * { "newarch", qemuMonitorJSONExtractCPUNewarch },
This should be obvious from the code.
> + * ...
> + * and implement the extraction callback.
> + * Check the QEMU QAPI schema for valid architecture names.
> + *
> + * Returns the called handler's return value or 0, if no handler
> + * was called.
> + */
> +static int
> +qemuMonitorJSONExtractCPUArchInfo(const char *arch, virJSONValuePtr jsoncpu,
> + struct qemuMonitorQueryCpusEntry *cpu)
Please don't mix header alignment styles.
> +{
> + qemuCpuArchInfoHandler handlers[] = {
> + { "s390", qemuMonitorJSONExtractCPUS390Info },
> + };
> + size_t i;
> +
> + for (i = 0; i < ARRAY_CARDINALITY(handlers); i++) {
> + if (STREQ(handlers[i].arch, arch))
> + return handlers[i].extract(jsoncpu, cpu);
You can remove the callback topology and hard-code the STREQs. It will
result in the same execution complexity, but be clearer to understand
than the callback-topology.
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * qemuMonitorJSONExtractCPUArchInfo:
> + * @data: JSON response data
> + * @entries: filled with detected cpu entries on success
> + * @nentries: number of entries returned
> + * @fast: true if this is a response from query-cpus-fast
> + *
> + * The JSON response @data will have the following format
> + * in case @fast == false
> * [{ "arch": "x86",
> * "current": true,
> * "CPU": 0,
> * "qom_path": "/machine/unattached/device[0]",
> * "pc": -2130415978,
> * "halted": true,
> - * "thread_id": 2631237},
> + * "thread_id": 2631237,
> + * ...},
> + * {...}
> + * ]
> + * and for @fast == true
> + * [{ "arch": "x86",
> + * "cpu-index": 0,
> + * "qom-path": "/machine/unattached/device[0]",
> + * "thread_id": 2631237,
> + * ...},
> * {...}
> * ]
> */
> @@ -1486,6 +1599,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
> int thread = 0;
> bool halted = false;
> const char *qom_path;
> + const char *arch;
> if (!entry) {
> ret = -2;
> goto cleanup;
> @@ -1505,6 +1619,11 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
> cpus[i].halted = halted;
> if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0)
> goto cleanup;
> +
> + /* process optional architecture-specific data */
> + if ((arch = virJSONValueObjectGetString(entry, "arch")))
> + ignore_value(qemuMonitorJSONExtractCPUArchInfo(arch, entry,
> + cpus + i));
So why does this function try to return something if the only caller
does not care?
> }
>
> VIR_STEAL_PTR(*entries, cpus);
> --
> 1.9.1
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
