[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv

From: Wenchao Xia
Subject: Re: [Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv_snapshot_dump() and bdrv_image_info_dump()
Date: Fri, 24 May 2013 10:31:48 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130509 Thunderbird/17.0.6

于 2013-5-24 9:48, Wenchao Xia 写道:
于 2013-5-23 23:31, Eric Blake 写道:
On 05/23/2013 02:47 AM, Wenchao Xia wrote:
Buffer is not used now so the string would not be truncated any more.
They can be used
by both qemu and qemu-img with correct parameter specified.

Signed-off-by: Wenchao Xia <address@hidden>
  block/qapi.c         |   65
  include/block/qapi.h |    5 ++-
  qemu-img.c           |   15 +++++++----
  savevm.c             |   11 ++++++--
  4 files changed, 55 insertions(+), 41 deletions(-)

@@ -282,17 +282,17 @@ char *bdrv_snapshot_dump(char *buf, int
buf_size, QEMUSnapshotInfo *sn)
                   (int)((secs / 60) % 60),
                   (int)(secs % 60),
                   (int)((sn->vm_clock_nsec / 1000000) % 1000));
-        snprintf(buf, buf_size,
-                 "%-10s%-20s%7s%20s%15s",
-                 sn->id_str, sn->name,
-                 get_human_readable_size(buf1, sizeof(buf1),
-                 date_buf,
-                 clock_buf);
+        message_printf(output,

You got rid of ONE buffer...

+                       "%-10s%-20s%7s%20s%15s",
+                       sn->id_str, sn->name,
+                       get_human_readable_size(buf1, sizeof(buf1),

...but what is this other buffer still doing?  get_human_readable_size
needs to be converted to use QemuOutput.

+void bdrv_image_info_dump(const QemuOutput *output, ImageInfo *info)
      char size_buf[128], dsize_buf[128];

Why do we still need size_buf and dsize_buf?

      if (!info->has_actual_size) {
@@ -302,43 +302,47 @@ void bdrv_image_info_dump(ImageInfo *info)
      get_human_readable_size(size_buf, sizeof(size_buf),

Again, get_human_readable_size should be converted to use QemuOutput.

   They do not likely have a chance to be truncated, so have not change
them. I will convert them also in next version.

  Just found this buffer is used in format control:

                       sn->id_str, sn->name,
                       get_human_readable_size(buf1, sizeof(buf1),

  if get_human_readable_size() is converted, a manual control is
needed. Same thing happens to clock_buf. It seems better to keep what
it is.

+++ b/qemu-img.c
@@ -1554,16 +1554,18 @@ static void dump_snapshots(BlockDriverState *bs)
      QEMUSnapshotInfo *sn_tab, *sn;
      int nb_sns, i;
-    char buf[256];
+    QemuOutput output = {OUTPUT_STREAM, {stdout,} };

This is relying on C99's rule that a union is initialized by its first
named member.  But I think it might be more readable as:

output = { .kind = OUTPUT_STREAM, .stream = stdout };

not to mention that you will HAVE to use a designator to ever initialize
the monitor element of the union in any parallel code that favors the
   This solve the initialization issue, will use it, thank u for the tip.

Hmm, does C99 even allow anonymous unions, or is that a gcc extension?

Overall, I like the direction this is headed.  The conversion looks
reasonable, although it didn't quite go far enough for getting rid of

Best Regards

Wenchao Xia

reply via email to

[Prev in Thread] Current Thread [Next in Thread]