Re: [PATCH v2 01/11] accel/kvm: Check MachineClass kvm_type() return value

[Cornelia Huck <cohuck@xxxxxxxxxx>]

On Mon, 22 Feb 2021 18:41:07 +0100
Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> wrote:

> On 2/22/21 6:24 PM, Cornelia Huck wrote:
> > On Fri, 19 Feb 2021 18:38:37 +0100
> > Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> wrote:
> >   
> >> MachineClass::kvm_type() can return -1 on failure.
> >> Document it, and add a check in kvm_init().
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> >> ---
> >>  include/hw/boards.h | 3 ++-
> >>  accel/kvm/kvm-all.c | 6 ++++++
> >>  2 files changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/hw/boards.h b/include/hw/boards.h
> >> index a46dfe5d1a6..68d3d10f6b0 100644
> >> --- a/include/hw/boards.h
> >> +++ b/include/hw/boards.h
> >> @@ -127,7 +127,8 @@ typedef struct {
> >>   *    implement and a stub device is required.
> >>   * @kvm_type:
> >>   *    Return the type of KVM corresponding to the kvm-type string option 
> >> or
> >> - *    computed based on other criteria such as the host kernel 
> >> capabilities.
> >> + *    computed based on other criteria such as the host kernel 
> >> capabilities
> >> + *    (which can't be negative), or -1 on error.
> >>   * @numa_mem_supported:
> >>   *    true if '--numa node.mem' option is supported and false otherwise
> >>   * @smp_parse:
> >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> >> index 84c943fcdb2..b069938d881 100644
> >> --- a/accel/kvm/kvm-all.c
> >> +++ b/accel/kvm/kvm-all.c
> >> @@ -2057,6 +2057,12 @@ static int kvm_init(MachineState *ms)
> >>                                                              "kvm-type",
> >>                                                              &error_abort);
> >>          type = mc->kvm_type(ms, kvm_type);
> >> +        if (type < 0) {
> >> +            ret = -EINVAL;
> >> +            fprintf(stderr, "Failed to detect kvm-type for machine 
> >> '%s'\n",
> >> +                    mc->name);
> >> +            goto err;
> >> +        }
> >>      }
> >>  
> >>      do {  
> > 
> > No objection to this patch; but I'm wondering why some non-pseries
> > machines implement the kvm_type callback, when I see the kvm-type
> > property only for pseries? Am I holding my git grep wrong?  
> 
> Can it be what David commented here?
> https://www.mail-archive.com/qemu-devel@xxxxxxxxxx/msg784508.html
> 

Ok, I might be confused about the other ppc machines; but I'm wondering
about the kvm_type callback for mips and arm/virt. Maybe I'm just
confused by the whole mechanism?