Re: [Qemu-ppc] [PATCH 2/3] hw/ppc: use error_report instead of fprintf

[Cédric Le Goater]

On 07/26/2016 07:58 AM, David Gibson wrote:
> On Mon, Jul 25, 2016 at 04:24:42PM +0200, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> So, when I did some previous conversions to error_report(), I left out
> the DT code, in the hopes I'd obsolete it with a more thorough
> cleanup.  Since that won't happen for a while yet, I'm happy enough to
> queue this for 2.8.
> 
> However, if you're going to change all these instances, you should
> probably change the one in _FDT as well.

Indeed :) 

I will resend this one then.

Thanks,

C. 


>> ---
>>  hw/ppc/spapr.c       | 12 ++++++------
>>  hw/ppc/spapr_drc.c   |  8 ++++----
>>  hw/ppc/spapr_iommu.c |  4 ++--
>>  hw/ppc/spapr_rtas.c  | 13 +++++++------
>>  hw/ppc/spapr_vio.c   |  3 ++-
>>  5 files changed, 21 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 538ff5a46768..4d073ce39e87 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -647,13 +647,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
>> *fdt, int offset,
>>          _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size",
>>                                 pcc->l1_dcache_size)));
>>      } else {
>> -        fprintf(stderr, "Warning: Unknown L1 dcache size for cpu\n");
>> +        error_report("Warning: Unknown L1 dcache size for cpu");
>>      }
>>      if (pcc->l1_icache_size) {
>>          _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size",
>>                                 pcc->l1_icache_size)));
>>      } else {
>> -        fprintf(stderr, "Warning: Unknown L1 icache size for cpu\n");
>> +        error_report("Warning: Unknown L1 icache size for cpu");
>>      }
>>  
>>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>> @@ -944,20 +944,20 @@ static void spapr_finalize_fdt(sPAPRMachineState 
>> *spapr,
>>  
>>      ret = spapr_populate_memory(spapr, fdt);
>>      if (ret < 0) {
>> -        fprintf(stderr, "couldn't setup memory nodes in fdt\n");
>> +        error_report("couldn't setup memory nodes in fdt");
>>          exit(1);
>>      }
>>  
>>      ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
>>      if (ret < 0) {
>> -        fprintf(stderr, "couldn't setup vio devices in fdt\n");
>> +        error_report("couldn't setup vio devices in fdt");
>>          exit(1);
>>      }
>>  
>>      if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
>>          ret = spapr_rng_populate_dt(fdt);
>>          if (ret < 0) {
>> -            fprintf(stderr, "could not set up rng device in the fdt\n");
>> +            error_report("could not set up rng device in the fdt");
>>              exit(1);
>>          }
>>      }
>> @@ -973,7 +973,7 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
>>      /* RTAS */
>>      ret = spapr_rtas_device_tree_setup(fdt, rtas_addr, rtas_size);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't set up RTAS device tree properties\n");
>> +        error_report("Couldn't set up RTAS device tree properties");
>>      }
>>  
>>      /* cpus */
>> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
>> index 26a067951c54..4b1a943b8e4d 100644
>> --- a/hw/ppc/spapr_drc.c
>> +++ b/hw/ppc/spapr_drc.c
>> @@ -816,7 +816,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, 
>> Object *owner,
>>                        drc_indexes->data,
>>                        drc_indexes->len * sizeof(uint32_t));
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't create ibm,drc-indexes property\n");
>> +        error_report("Couldn't create ibm,drc-indexes property");
>>          goto out;
>>      }
>>  
>> @@ -824,21 +824,21 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, 
>> Object *owner,
>>                        drc_power_domains->data,
>>                        drc_power_domains->len * sizeof(uint32_t));
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't finalize ibm,drc-power-domains 
>> property\n");
>> +        error_report("Couldn't finalize ibm,drc-power-domains property");
>>          goto out;
>>      }
>>  
>>      ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-names",
>>                        drc_names->str, drc_names->len);
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't finalize ibm,drc-names property\n");
>> +        error_report("Couldn't finalize ibm,drc-names property");
>>          goto out;
>>      }
>>  
>>      ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-types",
>>                        drc_types->str, drc_types->len);
>>      if (ret) {
>> -        fprintf(stderr, "Couldn't finalize ibm,drc-types property\n");
>> +        error_report("Couldn't finalize ibm,drc-types property");
>>          goto out;
>>      }
>>  
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index d57b05d5c0f1..53c2cd4fb657 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -310,8 +310,8 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, 
>> uint32_t liobn)
>>      char tmp[32];
>>  
>>      if (spapr_tce_find_by_liobn(liobn)) {
>> -        fprintf(stderr, "Attempted to create TCE table with duplicate"
>> -                " LIOBN 0x%x\n", liobn);
>> +        error_report("Attempted to create TCE table with duplicate"
>> +                " LIOBN 0x%x", liobn);
>>          return NULL;
>>      }
>>  
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index dc058e512b86..27b5ad4bc437 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -27,6 +27,7 @@
>>  #include "qemu/osdep.h"
>>  #include "cpu.h"
>>  #include "qemu/log.h"
>> +#include "qemu/error-report.h"
>>  #include "sysemu/sysemu.h"
>>  #include "sysemu/char.h"
>>  #include "hw/qdev.h"
>> @@ -716,7 +717,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
>> rtas_addr,
>>  
>>      ret = fdt_add_mem_rsv(fdt, rtas_addr, rtas_size);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add RTAS reserve entry: %s\n",
>> +        error_report("Couldn't add RTAS reserve entry: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -724,7 +725,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
>> rtas_addr,
>>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
>>                                  rtas_addr);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add linux,rtas-base property: %s\n",
>> +        error_report("Couldn't add linux,rtas-base property: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -732,7 +733,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
>> rtas_addr,
>>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
>>                                  rtas_addr);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n",
>> +        error_report("Couldn't add linux,rtas-entry property: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -740,7 +741,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
>> rtas_addr,
>>      ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
>>                                  rtas_size);
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add rtas-size property: %s\n",
>> +        error_report("Couldn't add rtas-size property: %s",
>>                  fdt_strerror(ret));
>>          return ret;
>>      }
>> @@ -755,7 +756,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
>> rtas_addr,
>>          ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
>>                                      i + RTAS_TOKEN_BASE);
>>          if (ret < 0) {
>> -            fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
>> +            error_report("Couldn't add rtas token for %s: %s",
>>                      call->name, fdt_strerror(ret));
>>              return ret;
>>          }
>> @@ -770,7 +771,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
>> rtas_addr,
>>      ret = qemu_fdt_setprop(fdt, "/rtas", "ibm,lrdr-capacity", lrdr_capacity,
>>                       sizeof(lrdr_capacity));
>>      if (ret < 0) {
>> -        fprintf(stderr, "Couldn't add ibm,lrdr-capacity rtas property\n");
>> +        error_report("Couldn't add ibm,lrdr-capacity rtas property");
>>          return ret;
>>      }
>>  
>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
>> index f93244d7c182..497028f075e3 100644
>> --- a/hw/ppc/spapr_vio.c
>> +++ b/hw/ppc/spapr_vio.c
>> @@ -20,6 +20,7 @@
>>   */
>>  
>>  #include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>>  #include "qapi/error.h"
>>  #include "hw/hw.h"
>>  #include "qemu/log.h"
>> @@ -276,7 +277,7 @@ int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq)
>>      uint8_t byte;
>>  
>>      if (!dev->crq.qsize) {
>> -        fprintf(stderr, "spapr_vio_send_creq on uninitialized queue\n");
>> +        error_report("spapr_vio_send_creq on uninitialized queue");
>>          return -1;
>>      }
>>  
>