diff mbox

[V12,14/18] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()

Message ID 1365843407-16504-15-git-send-email-xiawenc@linux.vnet.ibm.com
State New
Headers show

Commit Message

Wayne Xia April 13, 2013, 8:56 a.m. UTC 
This patch introduce a new print function, which will output message to
monitor when it present. With it, bdrv_snapshot_dump() need no more buffer
and can avoid string truncation, bdrv_image_info_dump() can also be used by
hmp code later, besides qemu-img code.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qapi.c                |   58 +++++++++++++++++++++---------------------
 include/block/qapi.h        |    2 +-
 include/qemu/error-report.h |    1 +
 qemu-img.c                  |    7 +++--
 savevm.c                    |    7 +++--
 util/qemu-error.c           |   18 +++++++++++++
 6 files changed, 57 insertions(+), 36 deletions(-)

Comments

Eric Blake April 16, 2013, 11:42 p.m. UTC | #1 
On 04/13/2013 02:56 AM, Wenchao Xia wrote:
>   This patch introduce a new print function, which will output message to
> monitor when it present. With it, bdrv_snapshot_dump() need no more buffer
> and can avoid string truncation, bdrv_image_info_dump() can also be used by
> hmp code later, besides qemu-img code.

Not a full review (yet), but can we get this particular cleanup rebased
to be done first in isolation, since Pavel's patches would benefit from
being based on top of this cleanup as well?

https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03287.html
Wayne Xia April 17, 2013, 5:25 a.m. UTC | #2 
于 2013-4-17 7:42, Eric Blake 写道:
> On 04/13/2013 02:56 AM, Wenchao Xia wrote:
>>    This patch introduce a new print function, which will output message to
>> monitor when it present. With it, bdrv_snapshot_dump() need no more buffer
>> and can avoid string truncation, bdrv_image_info_dump() can also be used by
>> hmp code later, besides qemu-img code.
>
> Not a full review (yet), but can we get this particular cleanup rebased
> to be done first in isolation, since Pavel's patches would benefit from
> being based on top of this cleanup as well?
>
> https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03287.html
>
    I hope this serial have no more issues to be commited, avoiding
troubles in rebasing and file moves.
diff mbox

Patch

diff --git a/block/qapi.c b/block/qapi.c
index dc72adc..6e04a7f 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -351,7 +351,7 @@  BlockInfoList *qmp_query_block(Error **errp)
     return NULL;
 }
 
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
+void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
 {
     char buf1[128], date_buf[128], clock_buf[128];
     struct tm tm;
@@ -359,9 +359,8 @@  char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
     int64_t secs;
 
     if (!sn) {
-        snprintf(buf, buf_size,
-                 "%-10s%-20s%7s%20s%15s",
-                 "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+        message_printf("%-10s%-20s%7s%20s%15s",
+                       "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
     } else {
         ti = sn->date_sec;
         localtime_r(&ti, &tm);
@@ -374,14 +373,13 @@  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), sn->vm_state_size),
-                 date_buf,
-                 clock_buf);
+        message_printf("%-10s%-20s%7s%20s%15s",
+                       sn->id_str, sn->name,
+                       get_human_readable_size(buf1, sizeof(buf1),
+                                               sn->vm_state_size),
+                       date_buf,
+                       clock_buf);
     }
-    return buf;
 }
 
 void bdrv_image_info_dump(ImageInfo *info)
@@ -394,43 +392,44 @@  void bdrv_image_info_dump(ImageInfo *info)
                                 info->actual_size);
     }
     get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
-    printf("image: %s\n"
-           "file format: %s\n"
-           "virtual size: %s (%" PRId64 " bytes)\n"
-           "disk size: %s\n",
-           info->filename, info->format, size_buf,
-           info->virtual_size,
-           dsize_buf);
+    message_printf("image: %s\n"
+                   "file format: %s\n"
+                   "virtual size: %s (%" PRId64 " bytes)\n"
+                   "disk size: %s\n",
+                   info->filename, info->format, size_buf,
+                   info->virtual_size,
+                   dsize_buf);
 
     if (info->has_encrypted && info->encrypted) {
-        printf("encrypted: yes\n");
+        message_printf("encrypted: yes\n");
     }
 
     if (info->has_cluster_size) {
-        printf("cluster_size: %" PRId64 "\n", info->cluster_size);
+        message_printf("cluster_size: %" PRId64 "\n", info->cluster_size);
     }
 
     if (info->has_dirty_flag && info->dirty_flag) {
-        printf("cleanly shut down: no\n");
+        message_printf("cleanly shut down: no\n");
     }
 
     if (info->has_backing_filename) {
-        printf("backing file: %s", info->backing_filename);
+        message_printf("backing file: %s", info->backing_filename);
         if (info->has_full_backing_filename) {
-            printf(" (actual path: %s)", info->full_backing_filename);
+            message_printf(" (actual path: %s)", info->full_backing_filename);
         }
-        putchar('\n');
+        message_printf("\n");
         if (info->has_backing_filename_format) {
-            printf("backing file format: %s\n", info->backing_filename_format);
+            message_printf("backing file format: %s\n",
+                           info->backing_filename_format);
         }
     }
 
     if (info->has_snapshots) {
         SnapshotInfoList *elem;
-        char buf[256];
 
-        printf("Snapshot list:\n");
-        printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+        message_printf("Snapshot list:\n");
+        bdrv_snapshot_dump(NULL);
+        message_printf("\n");
 
         /* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
          * we convert to the block layer's native QEMUSnapshotInfo for now.
@@ -446,7 +445,8 @@  void bdrv_image_info_dump(ImageInfo *info)
 
             pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
             pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
-            printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+            bdrv_snapshot_dump(&sn);
+            message_printf("\n");
         }
     }
 }
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 237660f..f7eda89 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -38,6 +38,6 @@  int bdrv_query_image_info(BlockDriverState *bs,
 void bdrv_query_info(BlockDriverState *bs,
                      BlockInfo **p_info,
                      Error **errp);
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn);
+void bdrv_snapshot_dump(QEMUSnapshotInfo *sn);
 void bdrv_image_info_dump(ImageInfo *info);
 #endif
diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index c902cc1..90341bd 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -40,4 +40,5 @@  void error_set_progname(const char *argv0);
 void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 const char *error_get_progname(void);
 
+void message_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index 5b229a9..8cbcb0b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1558,16 +1558,17 @@  static void dump_snapshots(BlockDriverState *bs)
 {
     QEMUSnapshotInfo *sn_tab, *sn;
     int nb_sns, i;
-    char buf[256];
 
     nb_sns = bdrv_snapshot_list(bs, &sn_tab);
     if (nb_sns <= 0)
         return;
     printf("Snapshot list:\n");
-    printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+    bdrv_snapshot_dump(NULL);
+    printf("\n");
     for(i = 0; i < nb_sns; i++) {
         sn = &sn_tab[i];
-        printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+        bdrv_snapshot_dump(sn);
+        printf("\n");
     }
     g_free(sn_tab);
 }
diff --git a/savevm.c b/savevm.c
index 1c8fbee..e125567 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2466,7 +2466,6 @@  void do_info_snapshots(Monitor *mon, const QDict *qdict)
     int nb_sns, i, ret, available;
     int total;
     int *available_snapshots;
-    char buf[256];
 
     bs = bdrv_snapshots();
     if (!bs) {
@@ -2509,10 +2508,12 @@  void do_info_snapshots(Monitor *mon, const QDict *qdict)
     }
 
     if (total > 0) {
-        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+        bdrv_snapshot_dump(NULL);
+        monitor_printf(mon, "\n");
         for (i = 0; i < total; i++) {
             sn = &sn_tab[available_snapshots[i]];
-            monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+            bdrv_snapshot_dump(sn);
+            monitor_printf(mon, "\n");
         }
     } else {
         monitor_printf(mon, "There is no suitable snapshot available\n");
diff --git a/util/qemu-error.c b/util/qemu-error.c
index 08a36f4..a47bf32 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -213,3 +213,21 @@  void error_report(const char *fmt, ...)
     va_end(ap);
     error_printf("\n");
 }
+
+/*
+ * Print to current monitor if we have one, else to stdout. It is similar with
+ * error_printf().
+ * TODO just like error_vprintf()
+ */
+void message_printf(const char *fmt, ...)
+{
+    va_list ap;
+
+    va_start(ap, fmt);
+    if (cur_mon) {
+        monitor_vprintf(cur_mon, fmt, ap);
+    } else {
+        vfprintf(stdout, fmt, ap);
+    }
+    va_end(ap);
+}