replace some fprintf(stderr, ...) with error_report()

PATCH Applied Le Tan 3 participants 13 messages May 9, 2014 – May 24, 2014 Block layer

Share & tools

Le Tan

author

2014-05-09 23:51 UTC #0 lore raw

cover replace some fprintf(stderr, ...) with error_report()

This series of patches replaces some more occurrences of fprintf(stderr, ...) with error_report() and removes the trailing "\n". Some of them are not changed because I am not sure if they should be changed. There are some inconsistency while dealing with macros that involves fprintf(stderr,...), that is, some of them remain the same while some are changed to error_report().
I have 43 more patches (most of them touch files in hw/) and would send them out once the first 4 patches are fine.

Le Tan (4):
arch_init: replace fprintf(stderr,...) with error_report() in
arch_init.c
audio: replace fprintf(stderr,...) with error_report() in audio
block: replace fprintf(stderr,...) with error_report() in block/
bsd-user: replace fprintf(stderr,...) with error_report()

 arch_init.c            |   36 ++++++++-------
 audio/spiceaudio.c     |    2 +-
 audio/wavcapture.c     |    4 +-
 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 +--
 bsd-user/bsdload.c     |    2 +-
 bsd-user/elfload.c     |    2 +-
 bsd-user/main.c        |   14 +++---
 22 files changed, 189 insertions(+), 194 deletions(-)

Signature · 1 line

1.7.9.5

Le Tan

author

2014-05-09 23:51 UTC #1 lore raw

PATCH 1/4 arch_init: replace fprintf(stderr, ...) with error_report() in arch_init.c

Replace fprintf(stderr,...) with error_report() in the file arch_init.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 <[email protected]>

---
 arch_init.c |   36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

arch_init.c +17 ~~-19~~

index 60c975d..0b41475 100644
@@ -921,12 +921,12 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
     xh_len = qemu_get_be16(f);
 
     if (xh_flags != ENCODING_FLAG_XBZRLE) {
-        fprintf(stderr, "Failed to load XBZRLE page - wrong compression!\n");
+        error_report("Failed to load XBZRLE page - wrong compression!");
         return -1;
     }
 
     if (xh_len > TARGET_PAGE_SIZE) {
-        fprintf(stderr, "Failed to load XBZRLE page - len overflow!\n");
+        error_report("Failed to load XBZRLE page - len overflow!");
         return -1;
     }
     /* load data and decode */

@@ -936,11 +936,11 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
     ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
                                TARGET_PAGE_SIZE);
     if (ret == -1) {
-        fprintf(stderr, "Failed to load XBZRLE page - decode error!\n");
+        error_report("Failed to load XBZRLE page - decode error!");
         rc = -1;
     } else  if (ret > TARGET_PAGE_SIZE) {
-        fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n",
-                ret, TARGET_PAGE_SIZE);
+        error_report("Failed to load XBZRLE page - size %d exceeds %d!",
+                     ret, TARGET_PAGE_SIZE);
         abort();
     }

@@ -957,7 +957,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
 
     if (flags & RAM_SAVE_FLAG_CONTINUE) {
         if (!block) {
-            fprintf(stderr, "Ack, bad migration stream!\n");
+            error_report("Ack, bad migration stream!");
             return NULL;
         }

@@ -973,7 +973,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
             return memory_region_get_ram_ptr(block->mr) + offset;
     }
 
-    fprintf(stderr, "Can't find block %s!\n", id);
+    error_report("Can't find block %s!", id);
     return NULL;
 }

