| Message ID | 20221106085115.257018-13-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/12] util/main-loop: Fix maximum number of wait objects for win32 | expand |
Hi all, and thanks Paolo for taking care of this series, I think I noticed something that seems off: On 11/6/22 09:51, Paolo Bonzini wrote: > From: Claudio Fontana <cfontana@suse.de> > > if QEMU is configured with modules enabled, it is possible that the > load of an accelerator module will fail. > Exit in this case, relying on module_object_class_by_name to report > the specific load error if any. > > Signed-off-by: Claudio Fontana <cfontana@suse.de> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > [claudio: changed abort() to exit(1)] > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Message-Id: <20220929093035.4231-6-cfontana@suse.de> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > accel/accel-softmmu.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c > index 67276e4f5222..f9cdafb148ac 100644 > --- a/accel/accel-softmmu.c > +++ b/accel/accel-softmmu.c > @@ -66,6 +66,7 @@ void accel_init_ops_interfaces(AccelClass *ac) > { > const char *ac_name; > char *ops_name; > + ObjectClass *oc; > AccelOpsClass *ops; > > ac_name = object_class_get_name(OBJECT_CLASS(ac)); > @@ -73,8 +74,13 @@ void accel_init_ops_interfaces(AccelClass *ac) > > ops_name = g_strdup_printf("%s" ACCEL_OPS_SUFFIX, ac_name); > ops = ACCEL_OPS_CLASS(module_object_class_by_name(ops_name)); I think this last line should be removed. I somehow left it there by mistake. Not sure if it hurts, but we assign to "ops" here, only to reassign to the same variable... I > + oc = module_object_class_by_name(ops_name); > + if (!oc) { > + error_report("fatal: could not load module for type '%s'", ops_name); > + exit(1); > + } > g_free(ops_name); > - > + ops = ACCEL_OPS_CLASS(oc); here. Do I see it right? > /* > * all accelerators need to define ops, providing at least a mandatory > * non-NULL create_vcpu_thread operation. So I suggest: - ops = ACCEL_OPS_CLASS(module_object_class_by_name(ops_name)); Thanks and sorry for the oversight, Claudio
diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c index 67276e4f5222..f9cdafb148ac 100644 --- a/accel/accel-softmmu.c +++ b/accel/accel-softmmu.c @@ -66,6 +66,7 @@ void accel_init_ops_interfaces(AccelClass *ac) { const char *ac_name; char *ops_name; + ObjectClass *oc; AccelOpsClass *ops; ac_name = object_class_get_name(OBJECT_CLASS(ac)); @@ -73,8 +74,13 @@ void accel_init_ops_interfaces(AccelClass *ac) ops_name = g_strdup_printf("%s" ACCEL_OPS_SUFFIX, ac_name); ops = ACCEL_OPS_CLASS(module_object_class_by_name(ops_name)); + oc = module_object_class_by_name(ops_name); + if (!oc) { + error_report("fatal: could not load module for type '%s'", ops_name); + exit(1); + } g_free(ops_name); - + ops = ACCEL_OPS_CLASS(oc); /* * all accelerators need to define ops, providing at least a mandatory * non-NULL create_vcpu_thread operation.