[Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv_snapshot_dump() and bdrv_image_info_dump()
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
---
block/qapi.c | 65 +++---
include/block/qapi.h |5 ++-
qemu-img.c | 15 +++
savevm.c | 11 ++--
4 files changed, 55 insertions(+), 41 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 155e77e..2e083f8 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -259,7 +259,7 @@ static char *get_human_readable_size(char *buf, int
buf_size, int64_t size)
return buf;
}
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
+void bdrv_snapshot_dump(const QemuOutput *output, QEMUSnapshotInfo *sn)
{
char buf1[128], date_buf[128], clock_buf[128];
struct tm tm;
@@ -267,9 +267,9 @@ 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(output,
+ "%-10s%-20s%7s%20s%15s",
+ "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
} else {
ti = sn->date_sec;
localtime_r(&ti, &tm);
@@ -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 / 100) % 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(output,
+ "%-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)
+void bdrv_image_info_dump(const QemuOutput *output, ImageInfo *info)
{
char size_buf[128], dsize_buf[128];
if (!info->has_actual_size) {
@@ -302,43 +302,47 @@ 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(output,
+ "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(output, "encrypted: yes\n");
}
if (info->has_cluster_size) {
-printf("cluster_size: %" PRId64 "\n", info->cluster_size);
+message_printf(output, "cluster_size: %" PRId64 "\n",
+ info->cluster_size);
}
if (info->has_dirty_flag && info->dirty_flag) {
-printf("cleanly shut down: no\n");
+message_printf(output, "cleanly shut down: no\n");
}
if (info->has_backing_filename) {
-printf("backing file: %s", info->backing_filename);
+message_printf(output, "backing file: %s", info->backing_filename);
if (info->has_full_backing_filename) {
-printf(" (actual path: %s)", info->full_backing_filename);
+message_printf(output, " (actual path: %s)",
+ info->full_backing_filename);
}
-putchar('\n');
+message_printf(output, "\n");
if (info->has_backing_filename_format) {
-printf("backing file format: %s\n", info->backing_filename_format);
+message_printf(output, "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(output, "Snapshot list:\n");
+bdrv_snapshot_dump(output, NULL);
+message_printf(output, "\n");
/* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
* we convert to the block layer's native QEMUSnapshotInfo for now.
@@ -
Re: [Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv_snapshot_dump() and bdrv_image_info_dump()
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
> ---
> 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 / 100) % 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(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)
> info->actual_size);
> }
> get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
Again, get_human_readable_size should be converted to use QemuOutput.
> +++ 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
monitor.
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
buffers.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 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
---
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 / 100) % 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(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)
info->actual_size);
}
get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
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.
+++ 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
monitor.
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
buffers.
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 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
---
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 / 100) % 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(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)
info->actual_size);
}
get_human_readable_size(size_buf, sizeof(size_buf),
info->virtual_size);
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:
message_printf(output,
"%-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);
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
monitor.
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
buffers.
--
Best Regards
Wenchao Xia