@@ -1026,10 +1026,10 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                     QTAILQ_FOREACH(block, &ram_list.blocks, next) {
                         if (!strncmp(id, block->idstr, sizeof(id))) {
                             if (block->length != length) {
-                                fprintf(stderr,
-                                        "Length mismatch: %s: " RAM_ADDR_FMT
-                                        " in != " RAM_ADDR_FMT "\n", id, length,
-                                        block->length);
+                                error_report(
+                                             "Length mismatch: %s: " RAM_ADDR_FMT
+                                             " in != " RAM_ADDR_FMT, id, length,
+                                             block->length);
                                 ret =  -EINVAL;
                                 goto done;
                             }

@@ -1038,8 +1038,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                     }
 
                     if (!block) {
-                        fprintf(stderr, "Unknown ramblock \"%s\", cannot "
-                                "accept migration\n", id);
+                        error_report("Unknown ramblock \"%s\", cannot "
+                                     "accept migration", id);
                         ret = -EINVAL;
                         goto done;
                     }

@@ -1186,12 +1186,10 @@ void select_soundhw(const char *optarg)
 
             if (!c->name) {
                 if (l > 80) {
-                    fprintf(stderr,
-                            "Unknown sound card name (too big to show)\n");
+                    error_report("Unknown sound card name (too big to show)");
                 }
                 else {
-                    fprintf(stderr, "Unknown sound card name `%.*s'\n",
-                            (int) l, p);
+                    error_report("Unknown sound card name `%.*s'", (int) l, p);
                 }
                 bad_card = 1;
             }

@@ -1214,13 +1212,13 @@ void audio_init(void)
         if (c->enabled) {
             if (c->isa) {
                 if (!isa_bus) {
-                    fprintf(stderr, "ISA bus not available for %s\n", c->name);
+                    error_report("ISA bus not available for %s", c->name);
                     exit(1);
                 }
                 c->init.init_isa(isa_bus);
             } else {
                 if (!pci_bus) {
-                    fprintf(stderr, "PCI bus not available for %s\n", c->name);
+                    error_report("PCI bus not available for %s", c->name);
                     exit(1);
                 }
                 c->init.init_pci(pci_bus);

Signature · 1 line

1.7.9.5

Le Tan

author

2014-05-24 05:41 UTC #2 lore raw

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

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 7:55 GMT+08:00 Le Tan <[email protected]>:

Quoted 121 lines

Replace fprintf(stderr,...) with error_report() in the file arch_init.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 <[email protected]>

---
 arch_init.c |   36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

arch_init.c +17 ~~-20~~

index 60c975d..0b41475 100644
@@ -921,12 +921,12 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
     xh_len = qemu_get_be16(f);
 
     if (xh_flags != ENCODING_FLAG_XBZRLE) {
-        fprintf(stderr, "Failed to load XBZRLE page - wrong compression!\n");
+        error_report("Failed to load XBZRLE page - wrong compression!");
         return -1;
     }
 
     if (xh_len > TARGET_PAGE_SIZE) {
-        fprintf(stderr, "Failed to load XBZRLE page - len overflow!\n");
+        error_report("Failed to load XBZRLE page - len overflow!");
         return -1;
     }
     /* load data and decode */

@@ -936,11 +936,11 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
     ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
                                TARGET_PAGE_SIZE);
     if (ret == -1) {
-        fprintf(stderr, "Failed to load XBZRLE page - decode error!\n");
+        error_report("Failed to load XBZRLE page - decode error!");
         rc = -1;
     } else  if (ret > TARGET_PAGE_SIZE) {
-        fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n",
-                ret, TARGET_PAGE_SIZE);
+        error_report("Failed to load XBZRLE page - size %d exceeds %d!",
+                     ret, TARGET_PAGE_SIZE);
         abort();
     }

@@ -957,7 +957,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
 
     if (flags & RAM_SAVE_FLAG_CONTINUE) {
         if (!block) {
-            fprintf(stderr, "Ack, bad migration stream!\n");
+            error_report("Ack, bad migration stream!");
             return NULL;
         }

@@ -973,7 +973,7 @@ static inline void *host_from_stream_offset(QEMUFile *f,
             return memory_region_get_ram_ptr(block->mr) + offset;
     }
 
-    fprintf(stderr, "Can't find block %s!\n", id);
+    error_report("Can't find block %s!", id);
     return NULL;
 }

@@ -1026,10 +1026,10 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                     QTAILQ_FOREACH(block, &ram_list.blocks, next) {
                         if (!strncmp(id, block->idstr, sizeof(id))) {
                             if (block->length != length) {
-                                fprintf(stderr,
-                                        "Length mismatch: %s: " RAM_ADDR_FMT
-                                        " in != " RAM_ADDR_FMT "\n", id, length,
-                                        block->length);
+                                error_report(
+                                             "Length mismatch: %s: " RAM_ADDR_FMT
+                                             " in != " RAM_ADDR_FMT, id, length,
+                                             block->length);
                                 ret =  -EINVAL;
                                 goto done;
                             }

@@ -1038,8 +1038,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                     }
 
                     if (!block) {
-                        fprintf(stderr, "Unknown ramblock \"%s\", cannot "
-                                "accept migration\n", id);
+                        error_report("Unknown ramblock \"%s\", cannot "
+                                     "accept migration", id);
                         ret = -EINVAL;
                         goto done;
                     }

@@ -1186,12 +1186,10 @@ void select_soundhw(const char *optarg)
 
             if (!c->name) {
                 if (l > 80) {
-                    fprintf(stderr,
-                            "Unknown sound card name (too big to show)\n");
+                    error_report("Unknown sound card name (too big to show)");
                 }
                 else {
-                    fprintf(stderr, "Unknown sound card name `%.*s'\n",
-                            (int) l, p);
+                    error_report("Unknown sound card name `%.*s'", (int) l, p);
                 }
                 bad_card = 1;
             }

@@ -1214,13 +1212,13 @@ void audio_init(void)
         if (c->enabled) {
             if (c->isa) {
                 if (!isa_bus) {
-                    fprintf(stderr, "ISA bus not available for %s\n", c->name);
+                    error_report("ISA bus not available for %s", c->name);
                     exit(1);
                 }
                 c->init.init_isa(isa_bus);
             } else {
                 if (!pci_bus) {
-                    fprintf(stderr, "PCI bus not available for %s\n", c->name);
+                    error_report("PCI bus not available for %s", c->name);
                     exit(1);
                 }
                 c->init.init_pci(pci_bus);
--

1.7.9.5

Michael Tokarev

2014-05-24 05:56 UTC #3 lore raw

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

Quoted 3 lines · 24.05.2014 09:41, Le Tan wrote:

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. :)

I've already applied your v2 to the trivial tree, and I told you just that, replying to the v2 patch.

/mjt

Le Tan

author

2014-05-09 23:52 UTC #4 lore raw

PATCH 2/4 audio: replace fprintf(stderr, ...) with error_report() in audio

In reply to Le Tan

Replace fprintf(stderr,...) with error_report() in files audio/*. 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 <[email protected]>

---
 audio/spiceaudio.c |    2 +-
 audio/wavcapture.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

audio/spiceaudio.c +1 -1

index fceee50..7b79bed 100644
@@ -105,7 +105,7 @@ static int rate_get_samples (struct audio_pcm_info *info, SpiceRateCtl *rate)
     bytes = muldiv64 (ticks, info->bytes_per_second, get_ticks_per_sec ());
     samples = (bytes - rate->bytes_sent) >> info->shift;
     if (samples < 0 || samples > 65536) {
-        fprintf (stderr, "Resetting rate control (%" PRId64 " samples)\n", samples);
+        error_report("Resetting rate control (%" PRId64 " samples)", samples);
         rate_start (rate);
         samples = 0;
     }

audio/wavcapture.c +2 -2

index 9d94623..60f7a72 100644
@@ -63,8 +63,8 @@ static void wav_destroy (void *opaque)
         }
     doclose:
         if (fclose (wav->f)) {
-            fprintf (stderr, "wav_destroy: fclose failed: %s",
-                     strerror (errno));
+            error_report("wav_destroy: fclose failed: %s",
+                         strerror (errno));
         }
     }

Signature · 1 line

1.7.9.5

Michael Tokarev

Applied

2014-05-23 21:04 UTC #5 lore raw

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

Quoted 3 lines · 10.05.2014 03:55, Le Tan wrote:

Replace fprintf(stderr,...) with error_report() in files audio/*. The trailing "\n"s of the @fmt argument have been removed because @fmt of error_report() should not contain newline.

Please always check your patches using ./scripts/checkpatch.pl.

I've applied this your patch to -trivial, after fixing this place:

audio/wavcapture.c static void wav_destroy (void *opaque) · line 67

Quoted 10 lines

audio/wavcapture.c +2 -2

@@ -63,8 +63,8 @@ static void wav_destroy (void *opaque)
         }
     doclose:
         if (fclose (wav->f)) {
-            fprintf (stderr, "wav_destroy: fclose failed: %s",
-                     strerror (errno));
+            error_report("wav_destroy: fclose failed: %s",
+                         strerror (errno));

to not contain an extra space after strerror, and folding whole thing into one line. Yes, the code around has the same style, but the usual rule is that you fix style issues in the code you touch, to comply with the coding style used in qemu.

Thanks,

/mjt

Le Tan

author

2014-05-09 23:52 UTC #6 lore raw

PATCH 3/4 block: replace fprintf(stderr, ...) with error_report() in block/

In reply to Le Tan

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 <[email protected]>

---
 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(-)

block-migration.c +2 -2

index 56951e0..5bcf7c8 100644
@@ -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);

block.c +2 -2

index b749d31..7dc4acb 100644
@@ -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);
     }

block/linux-aio.c +2 -2

index 53434e2..b706a59 100644
@@ -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));

block/nbd-client.h +1 -1

index f2a6337..74178f4 100644
@@ -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__)
 #else
 #define logout(fmt, ...) ((void)0)
 #endif

block/qcow2-cluster.c +2 -2

index 76d2bcf..b1c8daf 100644
@@ -67,8 +67,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
     }
 
 #ifdef DEBUG_ALLOC2
-    fprintf(stderr, "grow l1_table from %d to %" PRId64 "\n",
-            s->l1_size, new_l1_size);
+    error_report("grow l1_table from %d to %" PRId64,
+                 s->l1_size, new_l1_size);
 #endif
 
     new_l1_size2 = sizeof(uint64_t) * new_l1_size;

block/qcow2-refcount.c +57 ~~-57~~

278 lines not on this page · raw message

block/qcow2.c +8 -8

index a4b97e8..e01855b 100644
@@ -2225,12 +2225,12 @@ static int qcow2_amend_options(BlockDriverState *bs,
             } else if (!strcmp(options[i].value.s, "1.1")) {
                 new_version = 3;
             } else {
-                fprintf(stderr, "Unknown compatibility level %s.\n",
+                error_report("Unknown compatibility level %s.",
                         options[i].value.s);
                 return -EINVAL;
             }
         } else if (!strcmp(options[i].name, "preallocation")) {
-            fprintf(stderr, "Cannot change preallocation mode.\n");
+            error_report("Cannot change preallocation mode.");
             return -ENOTSUP;
         } else if (!strcmp(options[i].name, "size")) {
             new_size = options[i].value.n;

@@ -2240,14 +2240,14 @@ static int qcow2_amend_options(BlockDriverState *bs,
             backing_format = options[i].value.s;
         } else if (!strcmp(options[i].name, "encryption")) {
             if ((options[i].value.n != !!s->crypt_method)) {
-                fprintf(stderr, "Changing the encryption flag is not "
-                        "supported.\n");
+                error_report("Changing the encryption flag is not "
+                             "supported.");
                 return -ENOTSUP;
             }
         } else if (!strcmp(options[i].name, "cluster_size")) {
             if (options[i].value.n != s->cluster_size) {
-                fprintf(stderr, "Changing the cluster size is not "
-                        "supported.\n");
+                error_report("Changing the cluster size is not "
+                             "supported.");
                 return -ENOTSUP;
             }
         } else if (!strcmp(options[i].name, "lazy_refcounts")) {

@@ -2287,8 +2287,8 @@ static int qcow2_amend_options(BlockDriverState *bs,
     if (s->use_lazy_refcounts != lazy_refcounts) {
         if (lazy_refcounts) {
             if (s->qcow_version < 3) {
-                fprintf(stderr, "Lazy refcounts only supported with compatibility "
-                        "level 1.1 and above (use compat=1.1 or greater)\n");
+                error_report("Lazy refcounts only supported with compatibility "
+                             "level 1.1 and above (use compat=1.1 or greater)");
                 return -EINVAL;
             }
             s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;

block/quorum.c +2 -2

index ecec3a5..6fe65a6 100644
@@ -771,8 +771,8 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
         s->num_children == 2 && s->threshold == 2) {
         s->is_blkverify = true;
     } else if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false)) {
-        fprintf(stderr, "blkverify mode is set by setting blkverify=on "
-                "and using two files with vote_threshold=2\n");
+        error_report("blkverify mode is set by setting blkverify=on "
+                     "and using two files with vote_threshold=2");
     }
 
     /* allocate the children BlockDriverState array */

block/raw-posix.c +4 -4

index 3ce026d..27b34bf 100644
@@ -187,7 +187,7 @@ static int raw_normalize_devicepath(const char **filename)
     fname = *filename;
     dp = strrchr(fname, '/');
     if (lstat(fname, &sb) < 0) {
-        fprintf(stderr, "%s: stat failed: %s\n",
+        error_report("%s: stat failed: %s",
             fname, strerror(errno));
         return -errno;
     }

@@ -202,9 +202,9 @@ static int raw_normalize_devicepath(const char **filename)
         snprintf(namebuf, PATH_MAX, "%.*s/r%s",
             (int)(dp - fname), fname, dp + 1);
     }
-    fprintf(stderr, "%s is a block device", fname);
+    error_report("%s is a block device", fname);
     *filename = namebuf;
-    fprintf(stderr, ", using %s\n", *filename);
+    error_report(", using %s", *filename);
 
     return 0;
 }

@@ -942,7 +942,7 @@ static int aio_worker(void *arg)
         ret = handle_aiocb_write_zeroes(aiocb);
         break;
     default:
-        fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
+        error_report("invalid aio request (0x%x)", aiocb->aio_type);
         ret = -EINVAL;
         break;
     }

block/raw-win32.c +3 -3

index 064ea31..5f67285 100644
@@ -131,7 +131,7 @@ static int aio_worker(void *arg)
         }
         break;
     default:
-        fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
+        error_report("invalid aio request (0x%x)", aiocb->aio_type);
         ret = -EINVAL;
         break;
     }

@@ -410,11 +410,11 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset)
      */
     dwPtrLow = SetFilePointer(s->hfile, low, &high, FILE_BEGIN);
     if (dwPtrLow == INVALID_SET_FILE_POINTER && GetLastError() != NO_ERROR) {
-        fprintf(stderr, "SetFilePointer error: %lu\n", GetLastError());
+        error_report("SetFilePointer error: %lu", GetLastError());
         return -EIO;
     }
     if (SetEndOfFile(s->hfile) == 0) {
-        fprintf(stderr, "SetEndOfFile error: %lu\n", GetLastError());
+        error_report("SetEndOfFile error: %lu", GetLastError());
         return -EIO;
     }
     return 0;

block/ssh.c +2 -2

index aa63c9d..955f0eb 100644
@@ -49,7 +49,7 @@
 #define DPRINTF(fmt, ...)                           \
     do {                                            \
         if (DEBUG_SSH) {                            \
-            fprintf(stderr, "ssh: %-15s " fmt "\n", \
+            error_report("ssh: %-15s " fmt,         \
                     __func__, ##__VA_ARGS__);       \
         }                                           \
     } while (0)

@@ -1060,7 +1060,7 @@ static void bdrv_ssh_init(void)
 
     r = libssh2_init(0);
     if (r != 0) {
-        fprintf(stderr, "libssh2 initialization failed, %d\n", r);
+        error_report("libssh2 initialization failed, %d", r);
         exit(EXIT_FAILURE);
     }

block/vdi.c +7 -7

index 27737af..672c2d1 100644
@@ -89,7 +89,7 @@ typedef unsigned char uuid_t[16];
 
 #if defined(CONFIG_VDI_DEBUG)
 #define logout(fmt, ...) \
-                fprintf(stderr, "vdi\t%-24s" fmt, __func__, ##__VA_ARGS__)
+                error_report("vdi\t%-24s" fmt, __func__, ##__VA_ARGS__)
 #else
 #define logout(fmt, ...) ((void)0)
 #endif

@@ -305,20 +305,20 @@ static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res,
                 if (!VDI_IS_ALLOCATED(bmap[bmap_entry])) {
                     bmap[bmap_entry] = bmap_entry;
                 } else {
-                    fprintf(stderr, "ERROR: block index %" PRIu32
-                            " also used by %" PRIu32 "\n", bmap[bmap_entry], bmap_entry);
+                    error_report("ERROR: block index %" PRIu32
+                                 " also used by %" PRIu32, bmap[bmap_entry], bmap_entry);
                     res->corruptions++;
                 }
             } else {
-                fprintf(stderr, "ERROR: block index %" PRIu32
-                        " too large, is %" PRIu32 "\n", block, bmap_entry);
+                error_report("ERROR: block index %" PRIu32
+                             " too large, is %" PRIu32, block, bmap_entry);
                 res->corruptions++;
             }
         }
     }
     if (blocks_allocated != s->header.blocks_allocated) {
-        fprintf(stderr, "ERROR: allocated blocks mismatch, is %" PRIu32
-               ", should be %" PRIu32 "\n",
+        error_report("ERROR: allocated blocks mismatch, is %" PRIu32
+                     ", should be %" PRIu32,
                blocks_allocated, s->header.blocks_allocated);
         res->corruptions++;
     }

block/vmdk.c +9 ~~-12~~

index 06a1f9f..98b6871 100644
@@ -1396,8 +1396,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
 
     if (sector_num > bs->total_sectors) {
         error_report("Wrong offset: sector_num=0x%" PRIx64
-                " total_sectors=0x%" PRIx64 "\n",
-                sector_num, bs->total_sectors);
+                     " total_sectors=0x%" PRIx64,
+                     sector_num, bs->total_sectors);
         return -EIO;
     }

@@ -1416,7 +1416,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
             if (ret == VMDK_OK) {
                 /* Refuse write to allocated cluster for streamOptimized */
                 error_report("Could not write to allocated cluster"
-                              " for streamOptimized");
+                             " for streamOptimized");
                 return -EIO;
             } else {
                 /* allocate */

@@ -2005,24 +2005,21 @@ static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result,
         }
         extent = find_extent(s, sector_num, extent);
         if (!extent) {
-            fprintf(stderr,
-                    "ERROR: could not find extent for sector %" PRId64 "\n",
-                    sector_num);
+            error_report("ERROR: could not find extent for sector %" PRId64,
+                         sector_num);
             break;
         }
         ret = get_cluster_offset(bs, extent, NULL,
                                  sector_num << BDRV_SECTOR_BITS,
                                  0, &cluster_offset);
         if (ret == VMDK_ERROR) {
-            fprintf(stderr,
-                    "ERROR: could not get cluster_offset for sector %"
-                    PRId64 "\n", sector_num);
+            error_report("ERROR: could not get cluster_offset for sector %"
+                         PRId64, sector_num);
             break;
         }
         if (ret == VMDK_OK && cluster_offset >= bdrv_getlength(extent->file)) {
-            fprintf(stderr,
-                    "ERROR: cluster offset for sector %"
-                    PRId64 " points after EOF\n", sector_num);
+            error_report("ERROR: cluster offset for sector %"
+                         PRId64 " points after EOF", sector_num);
             break;
         }
         sector_num += extent->cluster_sectors;

