Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> writes: > Hi Ani, > > On 9/8/24 08:49, Ani Sinha wrote: >> error_report() is more appropriate for error situations. Replace fprintf with >> error_report. Cosmetic. No functional change. >> CC: qemu-trivial@xxxxxxxxxx >> CC: zhao1.liu@xxxxxxxxx > > (Pointless to carry Cc line when patch is already reviewed next line) > >> Reviewed-by: Zhao Liu <zhao1.liu@xxxxxxxxx> >> Signed-off-by: Ani Sinha <anisinha@xxxxxxxxxx> >> --- >> accel/kvm/kvm-all.c | 40 ++++++++++++++++++---------------------- >> 1 file changed, 18 insertions(+), 22 deletions(-) >> changelog: >> v2: fix a bug. >> v3: replace one instance of error_report() with error_printf(). added tags. >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >> index 75d11a07b2..5bc9d35b61 100644 >> --- a/accel/kvm/kvm-all.c >> +++ b/accel/kvm/kvm-all.c >> @@ -2427,7 +2427,7 @@ static int kvm_init(MachineState *ms) >> QLIST_INIT(&s->kvm_parked_vcpus); >> s->fd = qemu_open_old(s->device ?: "/dev/kvm", O_RDWR); >> if (s->fd == -1) { >> - fprintf(stderr, "Could not access KVM kernel module: %m\n"); >> + error_report("Could not access KVM kernel module: %m"); >> ret = -errno; >> goto err; >> } >> @@ -2437,13 +2437,13 @@ static int kvm_init(MachineState *ms) >> if (ret >= 0) { >> ret = -EINVAL; >> } >> - fprintf(stderr, "kvm version too old\n"); >> + error_report("kvm version too old"); >> goto err; >> } >> if (ret > KVM_API_VERSION) { >> ret = -EINVAL; >> - fprintf(stderr, "kvm version not supported\n"); >> + error_report("kvm version not supported"); >> goto err; >> } >> @@ -2488,26 +2488,22 @@ static int kvm_init(MachineState *ms) >> } while (ret == -EINTR); >> if (ret < 0) { >> - fprintf(stderr, "ioctl(KVM_CREATE_VM) failed: %d %s\n", -ret, >> - strerror(-ret)); >> + error_report("ioctl(KVM_CREATE_VM) failed: %d %s", -ret, >> + strerror(-ret)); >> #ifdef TARGET_S390X >> if (ret == -EINVAL) { >> - fprintf(stderr, >> - "Host kernel setup problem detected. Please verify:\n"); >> - fprintf(stderr, "- for kernels supporting the switch_amode or" >> - " user_mode parameters, whether\n"); >> - fprintf(stderr, >> - " user space is running in primary address space\n"); >> - fprintf(stderr, >> - "- for kernels supporting the vm.allocate_pgste sysctl, " >> - "whether it is enabled\n"); >> + error_report("Host kernel setup problem detected. > > \n" > > Should we use error_printf_unless_qmp() for the following? > > " Please verify:"); >> + error_report("- for kernels supporting the switch_amode or" >> + " user_mode parameters, whether"); >> + error_report(" user space is running in primary address space"); >> + error_report("- for kernels supporting the vm.allocate_pgste " >> + "sysctl, whether it is enabled"); Do not put newlines into error messages. error_report()'s function comment demands "The resulting message should be a single phrase, with no newline or trailing punctuation." You can do this: error_report(... the actual error message ...); error_printf(... hints on what to do about it ...); Questions? [...]