| Message ID | 20231126231210.112820-2-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Unified CPU type check | expand |
Hi Gavin, On 27/11/23 00:12, 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> > --- > v7: Add 'return' after error_propagate() to avoid calling into > mc->init() in the failing case (Marcin) > --- > hw/core/machine.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 0c17398141..b3ef325936 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; IIUC the big comment in "include/qapi/error.h", since commit ae7c80a7bd ("error: New macro ERRP_GUARD()") we want to use the new macro instead. > /* This checkpoint is required by replay to separate prior clock > reading from the other reads, because timer polling functions query > @@ -1466,15 +1467,17 @@ 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); > + return; > } > } >
Hi Phil, On 11/28/23 20:43, Philippe Mathieu-Daudé wrote: > On 27/11/23 00:12, 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> >> --- >> v7: Add 'return' after error_propagate() to avoid calling into >> mc->init() in the failing case (Marcin) >> --- >> hw/core/machine.c | 15 +++++++++------ >> 1 file changed, 9 insertions(+), 6 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index 0c17398141..b3ef325936 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; > > IIUC the big comment in "include/qapi/error.h", since commit ae7c80a7bd > ("error: New macro ERRP_GUARD()") we want to use the new macro instead. > Yeah, there already has a ERRP_GUARD() in machine_run_board_init(). After rechecking the implementation of ERRP_GUARD(), I found @local_err needs to be dropped and use @errp below. >> /* This checkpoint is required by replay to separate prior clock >> reading from the other reads, because timer polling functions query >> @@ -1466,15 +1467,17 @@ 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); >> + return; >> } >> } @local_err needs to be replaced with @errp in this chunk of code, as mentioned above. Thanks, Gavin
diff --git a/hw/core/machine.c b/hw/core/machine.c index 0c17398141..b3ef325936 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,17 @@ 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); + return; } }
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> --- v7: Add 'return' after error_propagate() to avoid calling into mc->init() in the failing case (Marcin) --- hw/core/machine.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)