block/vpc.c +2 -2

index 2e25f57..86f7890 100644
@@ -203,8 +203,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     checksum = be32_to_cpu(footer->checksum);
     footer->checksum = 0;
     if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum)
-        fprintf(stderr, "block-vpc: The header checksum of '%s' is "
-            "incorrect.\n", bs->filename);
+        error_report("block-vpc: The header checksum of '%s' is "
+                     "incorrect.", bs->filename);
 
     /* Write 'checksum' back to footer, or else will leave it with zero. */
     footer->checksum = be32_to_cpu(checksum);

block/vvfat.c +54 ~~-54~~

353 lines not on this page · raw message

blockdev.c +3 -3

index 7810e9f..c922099 100644
@@ -698,9 +698,9 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
 
     /* Deprecated option boot=[on|off] */
     if (qemu_opt_get(legacy_opts, "boot") != NULL) {
-        fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be "
-                "ignored. Future versions will reject this parameter. Please "
-                "update your scripts.\n");
+        error_report("qemu-kvm: boot=on|off is deprecated and will be "
+                     "ignored. Future versions will reject this parameter. Please "
+                     "update your scripts.");
     }
 
     /* Media type */

Signature · 1 line

1.7.9.5

Peter Crosthwaite

2014-05-10 13:18 UTC #7 lore raw

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

Quoted 81 lines · On Sat, May 10, 2014 at 9:55 AM, Le Tan <[email protected]> wrote:

Signed-off-by Le Tan <[email protected]>

---
 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(-)

block-migration.c +2 -2

index 56951e0..5bcf7c8 100644
@@ -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);

block.c +2 -2

index b749d31..7dc4acb 100644
@@ -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);
     }

block/linux-aio.c +2 -2

index 53434e2..b706a59 100644
@@ -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));

block/nbd-client.h +0 -0

index f2a6337..74178f4 100644

block/nbd-client.h line 12

Quoted 5 lines

block/nbd-client.h +1 -1

@@ -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

Le Tan

author

2014-05-11 13:27 UTC #8 lore raw

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

2014-05-10 21:18 GMT+08:00 Peter Crosthwaite <[email protected]>:

Quoted 98 lines · On Sat, May 10, 2014 at 9:55 AM, Le Tan <[email protected]> wrote:

Quoted 81 lines

Signed-off-by Le Tan <[email protected]>

---
 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(-)

block-migration.c +2 -2

index 56951e0..5bcf7c8 100644
@@ -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);

block.c +2 -2

index b749d31..7dc4acb 100644
@@ -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);
     }

block/linux-aio.c +2 -2

index 53434e2..b706a59 100644
@@ -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));

block/nbd-client.h +0 -0

index f2a6337..74178f4 100644

block/nbd-client.h line 12

Quoted 5 lines

block/nbd-client.h +1 -1

@@ -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__)

Quoted 8 lines

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

Hi! I am a novice and this is my warm-up task for GSoC. So you mean that it's good to convert printfs to error_report where the message is deemed to notice the user, such as some warning about wrong cmd arguments and so on, while it is better to let them be where the printfs are used for the developer to debug, such as:
#if defined(DEBUG_NBD)

#define logout(fmt, ...) \
    fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)

Am I right?
I think I should fix the patch and convert the printfs to error_report selectively. Do I get the idea?
Thanks very much!

Regards,
Le

Peter Crosthwaite

2014-05-12 01:13 UTC #9 lore raw

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

Quoted 113 lines · On Sun, May 11, 2014 at 11:27 PM, Le Tan <[email protected]> wrote:

2014-05-10 21:18 GMT+08:00 Peter Crosthwaite <[email protected]>:
Quoted 98 lines · On Sat, May 10, 2014 at 9:55 AM, Le Tan <[email protected]> wrote:
Quoted 81 lines
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 <[email protected]>
---
 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(-)
block-migration.c +2 -2
index 56951e0..5bcf7c8 100644
@@ -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);
block.c +2 -2
index b749d31..7dc4acb 100644
@@ -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);
     }
 
