* [Qemu-devel] [PATCH] fix monitor
@ 2013-03-19 14:04 Gerd Hoffmann
2013-03-19 14:45 ` Laszlo Ersek
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Gerd Hoffmann @ 2013-03-19 14:04 UTC (permalink / raw)
To: qemu-devel
Cc: amit.shah, Markus Armbruster, Gerd Hoffmann, anthony,
Luiz Capitulino
chardev flow control broke monitor, fix it by adding watch support.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
v2: fix tyops
---
monitor.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/monitor.c b/monitor.c
index 112e920..74807f9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -261,11 +261,30 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
}
}
+static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
+ void *opaque)
+{
+ monitor_flush(opaque);
+ return FALSE;
+}
+
void monitor_flush(Monitor *mon)
{
+ int rc;
+
if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
- qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
- mon->outbuf_index = 0;
+ rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
+ if (rc == mon->outbuf_index) {
+ /* all flushed */
+ mon->outbuf_index = 0;
+ return;
+ }
+ if (rc > 0) {
+ /* partial write */
+ memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
+ mon->outbuf_index -= rc;
+ }
+ qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
}
}
--
1.7.9.7
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 14:04 [Qemu-devel] [PATCH] fix monitor Gerd Hoffmann
@ 2013-03-19 14:45 ` Laszlo Ersek
2013-03-19 15:11 ` Gerd Hoffmann
2013-03-19 17:40 ` Markus Armbruster
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Laszlo Ersek @ 2013-03-19 14:45 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: amit.shah, Luiz Capitulino, qemu-devel, anthony,
Markus Armbruster
On 03/19/13 15:04, Gerd Hoffmann wrote:
> chardev flow control broke monitor, fix it by adding watch support.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> v2: fix tyops
Well played, Sir :)
> ---
> monitor.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 112e920..74807f9 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -261,11 +261,30 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> }
> }
>
> +static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
> + void *opaque)
> +{
> + monitor_flush(opaque);
> + return FALSE;
> +}
> +
> void monitor_flush(Monitor *mon)
> {
> + int rc;
> +
> if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
> - qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
> - mon->outbuf_index = 0;
> + rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
> + if (rc == mon->outbuf_index) {
> + /* all flushed */
> + mon->outbuf_index = 0;
> + return;
> + }
> + if (rc > 0) {
> + /* partial write */
> + memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
> + mon->outbuf_index -= rc;
> + }
> + qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
> }
> }
>
>
Looks good to me.
Should we maybe forego (re)installing the (new) watch when rc<0?
(Especially because on a persistent error the fd might immediately
report writability.)
Laszlo
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 14:45 ` Laszlo Ersek
@ 2013-03-19 15:11 ` Gerd Hoffmann
0 siblings, 0 replies; 13+ messages in thread
From: Gerd Hoffmann @ 2013-03-19 15:11 UTC (permalink / raw)
To: Laszlo Ersek
Cc: amit.shah, Luiz Capitulino, qemu-devel, anthony,
Markus Armbruster
On 03/19/13 15:45, Laszlo Ersek wrote:
> On 03/19/13 15:04, Gerd Hoffmann wrote:
>> chardev flow control broke monitor, fix it by adding watch support.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> v2: fix tyops
>
> Well played, Sir :)
>
>> ---
>> monitor.c | 23 +++++++++++++++++++++--
>> 1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 112e920..74807f9 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -261,11 +261,30 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>> }
>> }
>>
>> +static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
>> + void *opaque)
>> +{
>> + monitor_flush(opaque);
>> + return FALSE;
>> +}
>> +
>> void monitor_flush(Monitor *mon)
>> {
>> + int rc;
>> +
>> if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
>> - qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
>> - mon->outbuf_index = 0;
>> + rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
>> + if (rc == mon->outbuf_index) {
>> + /* all flushed */
>> + mon->outbuf_index = 0;
>> + return;
>> + }
>> + if (rc > 0) {
>> + /* partial write */
>> + memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
>> + mon->outbuf_index -= rc;
>> + }
>> + qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
>> }
>> }
>>
>>
>
> Looks good to me.
>
> Should we maybe forego (re)installing the (new) watch when rc<0?
> (Especially because on a persistent error the fd might immediately
> report writability.)
Yes, it is probably wise to check for errno == EAGAIN (assuming
qemu_chr_fe_write makes sure errno it set to something sensible on error).
cheers,
Gerd
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 14:04 [Qemu-devel] [PATCH] fix monitor Gerd Hoffmann
2013-03-19 14:45 ` Laszlo Ersek
@ 2013-03-19 17:40 ` Markus Armbruster
2013-03-19 19:00 ` Anthony Liguori
2013-03-19 18:58 ` Anthony Liguori
2013-04-03 12:17 ` Peter Lieven
3 siblings, 1 reply; 13+ messages in thread
From: Markus Armbruster @ 2013-03-19 17:40 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: amit.shah, qemu-devel, anthony, Luiz Capitulino
Gerd Hoffmann <kraxel@redhat.com> writes:
> chardev flow control broke monitor, fix it by adding watch support.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> v2: fix tyops
> ---
Subject lacks v2. Anthony, holler if you want a respin to unconfuse
your tools.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 17:40 ` Markus Armbruster
@ 2013-03-19 19:00 ` Anthony Liguori
0 siblings, 0 replies; 13+ messages in thread
From: Anthony Liguori @ 2013-03-19 19:00 UTC (permalink / raw)
To: Markus Armbruster, Gerd Hoffmann; +Cc: amit.shah, qemu-devel, Luiz Capitulino
Markus Armbruster <armbru@redhat.com> writes:
> Gerd Hoffmann <kraxel@redhat.com> writes:
>
>> chardev flow control broke monitor, fix it by adding watch support.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> v2: fix tyops
>> ---
>
> Subject lacks v2. Anthony, holler if you want a respin to unconfuse
> your tools.
I already processed this patch--specifically this copy of it. Newest
mail always wins when there's a conflict but please don't rely on that
:-)
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 14:04 [Qemu-devel] [PATCH] fix monitor Gerd Hoffmann
2013-03-19 14:45 ` Laszlo Ersek
2013-03-19 17:40 ` Markus Armbruster
@ 2013-03-19 18:58 ` Anthony Liguori
2013-04-03 12:17 ` Peter Lieven
3 siblings, 0 replies; 13+ messages in thread
From: Anthony Liguori @ 2013-03-19 18:58 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: amit.shah, Luiz Capitulino, Markus Armbruster, anthony
Applied. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 14:04 [Qemu-devel] [PATCH] fix monitor Gerd Hoffmann
` (2 preceding siblings ...)
2013-03-19 18:58 ` Anthony Liguori
@ 2013-04-03 12:17 ` Peter Lieven
2013-04-03 12:35 ` Gerd Hoffmann
3 siblings, 1 reply; 13+ messages in thread
From: Peter Lieven @ 2013-04-03 12:17 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Markus Armbruster, qemu-devel, Luiz Capitulino, anthony,
amit.shah
Hi Gerd,
I today saw this assert when live migrating a VM. Is this related? The
below patch was already applied.
qemu-1.4.5: /usr/src/qemu-1.4.5/monitor.c:297: monitor_puts: Assertion
`mon->outbuf_index < sizeof(mon->outbuf) - 1' failed.
Peter
Gerd Hoffmann wrote:
> chardev flow control broke monitor, fix it by adding watch support.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> v2: fix tyops
> ---
> monitor.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 112e920..74807f9 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -261,11 +261,30 @@ int monitor_read_password(Monitor *mon, ReadLineFunc
> *readline_func,
> }
> }
>
> +static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
> + void *opaque)
> +{
> + monitor_flush(opaque);
> + return FALSE;
> +}
> +
> void monitor_flush(Monitor *mon)
> {
> + int rc;
> +
> if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
> - qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
> - mon->outbuf_index = 0;
> + rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
> + if (rc == mon->outbuf_index) {
> + /* all flushed */
> + mon->outbuf_index = 0;
> + return;
> + }
> + if (rc > 0) {
> + /* partial write */
> + memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index -
> rc);
> + mon->outbuf_index -= rc;
> + }
> + qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked,
> mon);
> }
> }
>
> --
> 1.7.9.7
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH] fix monitor
2013-04-03 12:17 ` Peter Lieven
@ 2013-04-03 12:35 ` Gerd Hoffmann
2013-04-03 13:36 ` Luiz Capitulino
0 siblings, 1 reply; 13+ messages in thread
From: Gerd Hoffmann @ 2013-04-03 12:35 UTC (permalink / raw)
To: Peter Lieven
Cc: amit.shah, Luiz Capitulino, qemu-devel, anthony,
Markus Armbruster
On 04/03/13 14:17, Peter Lieven wrote:
> Hi Gerd,
>
> I today saw this assert when live migrating a VM. Is this related? The
> below patch was already applied.
>
> qemu-1.4.5: /usr/src/qemu-1.4.5/monitor.c:297: monitor_puts: Assertion
> `mon->outbuf_index < sizeof(mon->outbuf) - 1' failed.
Probably, patches (from luiz) are on the list to make the monitor buffer
size dynamic, they should fix it.
cheers,
Gerd
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] fix monitor
2013-04-03 12:35 ` Gerd Hoffmann
@ 2013-04-03 13:36 ` Luiz Capitulino
2013-04-04 5:42 ` Peter Lieven
0 siblings, 1 reply; 13+ messages in thread
From: Luiz Capitulino @ 2013-04-03 13:36 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: amit.shah, Peter Lieven, qemu-devel, anthony, Markus Armbruster
On Wed, 03 Apr 2013 14:35:46 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 04/03/13 14:17, Peter Lieven wrote:
> > Hi Gerd,
> >
> > I today saw this assert when live migrating a VM. Is this related? The
> > below patch was already applied.
> >
> > qemu-1.4.5: /usr/src/qemu-1.4.5/monitor.c:297: monitor_puts: Assertion
> > `mon->outbuf_index < sizeof(mon->outbuf) - 1' failed.
>
> Probably, patches (from luiz) are on the list to make the monitor buffer
> size dynamic, they should fix it.
Yes.
Peter, can you please try my series and report if it really fixes the
assertion for you?
http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg00369.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH] fix monitor
2013-04-03 13:36 ` Luiz Capitulino
@ 2013-04-04 5:42 ` Peter Lieven
0 siblings, 0 replies; 13+ messages in thread
From: Peter Lieven @ 2013-04-04 5:42 UTC (permalink / raw)
To: qemu-devel
On 03.04.2013 15:36, Luiz Capitulino wrote:
> On Wed, 03 Apr 2013 14:35:46 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>> On 04/03/13 14:17, Peter Lieven wrote:
>>> Hi Gerd,
>>>
>>> I today saw this assert when live migrating a VM. Is this related? The
>>> below patch was already applied.
>>>
>>> qemu-1.4.5: /usr/src/qemu-1.4.5/monitor.c:297: monitor_puts: Assertion
>>> `mon->outbuf_index < sizeof(mon->outbuf) - 1' failed.
>> Probably, patches (from luiz) are on the list to make the monitor buffer
>> size dynamic, they should fix it.
> Yes.
>
> Peter, can you please try my series and report if it really fixes the
> assertion for you?
>
> http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg00369.html
>
I will do, but the assertion was not as easy triggerable for me as typing '?'.
We run some tests today with the patches applied and I will let you know
if there where any further assertions.
Peter
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH] fix monitor
@ 2013-03-19 9:57 Gerd Hoffmann
2013-03-19 13:58 ` Hans de Goede
2013-03-19 18:58 ` Anthony Liguori
0 siblings, 2 replies; 13+ messages in thread
From: Gerd Hoffmann @ 2013-03-19 9:57 UTC (permalink / raw)
To: qemu-devel
Cc: amit.shah, Markus Armbruster, Gerd Hoffmann, anthony,
Luiz Capitulino
chardev flow control broke monitor, fix it by adding watch support.
---
monitor.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/monitor.c b/monitor.c
index 112e920..680d344 100644
--- a/monitor.c
+++ b/monitor.c
@@ -261,11 +261,30 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
}
}
+static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
+ void *opaque)
+{
+ monitor_flush(opaque);
+ return FALSE;
+}
+
void monitor_flush(Monitor *mon)
{
+ int rc;
+
if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
- qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
- mon->outbuf_index = 0;
+ rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
+ if (rc == mon->outbuf_index) {
+ /* all flushed */
+ mon->outbuf_index = 0;
+ return;
+ }
+ if (rc > 0) {
+ /* partinal write */
+ memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
+ mon->outbuf_index -= rc;
+ }
+ qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
}
}
--
1.7.9.7
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 9:57 Gerd Hoffmann
@ 2013-03-19 13:58 ` Hans de Goede
2013-03-19 18:58 ` Anthony Liguori
1 sibling, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2013-03-19 13:58 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: amit.shah, Luiz Capitulino, qemu-devel, anthony,
Markus Armbruster
Hi,
Looks good, one minor nitpick.
On 03/19/2013 10:57 AM, Gerd Hoffmann wrote:
> chardev flow control broke monitor, fix it by adding watch support.
> ---
> monitor.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 112e920..680d344 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -261,11 +261,30 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> }
> }
>
> +static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
> + void *opaque)
> +{
> + monitor_flush(opaque);
> + return FALSE;
> +}
> +
> void monitor_flush(Monitor *mon)
> {
> + int rc;
> +
> if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
> - qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
> - mon->outbuf_index = 0;
> + rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
> + if (rc == mon->outbuf_index) {
> + /* all flushed */
> + mon->outbuf_index = 0;
> + return;
> + }
> + if (rc > 0) {
> + /* partinal write */
Typo in the comment.
> + memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
> + mon->outbuf_index -= rc;
> + }
> + qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
> }
> }
>
>
Regards,
Hans
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH] fix monitor
2013-03-19 9:57 Gerd Hoffmann
2013-03-19 13:58 ` Hans de Goede
@ 2013-03-19 18:58 ` Anthony Liguori
1 sibling, 0 replies; 13+ messages in thread
From: Anthony Liguori @ 2013-03-19 18:58 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: amit.shah, Luiz Capitulino, Markus Armbruster, anthony
Applied. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-04-04 5:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-19 14:04 [Qemu-devel] [PATCH] fix monitor Gerd Hoffmann
2013-03-19 14:45 ` Laszlo Ersek
2013-03-19 15:11 ` Gerd Hoffmann
2013-03-19 17:40 ` Markus Armbruster
2013-03-19 19:00 ` Anthony Liguori
2013-03-19 18:58 ` Anthony Liguori
2013-04-03 12:17 ` Peter Lieven
2013-04-03 12:35 ` Gerd Hoffmann
2013-04-03 13:36 ` Luiz Capitulino
2013-04-04 5:42 ` Peter Lieven
-- strict thread matches above, loose matches on Subject: below --
2013-03-19 9:57 Gerd Hoffmann
2013-03-19 13:58 ` Hans de Goede
2013-03-19 18:58 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).