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

[Markus Armbruster]

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>