Re: [PATCH 4/6] qemu: add architecture-specific CPU info handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux