Re: [Qemu-devel] [PATCH 3/4] block: replace fprintf(stderr, ...) with error_report() in block/

[Le Tan]

There is a newer version of patch to fix this problem. I forgot to add
the version number when I sent the new patch. Add this comment just to
indicate that this patch should be dropped. :)

2014-05-10 21:18 GMT+08:00 Peter Crosthwaite <address@hidden>:
> On Sat, May 10, 2014 at 9:55 AM, Le Tan <address@hidden> wrote:
>> Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
>> block-migration.c and blockdev.c. The trailing "\n"s of the @fmt argument
>> have been removed because @fmt of error_report() should not contain newline.
>>
>> Signed-off-by: Le Tan <address@hidden>
>> ---
>>  block-migration.c      |    4 +-
>>  block.c                |    4 +-
>>  block/linux-aio.c      |    4 +-
>>  block/nbd-client.h     |    2 +-
>>  block/qcow2-cluster.c  |    4 +-
>>  block/qcow2-refcount.c |  114 
>> ++++++++++++++++++++++++------------------------
>>  block/qcow2.c          |   16 +++----
>>  block/quorum.c         |    4 +-
>>  block/raw-posix.c      |    8 ++--
>>  block/raw-win32.c      |    6 +--
>>  block/ssh.c            |    4 +-
>>  block/vdi.c            |   14 +++---
>>  block/vmdk.c           |   21 ++++-----
>>  block/vpc.c            |    4 +-
>>  block/vvfat.c          |  108 ++++++++++++++++++++++-----------------------
>>  blockdev.c             |    6 +--
>>  16 files changed, 160 insertions(+), 163 deletions(-)
>>
>> diff --git a/block-migration.c b/block-migration.c
>> index 56951e0..5bcf7c8 100644
>> --- a/block-migration.c
>> +++ b/block-migration.c
>> @@ -790,7 +790,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
>> version_id)
>>
>>              bs = bdrv_find(device_name);
>>              if (!bs) {
>> -                fprintf(stderr, "Error unknown block device %s\n",
>> +                error_report("Error unknown block device %s",
>>                          device_name);
>>                  return -EINVAL;
>>              }
>> @@ -833,7 +833,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
>> version_id)
>>                     (addr == 100) ? '\n' : '\r');
>>              fflush(stdout);
>>          } else if (!(flags & BLK_MIG_FLAG_EOS)) {
>> -            fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
>> +            error_report("Unknown block migration flags: %#x", flags);
>>              return -EINVAL;
>>          }
>>          ret = qemu_file_get_error(f);
>> diff --git a/block.c b/block.c
>> index b749d31..7dc4acb 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2694,8 +2694,8 @@ static int bdrv_prwv_co(BlockDriverState *bs, int64_t 
>> offset,
>>       * if it has been enabled.
>>       */
>>      if (bs->io_limits_enabled) {
>> -        fprintf(stderr, "Disabling I/O throttling on '%s' due "
>> -                        "to synchronous I/O.\n", bdrv_get_device_name(bs));
>> +        error_report("Disabling I/O throttling on '%s' due "
>> +                     "to synchronous I/O.", bdrv_get_device_name(bs));
>>          bdrv_io_limits_disable(bs);
>>      }
>>
>> diff --git a/block/linux-aio.c b/block/linux-aio.c
>> index 53434e2..b706a59 100644
>> --- a/block/linux-aio.c
>> +++ b/block/linux-aio.c
>> @@ -162,8 +162,8 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void 
>> *aio_ctx, int fd,
>>         break;
>>      /* Currently Linux kernel does not support other operations */
>>      default:
>> -        fprintf(stderr, "%s: invalid AIO request type 0x%x.\n",
>> -                        __func__, type);
>> +        error_report("%s: invalid AIO request type 0x%x.",
>> +                     __func__, type);
>>          goto out_free_aiocb;
>>      }
>>      io_set_eventfd(&laiocb->iocb, event_notifier_get_fd(&s->e));
>> diff --git a/block/nbd-client.h b/block/nbd-client.h
>> index f2a6337..74178f4 100644
>> --- a/block/nbd-client.h
>> +++ b/block/nbd-client.h
>> @@ -9,7 +9,7 @@
>>
>>  #if defined(DEBUG_NBD)
>>  #define logout(fmt, ...) \
>> -    fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
>> +    error_report("nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
>
> So i'm not sure we want to convert debug printfery to error_report.
> There's very good value in converting the printfs with user
> visibility, but ones like this seem intended for developers only as
> throwaway-output. My thinking is that this is a lower level output
> than error_report. For instance, as a developer you may do something
> to break your error API yet you still want your debug printfery.
> Wouldn't matter in this location, but there may be other parts of the
> tree where we don't want error_report relinace for debug
> instrumentation and it just seems better to keep it consistent.
>
> Thinking further afield, qemu_log may ultimately be the correct
> mechanism for this instead (I think thats what I have been using for
> new code recently anyway).
>
> Thoughts from others?
>
> Regards,
> Peter