| Message ID | 20231114235628.534334-25-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Unified CPU type check | expand |
On 11/14/23 15:56, Gavin Shan wrote: > QEMU will be terminated if the specified CPU type isn't supported > in machine_run_board_init(). The list of supported CPU type names > is tracked by mc->valid_cpu_types. > > The error handling can be used to propagate error messages, to be > consistent how the errors are handled for other situations in the > same function. > > No functional change intended. > > Suggested-by: Igor Mammedov <imammedo@redhat.com> > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > hw/core/machine.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 0c17398141..5b45dbbbd5 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -1394,6 +1394,7 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error * > MachineClass *machine_class = MACHINE_GET_CLASS(machine); > ObjectClass *oc = object_class_by_name(machine->cpu_type); > CPUClass *cc; > + Error *local_err = NULL; There is no need for local_error; just use errp throughout. With that, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > if (!machine_class->valid_cpu_types[i]) { > /* The user specified CPU is not valid */ > - error_report("Invalid CPU type: %s", machine->cpu_type); > - error_printf("The valid types are: %s", > - machine_class->valid_cpu_types[0]); > + error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type); > + error_append_hint(&local_err, "The valid types are: %s", > + machine_class->valid_cpu_types[0]); > for (i = 1; machine_class->valid_cpu_types[i]; i++) { > - error_printf(", %s", machine_class->valid_cpu_types[i]); > + error_append_hint(&local_err, ", %s", > + machine_class->valid_cpu_types[i]); > } > - error_printf("\n"); > + error_append_hint(&local_err, "\n"); > > - exit(1); > + error_propagate(errp, local_err); > } > } >
On 11/14/23 17:21, Richard Henderson wrote: > On 11/14/23 15:56, Gavin Shan wrote: >> QEMU will be terminated if the specified CPU type isn't supported >> in machine_run_board_init(). The list of supported CPU type names >> is tracked by mc->valid_cpu_types. >> >> The error handling can be used to propagate error messages, to be >> consistent how the errors are handled for other situations in the >> same function. >> >> No functional change intended. >> >> Suggested-by: Igor Mammedov <imammedo@redhat.com> >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> --- >> hw/core/machine.c | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index 0c17398141..5b45dbbbd5 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -1394,6 +1394,7 @@ void machine_run_board_init(MachineState *machine, const char >> *mem_path, Error * >> MachineClass *machine_class = MACHINE_GET_CLASS(machine); >> ObjectClass *oc = object_class_by_name(machine->cpu_type); >> CPUClass *cc; >> + Error *local_err = NULL; > > > There is no need for local_error; just use errp throughout. > > With that, > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Alternately, is this because passing &error_fatal will abort on the first error_setg, without all the hints? In which case you can move local_error into the inner block and add a comment to that effect. r~
diff --git a/hw/core/machine.c b/hw/core/machine.c index 0c17398141..5b45dbbbd5 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -1394,6 +1394,7 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error * MachineClass *machine_class = MACHINE_GET_CLASS(machine); ObjectClass *oc = object_class_by_name(machine->cpu_type); CPUClass *cc; + Error *local_err = NULL; /* This checkpoint is required by replay to separate prior clock reading from the other reads, because timer polling functions query @@ -1466,15 +1467,16 @@ void machine_run_board_init(MachineState *machine, const char *mem_path, Error * if (!machine_class->valid_cpu_types[i]) { /* The user specified CPU is not valid */ - error_report("Invalid CPU type: %s", machine->cpu_type); - error_printf("The valid types are: %s", - machine_class->valid_cpu_types[0]); + error_setg(&local_err, "Invalid CPU type: %s", machine->cpu_type); + error_append_hint(&local_err, "The valid types are: %s", + machine_class->valid_cpu_types[0]); for (i = 1; machine_class->valid_cpu_types[i]; i++) { - error_printf(", %s", machine_class->valid_cpu_types[i]); + error_append_hint(&local_err, ", %s", + machine_class->valid_cpu_types[i]); } - error_printf("\n"); + error_append_hint(&local_err, "\n"); - exit(1); + error_propagate(errp, local_err); } }
QEMU will be terminated if the specified CPU type isn't supported in machine_run_board_init(). The list of supported CPU type names is tracked by mc->valid_cpu_types. The error handling can be used to propagate error messages, to be consistent how the errors are handled for other situations in the same function. No functional change intended. Suggested-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Gavin Shan <gshan@redhat.com> --- hw/core/machine.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)