| Message ID | 20210219173847.2054123-2-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/accel: Exit gracefully when accelerator is invalid | expand |
On Fri, 19 Feb 2021 18:38:37 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> > --- > 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?
On 2/22/21 6:24 PM, Cornelia Huck wrote: > On Fri, 19 Feb 2021 18:38:37 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> >> --- >> 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@nongnu.org/msg784508.html
On Mon, 22 Feb 2021 18:41:07 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 2/22/21 6:24 PM, Cornelia Huck wrote: > > On Fri, 19 Feb 2021 18:38:37 +0100 > > Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> > >> --- > >> 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@nongnu.org/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?
On 2/22/21 6:50 PM, Cornelia Huck wrote: > On Mon, 22 Feb 2021 18:41:07 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> On 2/22/21 6:24 PM, Cornelia Huck wrote: >>> On Fri, 19 Feb 2021 18:38:37 +0100 >>> Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> >>>> --- >>>> 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@nongnu.org/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? For MIPS see https://www.linux-kvm.org/images/f/f2/01x08a-MIPS.pdf and Jiaxun comment here: https://lore.kernel.org/linux-mips/a2a2cfe3-5618-43b1-a6a4-cc768fc1b9fb@www.fastmail.com/ TE KVM: Trap-and-Emul guest kernel VZ KVM: HW Virtualized For "the whole mechanism" I'll defer to Paolo =)
On Mon, Feb 22, 2021 at 06:50:44PM +0100, Cornelia Huck wrote: > On Mon, 22 Feb 2021 18:41:07 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > On 2/22/21 6:24 PM, Cornelia Huck wrote: > > > On Fri, 19 Feb 2021 18:38:37 +0100 > > > Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> > > >> --- > > >> 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@nongnu.org/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? For ppc at least, not sure about in general, pseries is the only machine type that can possibly work under more than one KVM flavour (HV or PR). So, it's the only one where it's actually useful to be able to configure this.
On Tue, Feb 23, 2021 at 10:33:55AM +1100, David Gibson wrote: > On Mon, Feb 22, 2021 at 06:50:44PM +0100, Cornelia Huck wrote: > > On Mon, 22 Feb 2021 18:41:07 +0100 > > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > > > On 2/22/21 6:24 PM, Cornelia Huck wrote: > > > > On Fri, 19 Feb 2021 18:38:37 +0100 > > > > Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> > > > >> --- > > > >> 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@nongnu.org/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? > > For ppc at least, not sure about in general, pseries is the only > machine type that can possibly work under more than one KVM flavour > (HV or PR). So, it's the only one where it's actually useful to be > able to configure this. Wait... I'm not sure that's true. At least theoretically, some of the Book3E platforms could work with either PR or the Book3E specific KVM. Not sure if KVM PR supports all the BookE instructions it would need to in practice. Possibly pseries is just the platform where there's been enough people interested in setting the KVM flavour so far.
On Tue, 23 Feb 2021 10:37:01 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > On Tue, Feb 23, 2021 at 10:33:55AM +1100, David Gibson wrote: > > On Mon, Feb 22, 2021 at 06:50:44PM +0100, Cornelia Huck wrote: > > > On Mon, 22 Feb 2021 18:41:07 +0100 > > > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > > > > > On 2/22/21 6:24 PM, Cornelia Huck wrote: > > > > > On Fri, 19 Feb 2021 18:38:37 +0100 > > > > > Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> > > > > >> --- > > > > >> 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@nongnu.org/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? > > > > For ppc at least, not sure about in general, pseries is the only > > machine type that can possibly work under more than one KVM flavour > > (HV or PR). So, it's the only one where it's actually useful to be > > able to configure this. > > Wait... I'm not sure that's true. At least theoretically, some of the > Book3E platforms could work with either PR or the Book3E specific > KVM. Not sure if KVM PR supports all the BookE instructions it would > need to in practice. > > Possibly pseries is just the platform where there's been enough people > interested in setting the KVM flavour so far. If I'm not utterly confused by the code, it seems the pseries machines are the only ones where you can actually get to an invocation of ->kvm_type(): You need to have a 'kvm-type' machine property, and AFAICS only the pseries machine has that. (Or is something hiding behind some macro magic?)
On 2/23/21 11:36 AM, Cornelia Huck wrote: > On Tue, 23 Feb 2021 10:37:01 +1100 > David Gibson <david@gibson.dropbear.id.au> wrote: > >> On Tue, Feb 23, 2021 at 10:33:55AM +1100, David Gibson wrote: >>> On Mon, Feb 22, 2021 at 06:50:44PM +0100, Cornelia Huck wrote: >>>> On Mon, 22 Feb 2021 18:41:07 +0100 >>>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >>>> >>>>> On 2/22/21 6:24 PM, Cornelia Huck wrote: >>>>>> On Fri, 19 Feb 2021 18:38:37 +0100 >>>>>> Philippe Mathieu-Daudé <philmd@redhat.com> 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@redhat.com> >>>>>>> --- >>>>>>> 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@nongnu.org/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? >>> >>> For ppc at least, not sure about in general, pseries is the only >>> machine type that can possibly work under more than one KVM flavour >>> (HV or PR). So, it's the only one where it's actually useful to be >>> able to configure this. >> >> Wait... I'm not sure that's true. At least theoretically, some of the >> Book3E platforms could work with either PR or the Book3E specific >> KVM. Not sure if KVM PR supports all the BookE instructions it would >> need to in practice. >> >> Possibly pseries is just the platform where there's been enough people >> interested in setting the KVM flavour so far. > > If I'm not utterly confused by the code, it seems the pseries machines > are the only ones where you can actually get to an invocation of > ->kvm_type(): You need to have a 'kvm-type' machine property, and > AFAICS only the pseries machine has that. OMG you are right... This changed in commit f2ce39b4f06 ("vl: make qemu_get_machine_opts static"): @@ -2069,13 +2068,11 @@ static int kvm_init(MachineState *ms) } s->as = g_new0(struct KVMAs, s->nr_as); - kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type"); - if (mc->kvm_type) { + if (object_property_find(OBJECT(current_machine), "kvm-type")) { + g_autofree char *kvm_type = object_property_get_str(OBJECT(current_machine), + "kvm-type", + &error_abort); type = mc->kvm_type(ms, kvm_type); - } else if (kvm_type) { - ret = -EINVAL; - fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type); - goto err; } Paolo, is that expected? So these callbacks are dead code: hw/arm/virt.c:2585: mc->kvm_type = virt_kvm_type; hw/mips/loongson3_virt.c:625: mc->kvm_type = mips_kvm_type; hw/ppc/mac_newworld.c:598: mc->kvm_type = core99_kvm_type; hw/ppc/mac_oldworld.c:447: mc->kvm_type = heathrow_kvm_type; > > (Or is something hiding behind some macro magic?) >
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 {
MachineClass::kvm_type() can return -1 on failure. Document it, and add a check in kvm_init(). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/boards.h | 3 ++- accel/kvm/kvm-all.c | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-)