[Qemu-devel] "console: Properly switch consoles for screen dumps" breaks qxl screen dump

[Qemu-devel] "console: Properly switch consoles for screen dumps" breaks qxl screen dump

[Alon Levy]

Hi Jan,

 I've recently found the $SUBJECT, it's commit
  f81bdefb63243e82d16ce49332f7cf74d10b8f27.

 I'd like to fix it without breaking anything, can you provide me with
 the test that your original patch fixed?

Alon

p.s. for a simple command line with a single console (not even sure how
to get multiple) the following fixes my problem, I expect it doesn't
introduce any problems?


diff --git a/console.c b/console.c
index 6dfcc47..5a709fe 100644
--- a/console.c
+++ b/console.c
@@ -1067,6 +1067,10 @@ void console_select(unsigned int index)
 
     if (index >= MAX_CONSOLES)
         return;
+    if (active_console == consoles[index]) {
+        fprintf(stderr, "not changing console, not required\n");
+        return;
+    }
     if (active_console) {
         active_console->g_width = ds_get_width(active_console->ds);
         active_console->g_height = ds_get_height(active_console->ds);

Re: [Qemu-devel] "console: Properly switch consoles for screen dumps" breaks qxl screen dump

[Jan Kiszka]

On 2011-10-11 11:36, Alon Levy wrote:
> Hi Jan,
> 
>  I've recently found the $SUBJECT, it's commit
>   f81bdefb63243e82d16ce49332f7cf74d10b8f27.
> 
>  I'd like to fix it without breaking anything, can you provide me with
>  the test that your original patch fixed?

Have monitor on virtual console, switch to monitor and trigger screen
dump. So far that caused the to be dumped screen been written to the
monitor console, the console was furthermore resized, but all that mess
was left behind once dumping was finished. No we switch to the graphic
console first and restore the monitor afterwards.

> 
> Alon
> 
> p.s. for a simple command line with a single console (not even sure how
> to get multiple) the following fixes my problem, I expect it doesn't
> introduce any problems?
> 
> 
> diff --git a/console.c b/console.c
> index 6dfcc47..5a709fe 100644
> --- a/console.c
> +++ b/console.c
> @@ -1067,6 +1067,10 @@ void console_select(unsigned int index)
>  
>      if (index >= MAX_CONSOLES)
>          return;
> +    if (active_console == consoles[index]) {
> +        fprintf(stderr, "not changing console, not required\n");
> +        return;
> +    }
>      if (active_console) {
>          active_console->g_width = ds_get_width(active_console->ds);
>          active_console->g_height = ds_get_height(active_console->ds);

I suspect this papers over some QXL bug. SDL has no issues when I
trigger the screen dump from a monitor console that requires no switching.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] "console: Properly switch consoles for screen dumps" breaks qxl screen dump

[Alon Levy]

On Tue, Oct 11, 2011 at 07:36:27PM +0200, Jan Kiszka wrote:
> On 2011-10-11 11:36, Alon Levy wrote:
> > Hi Jan,
> > 
> >  I've recently found the $SUBJECT, it's commit
> >   f81bdefb63243e82d16ce49332f7cf74d10b8f27.
> > 
> >  I'd like to fix it without breaking anything, can you provide me with
> >  the test that your original patch fixed?
> 
> Have monitor on virtual console, switch to monitor and trigger screen
> dump. So far that caused the to be dumped screen been written to the
> monitor console, the console was furthermore resized, but all that mess
> was left behind once dumping was finished. No we switch to the graphic
> console first and restore the monitor afterwards.

I don't follow. Can you provide a qemu command line? how do I run the monitor
on a virtual console?

> 
> > 
> > Alon
> > 
> > p.s. for a simple command line with a single console (not even sure how
> > to get multiple) the following fixes my problem, I expect it doesn't
> > introduce any problems?
> > 
> > 
> > diff --git a/console.c b/console.c
> > index 6dfcc47..5a709fe 100644
> > --- a/console.c
> > +++ b/console.c
> > @@ -1067,6 +1067,10 @@ void console_select(unsigned int index)
> >  
> >      if (index >= MAX_CONSOLES)
> >          return;
> > +    if (active_console == consoles[index]) {
> > +        fprintf(stderr, "not changing console, not required\n");
> > +        return;
> > +    }
> >      if (active_console) {
> >          active_console->g_width = ds_get_width(active_console->ds);
> >          active_console->g_height = ds_get_height(active_console->ds);
> 
> I suspect this papers over some QXL bug. SDL has no issues when I
> trigger the screen dump from a monitor console that requires no switching.

You are absolutely right. So far I haven't managed to find the exact bug
(I have been looking without applying this patch). So I've sent this
patch anyway. the rest of console_select reallocates the surface, which
is not terrible (this isn't high frequency) but still redundant.

> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] "console: Properly switch consoles for screen dumps" breaks qxl screen dump

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2284 bytes --]

On 2011-10-11 19:44, Alon Levy wrote:
> On Tue, Oct 11, 2011 at 07:36:27PM +0200, Jan Kiszka wrote:
>> On 2011-10-11 11:36, Alon Levy wrote:
>>> Hi Jan,
>>>
>>>  I've recently found the $SUBJECT, it's commit
>>>   f81bdefb63243e82d16ce49332f7cf74d10b8f27.
>>>
>>>  I'd like to fix it without breaking anything, can you provide me with
>>>  the test that your original patch fixed?
>>
>> Have monitor on virtual console, switch to monitor and trigger screen
>> dump. So far that caused the to be dumped screen been written to the
>> monitor console, the console was furthermore resized, but all that mess
>> was left behind once dumping was finished. No we switch to the graphic
>> console first and restore the monitor afterwards.
> 
> I don't follow. Can you provide a qemu command line? how do I run the monitor
> on a virtual console?

It's there by default unless you specify something else via -mon[itor].
In that case you can add another one via -monitor vc.

> 
>>
>>>
>>> Alon
>>>
>>> p.s. for a simple command line with a single console (not even sure how
>>> to get multiple) the following fixes my problem, I expect it doesn't
>>> introduce any problems?
>>>
>>>
>>> diff --git a/console.c b/console.c
>>> index 6dfcc47..5a709fe 100644
>>> --- a/console.c
>>> +++ b/console.c
>>> @@ -1067,6 +1067,10 @@ void console_select(unsigned int index)
>>>  
>>>      if (index >= MAX_CONSOLES)
>>>          return;
>>> +    if (active_console == consoles[index]) {
>>> +        fprintf(stderr, "not changing console, not required\n");
>>> +        return;
>>> +    }
>>>      if (active_console) {
>>>          active_console->g_width = ds_get_width(active_console->ds);
>>>          active_console->g_height = ds_get_height(active_console->ds);
>>
>> I suspect this papers over some QXL bug. SDL has no issues when I
>> trigger the screen dump from a monitor console that requires no switching.
> 
> You are absolutely right. So far I haven't managed to find the exact bug
> (I have been looking without applying this patch). So I've sent this
> patch anyway. the rest of console_select reallocates the surface, which
> is not terrible (this isn't high frequency) but still redundant.

Yeah, I see. No concerns.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]