Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support

[Igor Mammedov]

On Tue,  3 Dec 2013 14:42:48 +1100
Alexey Kardashevskiy <address@hidden> wrote:

> This adds suboptions support for -cpu. This keeps @cpu_model in order not
> to break the existing architectures/machines.
> 
> Cc: Andreas Färber <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> Changes:
> v4:
> * moved QemuOpts logic to qeom/cpu.c
> * added cpu_opt_get() as the machine init code wants to know the CPU name
> to create a CPU object
> ---
>  include/qom/cpu.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  qom/cpu.c         | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  vl.c              | 23 +++++++++++++++++++++++
>  3 files changed, 113 insertions(+)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 7739e00..07330e1 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -124,6 +124,7 @@ typedef struct CPUClass {
>                              int cpuid, void *opaque);
>      int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
>                                  void *opaque);
> +    void (*parse_options)(CPUState *cpu, Error **errp);
>  
>      const struct VMStateDescription *vmsd;
>      int gdb_num_core_regs;
> @@ -327,6 +328,46 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState 
> *cpu, vaddr addr)
>  #endif
>  
>  /**
> + * cpu_parse_options:
> + * @cpu: The CPU to set options for.
> + *
> + * Parses CPU options if the CPU class has a custom handler defined.
> + *
> + * Returns: -1 if a custom handler is defined and failed, 0 otherwise.
> + */
> +static inline int cpu_parse_options(CPUState *cpu)
why not pass up the stack errp, vs returning -1?

And of cause, there should be a patch that demonstrates an actual user
of  cpu_parse_options().

> +{
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> +    Error *err = NULL;
> +
> +    if (cc->parse_options) {
> +        cc->parse_options(cpu, &err);
> +        if (err) {
> +            return -1;
> +        }
> +    }
> +
> +    /* No callback, let arch do it the old way */
> +    return 0;
> +}
> +
> +/**
> + * cpu_default_parse_options_func:
> + * The default handler for CPUClass::parse_options
> + * @cpu: the CPU to set option for.
> + * @errp: the handling error descriptor.
> + */
> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
where is it used?
Maybe it should be static inside of qom/cpu.c
and assigned in cpu_class_init() for TYPE_CPU.



> +
> +/**
> + * cpu_get_opt:
> + * @name: The parameter name
> + *
> + * Returns: a CPU parameter value.
> + */
> +const char *cpu_opt_get(const char *name);
> +
> +/**
>   * cpu_reset:
>   * @cpu: The CPU whose state is to be reset.
>   */
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 818fb26..fb95cb4 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -24,6 +24,8 @@
>  #include "qemu/notify.h"
>  #include "qemu/log.h"
>  #include "sysemu/sysemu.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qemu/config-file.h"
>  
>  bool cpu_exists(int64_t id)
>  {
> @@ -273,3 +275,50 @@ static void cpu_register_types(void)
>  }
>  
>  type_init(cpu_register_types)
> +
> +static int cpu_set_property(const char *name, const char *value, void 
> *opaque)
> +{
> +    Error *err = NULL;
> +
> +    if (strcmp(name, "type") == 0) {
> +        return 0;
> +    }
> +
> +    object_property_parse(opaque, value, name, &err);
> +    if (err != NULL) {
> +        qerror_report_err(err);
> +        error_free(err);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static QemuOpts *cpu_get_opts(void)
> +{
> +    QemuOptsList *list;
> +
> +    list = qemu_find_opts("cpu");
> +    assert(list);
> +    return qemu_opts_find(list, NULL);
> +}
> +
> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp)
> +{
> +    QemuOpts *opts = cpu_get_opts();
> +
> +    if (opts && qemu_opt_foreach(opts, cpu_set_property, cpu, 1)) {
> +        error_setg(errp, "Bad option");
> +    }
> +}
> +
> +const char *cpu_opt_get(const char *name)
> +{
> +    QemuOpts *opts = cpu_get_opts();
> +
> +    if (!opts) {
> +        return NULL;
> +    }
> +
> +    return qemu_opt_get(cpu_get_opts(), name);
> +}
> diff --git a/vl.c b/vl.c
> index 8d5d874..f7c4c0a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -433,6 +433,16 @@ static QemuOptsList qemu_machine_opts = {
>      },
>  };
>  
> +static QemuOptsList qemu_cpu_opts = {
> +    .name = "cpu",
> +    .implied_opt_name = "type",
> +    .merge_lists = true,
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
> +    .desc = {
> +        { /* End of list */ }
> +    },
> +};
> +
>  static QemuOptsList qemu_boot_opts = {
>      .name = "boot-opts",
>      .implied_opt_name = "order",
> @@ -2880,6 +2890,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_trace_opts);
>      qemu_add_opts(&qemu_option_rom_opts);
>      qemu_add_opts(&qemu_machine_opts);
> +    qemu_add_opts(&qemu_cpu_opts);
>      qemu_add_opts(&qemu_smp_opts);
>      qemu_add_opts(&qemu_boot_opts);
>      qemu_add_opts(&qemu_sandbox_opts);
> @@ -2975,7 +2986,19 @@ int main(int argc, char **argv, char **envp)
>              }
>              case QEMU_OPTION_cpu:
>                  /* hw initialization will check this */
> +
> +                /* Store cpu_model for old style parser */
>                  cpu_model = optarg;
> +
> +                /*
> +                 * Parse and save options in the cpu options list
> +                 * for a new parser
> +                 */
> +                olist = qemu_find_opts("cpu");
> +                opts = qemu_opts_parse(olist, optarg, 1);
> +                if (!opts) {
> +                    exit(1);
> +                }
>                  break;
>              case QEMU_OPTION_hda:
>                  {