Re: [Qemu-devel] [PATCH v1 2/5] Convert remaining error_report() to warn_report()

[Alistair Francis]

On Mon, Aug 14, 2017 at 4:45 AM, Markus Armbruster <address@hidden> wrote:
> Alistair Francis <address@hidden> writes:
>
>> In a previous patch (3dc6f8693694a649a9c83f1e2746565b47683923) we
>> converted uses of error_report("warning:"... to use warn_report()
>> instead. This was to help standardise on a single method of printing
>> warnings to the user.
>>
>> There appears to have been some cases that slipped through in patch sets
>> applied around the same time, this patch catches the few remaining
>> cases.
>>
>> All of the warnings were changed using this command:
>>   find ./* -type f -exec sed -i \
>>     's|error_report(".*warning[,:] |warn_report("|Ig' {} +
>>
>> Indentation fixed up manually afterwards.
>>
>> Signed-off-by: Alistair Francis <address@hidden>
>> Cc: Kevin Wolf <address@hidden>
>> Cc: Max Reitz <address@hidden>
>> Cc: Christian Borntraeger <address@hidden>
>> Cc: Cornelia Huck <address@hidden>
>> Cc: Alexander Graf <address@hidden>
>> Cc: Richard Henderson <address@hidden>
>> Cc: Stefan Hajnoczi <address@hidden>
>> ---
>>
>>  block/qcow2.c      | 10 +++++-----
>>  target/s390x/kvm.c |  4 ++--
>>  trace/control.c    |  4 ++--
>>  3 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 90efa4477b..e6c6be0822 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -302,11 +302,11 @@ static int qcow2_read_extensions(BlockDriverState *bs, 
>> uint64_t start_offset,
>>              }
>>
>>              if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS)) {
>> -                error_report("WARNING: a program lacking bitmap support "
>> -                             "modified this file, so all bitmaps are now "
>> -                             "considered inconsistent. Some clusters may be 
>> "
>> -                             "leaked, run 'qemu-img check -r' on the image "
>> -                             "file to fix.");
>> +                warn_report("a program lacking bitmap support "
>> +                            "modified this file, so all bitmaps are now "
>> +                            "considered inconsistent. Some clusters may be "
>> +                            "leaked, run 'qemu-img check -r' on the image "
>> +                            "file to fix.");
>
> This message is awfully long.  What about splitting it while we're there:
>
>                    warn_report("a program lacking bitmap support "
>                                "modified this file, so all bitmaps are now "
>                                "considered inconsistent");
>                    error_printf("Some clusters may be "leaked, "
>                                 "run 'qemu-img check -r' on the image "
>                                 "file to fix.");
>
>>                  if (need_update_header != NULL) {
>>                      /* Updating is needed to drop invalid bitmap extension. 
>> */
>>                      *need_update_header = true;
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index c4c5791d27..1084923adb 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -222,8 +222,8 @@ static void kvm_s390_enable_cmma(void)
>>      };
>>
>>      if (mem_path) {
>> -        error_report("Warning: CMM will not be enabled because it is not "
>> -                     "compatible to hugetlbfs.");
>> +        warn_report("CMM will not be enabled because it is not "
>> +                    "compatible to hugetlbfs.");
>
> Not a native speaker, but here goes anyway: "compatible with".
>
>>          return;
>>      }
>>      rc = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
>> diff --git a/trace/control.c b/trace/control.c
>> index 82d8989c4d..2769934bec 100644
>> --- a/trace/control.c
>> +++ b/trace/control.c
>> @@ -72,8 +72,8 @@ void trace_event_register_group(TraceEvent **events)
>>          if (likely(next_vcpu_id < CPU_TRACE_DSTATE_MAX_EVENTS)) {
>>              events[i]->vcpu_id = next_vcpu_id++;
>>          } else {
>> -            error_report("WARNING: too many vcpu trace events; dropping 
>> '%s'",
>> -                         events[i]->name);
>> +            warn_report("too many vcpu trace events; dropping '%s'",
>> +                        events[i]->name);
>>          }
>>      }
>>      event_groups = g_renew(TraceEventGroup, event_groups, nevent_groups + 
>> 1);
>
> The nits I picked predate your patch, so:
> Reviewed-by: Markus Armbruster <address@hidden>

Thanks, I fixed up the changes you mentioned as well.

Thanks,
Alistair