block/linux-aio.c +2 -2
index 53434e2..b706a59 100644
@@ -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));
block/nbd-client.h +0 -0
index f2a6337..74178f4 100644
block/nbd-client.h line 12
Quoted 5 lines
block/nbd-client.h +1 -1
@@ -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.
Quoted 8 lines
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
Hi! I am a novice and this is my warm-up task for GSoC. So you mean that it's good to convert printfs to error_report where the message is deemed to notice the user, such as some warning about wrong cmd arguments and so on, while it is better to let them be where the printfs are used for the developer to debug, such as:
#if defined(DEBUG_NBD)

Quoted 6 lines

#define logout(fmt, ...) \
    fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
Am I right?
I think I should fix the patch and convert the printfs to error_report selectively. Do I get the idea?

Yes sounds clear. Just leave those that aren't errors alone.

Give others a chance (a day or two) to weigh in on discussion as well.

Regards,
Peter

Quoted 5 lines

Thanks very much!
Regards,
Le

Le Tan

author

2014-05-24 05:44 UTC #10 lore raw

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

In reply to Peter Crosthwaite

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 <[email protected]>:

Quoted 106 lines · On Sat, May 10, 2014 at 9:55 AM, Le Tan <[email protected]> wrote:

Quoted 81 lines

Signed-off-by Le Tan <[email protected]>

---
 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(-)

block-migration.c +2 -2

index 56951e0..5bcf7c8 100644
@@ -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);

block.c +2 -2

index b749d31..7dc4acb 100644
@@ -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);
     }

block/linux-aio.c +2 -2

index 53434e2..b706a59 100644
@@ -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));

block/nbd-client.h +0 -0

index f2a6337..74178f4 100644

block/nbd-client.h line 12

Quoted 5 lines

block/nbd-client.h +1 -1

@@ -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__)

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

Le Tan

author

2014-05-09 23:52 UTC #11 lore raw

PATCH 4/4 bsd-user: replace fprintf(stderr, ...) with error_report()

In reply to Le Tan

