Re: [Qemu-devel] [PATCH v6 06/18] hw/boards: Add a MachineState parameter to kvm_type callback

[Peter Maydell]

On Tue, 5 Feb 2019 at 17:33, Eric Auger <address@hidden> wrote:
>
> On ARM, the kvm_type will be resolved by querying the KVMState.
> Let's add the MachineState handle to the callback so that we
> can retrieve the  KVMState handle. in kvm_init, when the callback
> is called, the kvm_state variable is not yet set.
>
> Signed-off-by: Eric Auger <address@hidden>
> Acked-by: David Gibson <address@hidden>
> [ppc parts]
> ---
>  accel/kvm/kvm-all.c   | 2 +-
>  hw/ppc/mac_newworld.c | 3 +--
>  hw/ppc/mac_oldworld.c | 2 +-
>  hw/ppc/spapr.c        | 2 +-
>  include/hw/boards.h   | 2 +-
>  5 files changed, 5 insertions(+), 6 deletions(-)
>


> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 02f114085f..425d2c86a6 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -171,7 +171,7 @@ struct MachineClass {
>      void (*init)(MachineState *state);
>      void (*reset)(void);
>      void (*hot_add_cpu)(const int64_t id, Error **errp);
> -    int (*kvm_type)(const char *arg);
> +    int (*kvm_type)(MachineState *ms, const char *arg);
>
>      BlockInterfaceType block_default_type;
>      int units_per_default_bus;
> --

Can you add a line to the struct's documentation comment for the
@kvm_type field, please ?

We're rather inconsistent about what we name the MachineState*
parameter in methods here:
 "state" x 1   (init)
 "machine" x 3 (get_hotplug_handler, cpu_index_to_instance_props,
                possible_cpu_arch_ids)
 "ms" x 1 (get_default_cpu_node_id)

It would probably be better to follow the most common option
rather than one of the rarer ones.

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM