Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Wed, 22 May 2013 10:09:19 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-20 10:39, Wenchao Xia 写道:
于 2013-5-17 20:30, Luiz Capitulino 写道:
On Fri, 17 May 2013 11:30:31 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-16 20:17, Luiz Capitulino 写道:
On Thu, 16 May 2013 10:22:09 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-15 20:28, Luiz Capitulino 写道:
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a
reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable
cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess
that is the
question.
I'd have to see the code to tell, but yes, what Stefan
described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants
more than one
human monitor console, and then we may require a data
structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no
issues for it.
As I said before, I'd have to see the code to tell. But
answering your comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now,
waiting for
your confirm.
Can you point me to the code in question?
Sure, it is
+
+/*
+ * 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);
+}
This function used global variable cur_mon instead of input
parameter,
similar to error_printf().
Why do you need it? Why can't you you use error_printf() for example?
error_printf() will print out to stderr in qemu-img, but stdout is
wanted for those dump info function.
You can refactor the code so that you can pass a FILE *stream argument to
error_vprintf() and maybe add error_printf_stream()?
The name is a bit confusing, maybe qemu_printf()? Another problem is,
monitor have a buf[] instead of a FILE*, I think it need a structure
include those:
typdef enum QemuOutputType {
QEMU_OUTPUT_TYPE_STREAM,
QEMU_OUTPUT_TYPE_MONITOR,
} QemuOutputType;
typedef struct QemuOutput {
QemuOutputType type;
union {
FILE *file;
Monitor *mon;
};
}
It may brings some inconvienience to caller, but this is what I can
think out now.
Luiz, I am going to respin V2 as above, can I have you confirm on it?
I'm honestly a bit confused with what you're suggesting. I'd just respin the
patches and restart the discussion.
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-5-20 10:39, Wenchao Xia 写道:
于 2013-5-17 20:30, Luiz Capitulino 写道:
On Fri, 17 May 2013 11:30:31 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-16 20:17, Luiz Capitulino 写道:
On Thu, 16 May 2013 10:22:09 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-15 20:28, Luiz Capitulino 写道:
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a
reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable
cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess
that is the
question.
I'd have to see the code to tell, but yes, what Stefan
described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants
more than one
human monitor console, and then we may require a data
structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no
issues for it.
As I said before, I'd have to see the code to tell. But
answering your comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now,
waiting for
your confirm.
Can you point me to the code in question?
Sure, it is
+
+/*
+ * 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);
+}
This function used global variable cur_mon instead of input
parameter,
similar to error_printf().
Why do you need it? Why can't you you use error_printf() for example?
error_printf() will print out to stderr in qemu-img, but stdout is
wanted for those dump info function.
You can refactor the code so that you can pass a FILE *stream argument to
error_vprintf() and maybe add error_printf_stream()?
The name is a bit confusing, maybe qemu_printf()? Another problem is,
monitor have a buf[] instead of a FILE*, I think it need a structure
include those:
typdef enum QemuOutputType {
QEMU_OUTPUT_TYPE_STREAM,
QEMU_OUTPUT_TYPE_MONITOR,
} QemuOutputType;
typedef struct QemuOutput {
QemuOutputType type;
union {
FILE *file;
Monitor *mon;
};
}
It may brings some inconvienience to caller, but this is what I can
think out now.
Luiz, I am going to respin V2 as above, can I have you confirm on it?
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-5-17 20:30, Luiz Capitulino 写道:
On Fri, 17 May 2013 11:30:31 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-16 20:17, Luiz Capitulino 写道:
On Thu, 16 May 2013 10:22:09 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-15 20:28, Luiz Capitulino 写道:
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
As I said before, I'd have to see the code to tell. But answering your comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now, waiting for
your confirm.
Can you point me to the code in question?
Sure, it is
+
+/*
+ * 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);
+}
This function used global variable cur_mon instead of input parameter,
similar to error_printf().
Why do you need it? Why can't you you use error_printf() for example?
error_printf() will print out to stderr in qemu-img, but stdout is
wanted for those dump info function.
You can refactor the code so that you can pass a FILE *stream argument to
error_vprintf() and maybe add error_printf_stream()?
The name is a bit confusing, maybe qemu_printf()? Another problem is,
monitor have a buf[] instead of a FILE*, I think it need a structure
include those:
typdef enum QemuOutputType {
QEMU_OUTPUT_TYPE_STREAM,
QEMU_OUTPUT_TYPE_MONITOR,
} QemuOutputType;
typedef struct QemuOutput {
QemuOutputType type;
union {
FILE *file;
Monitor *mon;
};
}
It may brings some inconvienience to caller, but this is what I can
think out now.
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Fri, 17 May 2013 11:30:31 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-16 20:17, Luiz Capitulino 写道:
On Thu, 16 May 2013 10:22:09 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-15 20:28, Luiz Capitulino 写道:
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is
the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than
one
human monitor console, and then we may require a data structure to
tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for
it.
As I said before, I'd have to see the code to tell. But answering your
comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now, waiting for
your confirm.
Can you point me to the code in question?
Sure, it is
+
+/*
+ * 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);
+}
This function used global variable cur_mon instead of input parameter,
similar to error_printf().
Why do you need it? Why can't you you use error_printf() for example?
error_printf() will print out to stderr in qemu-img, but stdout is
wanted for those dump info function.
You can refactor the code so that you can pass a FILE *stream argument to
error_vprintf() and maybe add error_printf_stream()?
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Thu, 16 May 2013 10:22:09 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-15 20:28, Luiz Capitulino 写道:
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
As I said before, I'd have to see the code to tell. But answering your
comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now, waiting for
your confirm.
Can you point me to the code in question?
Sure, it is
+
+/*
+ * 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);
+}
This function used global variable cur_mon instead of input parameter,
similar to error_printf().
Why do you need it? Why can't you you use error_printf() for example?
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-5-16 20:17, Luiz Capitulino 写道:
On Thu, 16 May 2013 10:22:09 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-15 20:28, Luiz Capitulino 写道:
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
As I said before, I'd have to see the code to tell. But answering your comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now, waiting for
your confirm.
Can you point me to the code in question?
Sure, it is
+
+/*
+ * 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);
+}
This function used global variable cur_mon instead of input parameter,
similar to error_printf().
Why do you need it? Why can't you you use error_printf() for example?
error_printf() will print out to stderr in qemu-img, but stdout is
wanted for those dump info function.
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
As I said before, I'd have to see the code to tell. But answering your
comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now, waiting for
your confirm.
Can you point me to the code in question?
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-5-15 20:28, Luiz Capitulino 写道:
On Wed, 15 May 2013 10:10:37 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
As I said before, I'd have to see the code to tell. But answering your comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now, waiting for
your confirm.
Can you point me to the code in question?
Sure, it is
+
+/*
+ * 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);
+}
This function used global variable cur_mon instead of input parameter,
similar to error_printf().
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-5-6 21:22, Luiz Capitulino 写道:
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
As I said before, I'd have to see the code to tell. But answering your comment,
the code does support multiple monitors.
Hi Luiz,
Sorry to ask again, do you think method above is OK now, waiting for
your confirm.
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Mon, 06 May 2013 10:09:43 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
As I said before, I'd have to see the code to tell. But answering your comment,
the code does support multiple monitors.
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-5-3 10:51, Wenchao Xia 写道:
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside,
instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
Luiz, what is your idea? I'd like to respin v2 if no issues for it.
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside, instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-5-2 20:02, Luiz Capitulino 写道:
On Thu, 02 May 2013 10:05:08 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside, instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
I'd have to see the code to tell, but yes, what Stefan described is the
best practice for the Monitor.
I think this would not be a problem until qemu wants more than one
human monitor console, and then we may require a data structure to tell
where to output the string: stdout, *mon, or even stderr, and
error_printf() also need to be changed.
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-4-30 3:05, Luiz Capitulino 写道:
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
bdrv_snapshot_dump() used a global variable cur_mon inside, instead
of let caller pass in a explicit montior* mon, I guess that is the
question.
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi stefa...@gmail.com wrote:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
where are they being mixed?
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
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()
TODO?
Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()
于 2013-4-26 22:46, Stefan Hajnoczi 写道:
On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:
@@ -2586,10 +2585,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);
Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls? I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.
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()
TODO?
Same with error_vprintf's comments:
/*
* Print to current monitor if we have one, else to stderr.
* TODO should return int, so callers can calculate width, but that
* requires surgery to monitor_vprintf(). Left for another day.
*/
--
Best Regards
Wenchao Xia