Replace fprintf(stderr,...) with error_report() in files bsd-user/*. 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 <[email protected]>

---
 bsd-user/bsdload.c |    2 +-
 bsd-user/elfload.c |    2 +-
 bsd-user/main.c    |   14 +++++++-------
 3 files changed, 9 insertions(+), 9 deletions(-)

bsd-user/bsdload.c +1 -1

index 2abc713..6b52e08 100644
@@ -183,7 +183,7 @@ int loader_exec(const char * filename, char ** argv, char ** envp,
                 && bprm.buf[3] == 'F') {
             retval = load_elf_binary(&bprm,regs,infop);
         } else {
-            fprintf(stderr, "Unknown binary format\n");
+            error_report("Unknown binary format");
             return -1;
         }
     }

bsd-user/elfload.c +1 -1

index 93fd9e4..95652b1 100644
@@ -628,7 +628,7 @@ static abi_ulong copy_elf_strings(int argc,char ** argv, void **page,
     while (argc-- > 0) {
         tmp = argv[argc];
         if (!tmp) {
-            fprintf(stderr, "VFS: argc is wrong");
+            error_report("VFS: argc is wrong");
             exit(-1);
         }
         tmp1 = tmp;

bsd-user/main.c +7 -7

index f81ba55..3825e76 100644
@@ -378,8 +378,8 @@ void cpu_loop(CPUX86State *env)
 #endif
         default:
             pc = env->segs[R_CS].base + env->eip;
-            fprintf(stderr, "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
-                    (long)pc, trapnr);
+            error_report("qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting",
+                         (long)pc, trapnr);
             abort();
         }
         process_pending_signals(env);

@@ -752,7 +752,7 @@ int main(int argc, char **argv)
     module_call_init(MODULE_INIT_QOM);
 
     if ((envlist = envlist_create()) == NULL) {
-        (void) fprintf(stderr, "Unable to allocate envlist\n");
+        (void) error_report("Unable to allocate envlist");
         exit(1);
     }

@@ -794,7 +794,7 @@ int main(int argc, char **argv)
         } else if (!strcmp(r, "ignore-environment")) {
             envlist_free(envlist);
             if ((envlist = envlist_create()) == NULL) {
-                (void) fprintf(stderr, "Unable to allocate envlist\n");
+                (void) error_report("Unable to allocate envlist");
                 exit(1);
             }
         } else if (!strcmp(r, "U")) {

@@ -816,7 +816,7 @@ int main(int argc, char **argv)
             qemu_host_page_size = atoi(argv[optind++]);
             if (qemu_host_page_size == 0 ||
                 (qemu_host_page_size & (qemu_host_page_size - 1)) != 0) {
-                fprintf(stderr, "page size must be a power of two\n");
+                error_report("page size must be a power of two");
                 exit(1);
             }
         } else if (!strcmp(r, "g")) {

@@ -910,7 +910,7 @@ int main(int argc, char **argv)
        qemu_host_page_size */
     env = cpu_init(cpu_model);
     if (!env) {
-        fprintf(stderr, "Unable to find CPU definition\n");
+        error_report("Unable to find CPU definition");
         exit(1);
     }
     cpu = ENV_GET_CPU(env);

@@ -1014,7 +1014,7 @@ int main(int argc, char **argv)
 #ifndef TARGET_ABI32
     /* enable 64 bit mode if possible */
     if (!(env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
-        fprintf(stderr, "The selected x86 CPU does not support 64 bit mode\n");
+        error_report("The selected x86 CPU does not support 64 bit mode");
         exit(1);
     }
     env->cr[4] |= CR4_PAE_MASK;

Signature · 1 line

1.7.9.5

Michael Tokarev

2014-05-23 21:16 UTC #12 lore raw

Re: [Qemu-devel] [Qemu-trivial] [PATCH 4/4] bsd-user: replace fprintf(stderr, ...) with error_report()

Quoted 3 lines · 10.05.2014 03:55, Le Tan wrote:

Replace fprintf(stderr,...) with error_report() in files bsd-user/*. The trailing "\n"s of the @fmt argument have been removed because @fmt of error_report() should not contain newline.

I'd fix two issues in there:

...

bsd-user/main.c void cpu_loop(CPUX86State *env) · line 383

Quoted 7 lines

         default:
             pc = env->segs[R_CS].base + env->eip;

bsd-user/main.c +2 -2

-            fprintf(stderr, "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
-                    (long)pc, trapnr);
+            error_report("qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting",
+                         (long)pc, trapnr);
             abort();

Wrapping this line differently to fit in 80 columns. Yes, the original line was even longer.

Review

Quoted 3 lines

     if ((envlist = envlist_create()) == NULL) {

(quoted diff) +1 -1

-        (void) fprintf(stderr, "Unable to allocate envlist\n");
+        (void) error_report("Unable to allocate envlist");

And removed these void casts (here and elsewhere), these aern't needed, error_report() is a function returning void.

Quoted 1 line

         exit(1);

I think we may use some other function here and in all other places, which combines error_report with exit/abort. What's the proper function for this?

Anyway, applying this patch for now, with the fixed outlined. Please always check your patches using checkpatch.pl before sending.

Thanks,

/mjt