diff mbox

creen dump not supported when no console

Message ID 1321781468-11126-1-git-send-email-mars@linux.vnet.ibm.com
State New
Headers show

Commit Message

Mars.cao Nov. 20, 2011, 9:31 a.m. UTC
I have tested the issue use "-vga none -nographic" option.
QEMU segment faults.

But i think there is no any text and graphic console created in this case.
The console[0] is NULL.

The vga_hw_screen_dump() should return before console_select().
What do you think?


---
 console.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Paolo Bonzini Nov. 20, 2011, 11:02 a.m. UTC | #1
On 11/20/2011 10:31 AM, Cao,Bing Bu wrote:
> +    } else {
> +	fprintf(stderr,"no any console,could not screen dump \n");
> +	return;

This should be error_printf.

Paolo
Markus Armbruster Nov. 21, 2011, 12:30 p.m. UTC | #2
"Cao,Bing Bu" <mars@linux.vnet.ibm.com> writes:

> I have tested the issue use "-vga none -nographic" option.
> QEMU segment faults.
>
> But i think there is no any text and graphic console created in this case.
> The console[0] is NULL.
>
> The vga_hw_screen_dump() should return before console_select().
> What do you think?
>
>
> ---
>  console.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/console.c b/console.c
> index f6fe441..957948e 100644
> --- a/console.c
> +++ b/console.c
> @@ -184,6 +184,9 @@ void vga_hw_screen_dump(const char *filename)
>      console_select(0);
>      if (consoles[0] && consoles[0]->hw_screen_dump) {
>          consoles[0]->hw_screen_dump(consoles[0]->hw, filename);
> +    } else {
> +	fprintf(stderr,"no any console,could not screen dump \n");
> +	return;
>      }
>  
>      console_select(previous_active_console->index);

Tab damage.

Subject misspells "screendump".

Error message could use some polish.  Suggest "Can't screendump without
a console".

Please don't use printf & friends for output to the monitor user, it
doesn't go to the monitor unless the monitor happens to be on stdio.
Use error_printf() & friends.

Except when the command is in QMP, and you need nice error reports in
QMP.  Then use qerror_report().  Yes, that's a pain to use.

Except when the command is already implemented in QAPI.  Then you use
error_set() instead.  Yes, that one's a PITA, too.
diff mbox

Patch

diff --git a/console.c b/console.c
index f6fe441..957948e 100644
--- a/console.c
+++ b/console.c
@@ -184,6 +184,9 @@  void vga_hw_screen_dump(const char *filename)
     console_select(0);
     if (consoles[0] && consoles[0]->hw_screen_dump) {
         consoles[0]->hw_screen_dump(consoles[0]->hw, filename);
+    } else {
+	fprintf(stderr,"no any console,could not screen dump \n");
+	return;
     }
 
     console_select(previous_active_console->index);