* [Qemu-devel] [PATCH 1/2] qapi: Convert screendump
@ 2012-03-14 15:55 Alon Levy
2012-03-14 15:55 ` [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure Alon Levy
2012-03-14 17:33 ` [Qemu-devel] [PATCH 1/2] qapi: Convert screendump Luiz Capitulino
0 siblings, 2 replies; 6+ messages in thread
From: Alon Levy @ 2012-03-14 15:55 UTC (permalink / raw)
To: qemu-devel, lcapitulino; +Cc: mlureau
From: Luiz Capitulino <lcapitulino@redhat.com>
Also, makes it possible for devices to report errors in the
hw_screen_dump() callback.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
console.c | 5 +++--
console.h | 6 ++++--
hmp-commands.hx | 3 +--
hmp.c | 8 ++++++++
hmp.h | 1 +
hw/blizzard.c | 3 ++-
hw/g364fb.c | 4 +++-
hw/omap_lcdc.c | 4 +++-
hw/qxl.c | 6 ++++--
hw/tcx.c | 13 +++++++++----
hw/vga.c | 7 +++++--
hw/vmware_vga.c | 6 ++++--
monitor.c | 6 ------
qapi-schema.json | 13 +++++++++++++
qmp-commands.hx | 5 +----
qmp.c | 5 +++++
16 files changed, 66 insertions(+), 29 deletions(-)
diff --git a/console.c b/console.c
index 6a463f5..d3fccf3 100644
--- a/console.c
+++ b/console.c
@@ -24,6 +24,7 @@
#include "qemu-common.h"
#include "console.h"
#include "qemu-timer.h"
+#include "error.h"
//#define DEBUG_CONSOLE
#define DEFAULT_BACKSCROLL 512
@@ -173,7 +174,7 @@ void vga_hw_invalidate(void)
active_console->hw_invalidate(active_console->hw);
}
-void vga_hw_screen_dump(const char *filename)
+void vga_hw_screen_dump(const char *filename, Error **errp)
{
TextConsole *previous_active_console;
bool cswitch;
@@ -187,7 +188,7 @@ 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, cswitch);
+ consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch, errp);
} else {
error_report("screen dump not implemented");
}
diff --git a/console.h b/console.h
index 4334db5..caf13f5 100644
--- a/console.h
+++ b/console.h
@@ -6,6 +6,7 @@
#include "notify.h"
#include "monitor.h"
#include "trace.h"
+#include "error.h"
/* keyboard/mouse support */
@@ -343,7 +344,8 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
typedef void (*vga_hw_update_ptr)(void *);
typedef void (*vga_hw_invalidate_ptr)(void *);
-typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch);
+typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch,
+ Error **errp);
typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *);
DisplayState *graphic_console_init(vga_hw_update_ptr update,
@@ -354,7 +356,7 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update,
void vga_hw_update(void);
void vga_hw_invalidate(void);
-void vga_hw_screen_dump(const char *filename);
+void vga_hw_screen_dump(const char *filename, Error **errp);
void vga_hw_text_update(console_ch_t *chardata);
int is_graphic_console(void);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 6980214..d26421a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -194,8 +194,7 @@ ETEXI
.args_type = "filename:F",
.params = "filename",
.help = "save screen into PPM image 'filename'",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_screen_dump,
+ .mhandler.cmd = hmp_screendump,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 290c43d..42dc79a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -860,3 +860,11 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &error);
}
+
+void hmp_screendump(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+
+ qmp_screendump(qdict_get_str(qdict, "filename"), &err);
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 5409464..25d123f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -59,5 +59,6 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
void hmp_block_stream(Monitor *mon, const QDict *qdict);
void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
+void hmp_screendump(Monitor *mon, const QDict *qdict);
#endif
diff --git a/hw/blizzard.c b/hw/blizzard.c
index c7d844d..76df78c 100644
--- a/hw/blizzard.c
+++ b/hw/blizzard.c
@@ -23,6 +23,7 @@
#include "devices.h"
#include "vga_int.h"
#include "pixel_ops.h"
+#include "error.h"
typedef void (*blizzard_fn_t)(uint8_t *, const uint8_t *, unsigned int);
@@ -933,7 +934,7 @@ static void blizzard_update_display(void *opaque)
}
static void blizzard_screen_dump(void *opaque, const char *filename,
- bool cswitch)
+ bool cswitch, Error **errp)
{
BlizzardState *s = (BlizzardState *) opaque;
diff --git a/hw/g364fb.c b/hw/g364fb.c
index 3a0b68f..7774d05 100644
--- a/hw/g364fb.c
+++ b/hw/g364fb.c
@@ -22,6 +22,7 @@
#include "pixel_ops.h"
#include "trace.h"
#include "sysbus.h"
+#include "error.h"
typedef struct G364State {
/* hardware */
@@ -289,7 +290,8 @@ static void g364fb_reset(G364State *s)
g364fb_invalidate_display(s);
}
-static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
G364State *s = opaque;
int y, x;
diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
index f172093..aec7210 100644
--- a/hw/omap_lcdc.c
+++ b/hw/omap_lcdc.c
@@ -20,6 +20,7 @@
#include "console.h"
#include "omap.h"
#include "framebuffer.h"
+#include "error.h"
struct omap_lcd_panel_s {
MemoryRegion *sysmem;
@@ -264,7 +265,8 @@ static int ppm_save(const char *filename, uint8_t *data,
return 0;
}
-static void omap_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void omap_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
struct omap_lcd_panel_s *omap_lcd = opaque;
if (cswitch) {
diff --git a/hw/qxl.c b/hw/qxl.c
index e17b0e3..27f27f5 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -23,6 +23,7 @@
#include "qemu-queue.h"
#include "monitor.h"
#include "sysemu.h"
+#include "error.h"
#include "qxl.h"
@@ -1492,7 +1493,8 @@ static void qxl_hw_invalidate(void *opaque)
vga->invalidate(vga);
}
-static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
PCIQXLDevice *qxl = opaque;
VGACommonState *vga = &qxl->vga;
@@ -1504,7 +1506,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
ppm_save(filename, qxl->ssd.ds->surface);
break;
case QXL_MODE_VGA:
- vga->screen_dump(vga, filename, cswitch);
+ vga->screen_dump(vga, filename, cswitch, errp);
break;
default:
break;
diff --git a/hw/tcx.c b/hw/tcx.c
index ac7dcb4..f342a5d 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -26,6 +26,7 @@
#include "pixel_ops.h"
#include "sysbus.h"
#include "qdev-addr.h"
+#include "error.h"
#define MAXX 1024
#define MAXY 768
@@ -56,8 +57,10 @@ typedef struct TCXState {
uint8_t dac_index, dac_state;
} TCXState;
-static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch);
-static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch);
+static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error *errp);
+static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error *errp);
static void tcx_set_dirty(TCXState *s)
{
@@ -574,7 +577,8 @@ static int tcx_init1(SysBusDevice *dev)
return 0;
}
-static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error *errp)
{
TCXState *s = opaque;
FILE *f;
@@ -601,7 +605,8 @@ static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
return;
}
-static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error *errp)
{
TCXState *s = opaque;
FILE *f;
diff --git a/hw/vga.c b/hw/vga.c
index 6dc98f6..b80a079 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -31,6 +31,7 @@
#include "qemu-timer.h"
#include "xen.h"
#include "trace.h"
+#include "error.h"
//#define DEBUG_VGA
//#define DEBUG_VGA_MEM
@@ -163,7 +164,8 @@ static uint32_t expand4[256];
static uint16_t expand2[256];
static uint8_t expand4to8[16];
-static void vga_screen_dump(void *opaque, const char *filename, bool cswitch);
+static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error *errp);
static void vga_update_memory_access(VGACommonState *s)
{
@@ -2409,7 +2411,8 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
/* save the vga display in a PPM image even if no display is
available */
-static void vga_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
VGACommonState *s = opaque;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 142d9f4..6868778 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -26,6 +26,7 @@
#include "console.h"
#include "pci.h"
#include "vmware_vga.h"
+#include "error.h"
#undef VERBOSE
#define HW_RECT_ACCEL
@@ -1003,11 +1004,12 @@ static void vmsvga_invalidate_display(void *opaque)
/* save the vga display in a PPM image even if no display is
available */
-static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch)
+static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch,
+ Error **errp)
{
struct vmsvga_state_s *s = opaque;
if (!s->enable) {
- s->vga.screen_dump(&s->vga, filename, cswitch);
+ s->vga.screen_dump(&s->vga, filename, cswitch, errp);
return;
}
diff --git a/monitor.c b/monitor.c
index cbdfbad..f79ce9a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -893,12 +893,6 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
return -1;
}
-static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
- vga_hw_screen_dump(qdict_get_str(qdict, "filename"));
- return 0;
-}
-
static void do_logfile(Monitor *mon, const QDict *qdict)
{
cpu_set_log_filename(qdict_get_str(qdict, "filename"));
diff --git a/qapi-schema.json b/qapi-schema.json
index 04fa84f..4f251ca 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1663,3 +1663,16 @@
{ 'command': 'qom-list-types',
'data': { '*implements': 'str', '*abstract': 'bool' },
'returns': [ 'ObjectTypeInfo' ] }
+
+##
+# @screendump:
+#
+# Write a PPM of the VGA screen to a file.
+#
+# @filename: the name of a new PPM file to create to store the image
+#
+# Returns: Nothing on success
+#
+# Since: 1.1
+##
+{ 'command': 'screendump', 'data': {'filename': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index dfe8a5b..5fe57fd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -146,10 +146,7 @@ EQMP
{
.name = "screendump",
.args_type = "filename:F",
- .params = "filename",
- .help = "save screen into PPM image 'filename'",
- .user_print = monitor_user_noop,
- .mhandler.cmd_new = do_screen_dump,
+ .mhandler.cmd_new = qmp_marshal_input_screendump,
},
SQMP
diff --git a/qmp.c b/qmp.c
index a182b51..086cec8 100644
--- a/qmp.c
+++ b/qmp.c
@@ -415,3 +415,8 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
return ret;
}
+
+void qmp_screendump(const char *filename, Error **errp)
+{
+ vga_hw_screen_dump(filename, errp);
+}
--
1.7.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure
2012-03-14 15:55 [Qemu-devel] [PATCH 1/2] qapi: Convert screendump Alon Levy
@ 2012-03-14 15:55 ` Alon Levy
2012-03-14 17:40 ` Luiz Capitulino
2012-03-14 17:33 ` [Qemu-devel] [PATCH 1/2] qapi: Convert screendump Luiz Capitulino
1 sibling, 1 reply; 6+ messages in thread
From: Alon Levy @ 2012-03-14 15:55 UTC (permalink / raw)
To: qemu-devel, lcapitulino; +Cc: mlureau
From: Luiz Capitulino <lcapitulino@redhat.com>
This makes all devices using ppm_save() return an error appropriately
when the screendump command fails.
Based on a code by Anthony Liguori.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/blizzard.c | 2 +-
hw/qxl.c | 2 +-
hw/vga.c | 10 ++++++----
hw/vga_int.h | 3 ++-
hw/vmware_vga.c | 2 +-
5 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/hw/blizzard.c b/hw/blizzard.c
index 76df78c..29e5ae6 100644
--- a/hw/blizzard.c
+++ b/hw/blizzard.c
@@ -942,7 +942,7 @@ static void blizzard_screen_dump(void *opaque, const char *filename,
blizzard_update_display(opaque);
}
if (s && ds_get_data(s->state))
- ppm_save(filename, s->state->surface);
+ ppm_save(filename, s->state->surface, errp);
}
#define DEPTH 8
diff --git a/hw/qxl.c b/hw/qxl.c
index 27f27f5..aa68612 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1503,7 +1503,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch,
case QXL_MODE_COMPAT:
case QXL_MODE_NATIVE:
qxl_render_update(qxl);
- ppm_save(filename, qxl->ssd.ds->surface);
+ ppm_save(filename, qxl->ssd.ds->surface, errp);
break;
case QXL_MODE_VGA:
vga->screen_dump(vga, filename, cswitch, errp);
diff --git a/hw/vga.c b/hw/vga.c
index b80a079..7e90cf4 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -165,7 +165,7 @@ static uint16_t expand2[256];
static uint8_t expand4to8[16];
static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
- Error *errp);
+ Error **errp);
static void vga_update_memory_access(VGACommonState *s)
{
@@ -2365,7 +2365,7 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion *system_memory)
/********************************************************/
/* vga screen dump */
-int ppm_save(const char *filename, struct DisplaySurface *ds)
+int ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
{
FILE *f;
uint8_t *d, *d1;
@@ -2377,8 +2377,10 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
trace_ppm_save(filename, ds);
f = fopen(filename, "wb");
- if (!f)
+ if (!f) {
+ error_set(errp, QERR_OPEN_FILE_FAILED, filename);
return -1;
+ }
fprintf(f, "P6\n%d %d\n%d\n",
ds->width, ds->height, 255);
linebuf = g_malloc(ds->width * 3);
@@ -2420,5 +2422,5 @@ static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
vga_invalidate_display(s);
vga_hw_update();
}
- ppm_save(filename, s->ds->surface);
+ ppm_save(filename, s->ds->surface, errp);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 7685b2b..63078ba 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -24,6 +24,7 @@
#include <hw/hw.h>
#include "memory.h"
+#include "error.h"
#define ST01_V_RETRACE 0x08
#define ST01_DISP_ENABLE 0x01
@@ -200,7 +201,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val);
uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr);
void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val);
void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
-int ppm_save(const char *filename, struct DisplaySurface *ds);
+int ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp);
int vga_ioport_invalid(VGACommonState *s, uint32_t addr);
void vga_init_vbe(VGACommonState *s, MemoryRegion *address_space);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 6868778..0769652 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1016,7 +1016,7 @@ static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch,
if (s->depth == 32) {
DisplaySurface *ds = qemu_create_displaysurface_from(s->width,
s->height, 32, ds_get_linesize(s->vga.ds), s->vga.vram_ptr);
- ppm_save(filename, ds);
+ ppm_save(filename, ds, errp);
g_free(ds);
}
}
--
1.7.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure
2012-03-14 15:55 ` [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure Alon Levy
@ 2012-03-14 17:40 ` Luiz Capitulino
2012-03-14 18:02 ` Alon Levy
0 siblings, 1 reply; 6+ messages in thread
From: Luiz Capitulino @ 2012-03-14 17:40 UTC (permalink / raw)
To: Alon Levy; +Cc: mlureau, qemu-devel
On Wed, 14 Mar 2012 17:55:33 +0200
Alon Levy <alevy@redhat.com> wrote:
> From: Luiz Capitulino <lcapitulino@redhat.com>
>
> This makes all devices using ppm_save() return an error appropriately
> when the screendump command fails.
>
> Based on a code by Anthony Liguori.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> hw/blizzard.c | 2 +-
> hw/qxl.c | 2 +-
> hw/vga.c | 10 ++++++----
> hw/vga_int.h | 3 ++-
> hw/vmware_vga.c | 2 +-
> 5 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/hw/blizzard.c b/hw/blizzard.c
> index 76df78c..29e5ae6 100644
> --- a/hw/blizzard.c
> +++ b/hw/blizzard.c
> @@ -942,7 +942,7 @@ static void blizzard_screen_dump(void *opaque, const char *filename,
> blizzard_update_display(opaque);
> }
> if (s && ds_get_data(s->state))
> - ppm_save(filename, s->state->surface);
> + ppm_save(filename, s->state->surface, errp);
> }
>
> #define DEPTH 8
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 27f27f5..aa68612 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -1503,7 +1503,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch,
> case QXL_MODE_COMPAT:
> case QXL_MODE_NATIVE:
> qxl_render_update(qxl);
> - ppm_save(filename, qxl->ssd.ds->surface);
> + ppm_save(filename, qxl->ssd.ds->surface, errp);
> break;
> case QXL_MODE_VGA:
> vga->screen_dump(vga, filename, cswitch, errp);
> diff --git a/hw/vga.c b/hw/vga.c
> index b80a079..7e90cf4 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -165,7 +165,7 @@ static uint16_t expand2[256];
> static uint8_t expand4to8[16];
>
> static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> - Error *errp);
> + Error **errp);
>
> static void vga_update_memory_access(VGACommonState *s)
> {
> @@ -2365,7 +2365,7 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion *system_memory)
> /********************************************************/
> /* vga screen dump */
>
> -int ppm_save(const char *filename, struct DisplaySurface *ds)
> +int ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
> {
> FILE *f;
> uint8_t *d, *d1;
> @@ -2377,8 +2377,10 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
>
> trace_ppm_save(filename, ds);
> f = fopen(filename, "wb");
> - if (!f)
> + if (!f) {
> + error_set(errp, QERR_OPEN_FILE_FAILED, filename);
This is a generic error, and it's unfortunate that we're using it in several
places now. The best thing to do here - and I'm including write() in this
discussion - is to report the most possible errors, like EACCESS, ENOSPC,
EPERM, EIO. Maybe not all of them exist as QErrors.
Also, note that some device models have their own way of saving the screen
dump (eg. g364fb_screen_dump()) and should be converted to error_set()
separately.
> return -1;
> + }
> fprintf(f, "P6\n%d %d\n%d\n",
> ds->width, ds->height, 255);
> linebuf = g_malloc(ds->width * 3);
> @@ -2420,5 +2422,5 @@ static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> vga_invalidate_display(s);
> vga_hw_update();
> }
> - ppm_save(filename, s->ds->surface);
> + ppm_save(filename, s->ds->surface, errp);
> }
> diff --git a/hw/vga_int.h b/hw/vga_int.h
> index 7685b2b..63078ba 100644
> --- a/hw/vga_int.h
> +++ b/hw/vga_int.h
> @@ -24,6 +24,7 @@
>
> #include <hw/hw.h>
> #include "memory.h"
> +#include "error.h"
>
> #define ST01_V_RETRACE 0x08
> #define ST01_DISP_ENABLE 0x01
> @@ -200,7 +201,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val);
> uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr);
> void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val);
> void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
> -int ppm_save(const char *filename, struct DisplaySurface *ds);
> +int ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp);
>
> int vga_ioport_invalid(VGACommonState *s, uint32_t addr);
> void vga_init_vbe(VGACommonState *s, MemoryRegion *address_space);
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index 6868778..0769652 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -1016,7 +1016,7 @@ static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch,
> if (s->depth == 32) {
> DisplaySurface *ds = qemu_create_displaysurface_from(s->width,
> s->height, 32, ds_get_linesize(s->vga.ds), s->vga.vram_ptr);
> - ppm_save(filename, ds);
> + ppm_save(filename, ds, errp);
> g_free(ds);
> }
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure
2012-03-14 17:40 ` Luiz Capitulino
@ 2012-03-14 18:02 ` Alon Levy
0 siblings, 0 replies; 6+ messages in thread
From: Alon Levy @ 2012-03-14 18:02 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mlureau, qemu-devel
On Wed, Mar 14, 2012 at 02:40:42PM -0300, Luiz Capitulino wrote:
> On Wed, 14 Mar 2012 17:55:33 +0200
> Alon Levy <alevy@redhat.com> wrote:
>
> > From: Luiz Capitulino <lcapitulino@redhat.com>
> >
> > This makes all devices using ppm_save() return an error appropriately
> > when the screendump command fails.
> >
> > Based on a code by Anthony Liguori.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > hw/blizzard.c | 2 +-
> > hw/qxl.c | 2 +-
> > hw/vga.c | 10 ++++++----
> > hw/vga_int.h | 3 ++-
> > hw/vmware_vga.c | 2 +-
> > 5 files changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/blizzard.c b/hw/blizzard.c
> > index 76df78c..29e5ae6 100644
> > --- a/hw/blizzard.c
> > +++ b/hw/blizzard.c
> > @@ -942,7 +942,7 @@ static void blizzard_screen_dump(void *opaque, const char *filename,
> > blizzard_update_display(opaque);
> > }
> > if (s && ds_get_data(s->state))
> > - ppm_save(filename, s->state->surface);
> > + ppm_save(filename, s->state->surface, errp);
> > }
> >
> > #define DEPTH 8
> > diff --git a/hw/qxl.c b/hw/qxl.c
> > index 27f27f5..aa68612 100644
> > --- a/hw/qxl.c
> > +++ b/hw/qxl.c
> > @@ -1503,7 +1503,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch,
> > case QXL_MODE_COMPAT:
> > case QXL_MODE_NATIVE:
> > qxl_render_update(qxl);
> > - ppm_save(filename, qxl->ssd.ds->surface);
> > + ppm_save(filename, qxl->ssd.ds->surface, errp);
> > break;
> > case QXL_MODE_VGA:
> > vga->screen_dump(vga, filename, cswitch, errp);
> > diff --git a/hw/vga.c b/hw/vga.c
> > index b80a079..7e90cf4 100644
> > --- a/hw/vga.c
> > +++ b/hw/vga.c
> > @@ -165,7 +165,7 @@ static uint16_t expand2[256];
> > static uint8_t expand4to8[16];
> >
> > static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> > - Error *errp);
> > + Error **errp);
> >
> > static void vga_update_memory_access(VGACommonState *s)
> > {
> > @@ -2365,7 +2365,7 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion *system_memory)
> > /********************************************************/
> > /* vga screen dump */
> >
> > -int ppm_save(const char *filename, struct DisplaySurface *ds)
> > +int ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
> > {
> > FILE *f;
> > uint8_t *d, *d1;
> > @@ -2377,8 +2377,10 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
> >
> > trace_ppm_save(filename, ds);
> > f = fopen(filename, "wb");
> > - if (!f)
> > + if (!f) {
> > + error_set(errp, QERR_OPEN_FILE_FAILED, filename);
>
> This is a generic error, and it's unfortunate that we're using it in several
> places now. The best thing to do here - and I'm including write() in this
> discussion - is to report the most possible errors, like EACCESS, ENOSPC,
> EPERM, EIO. Maybe not all of them exist as QErrors.
OK, I'll add new QErrors then.
>
> Also, note that some device models have their own way of saving the screen
> dump (eg. g364fb_screen_dump()) and should be converted to error_set()
> separately.
Makes sense.
>
> > return -1;
> > + }
> > fprintf(f, "P6\n%d %d\n%d\n",
> > ds->width, ds->height, 255);
> > linebuf = g_malloc(ds->width * 3);
> > @@ -2420,5 +2422,5 @@ static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> > vga_invalidate_display(s);
> > vga_hw_update();
> > }
> > - ppm_save(filename, s->ds->surface);
> > + ppm_save(filename, s->ds->surface, errp);
> > }
> > diff --git a/hw/vga_int.h b/hw/vga_int.h
> > index 7685b2b..63078ba 100644
> > --- a/hw/vga_int.h
> > +++ b/hw/vga_int.h
> > @@ -24,6 +24,7 @@
> >
> > #include <hw/hw.h>
> > #include "memory.h"
> > +#include "error.h"
> >
> > #define ST01_V_RETRACE 0x08
> > #define ST01_DISP_ENABLE 0x01
> > @@ -200,7 +201,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val);
> > uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr);
> > void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val);
> > void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
> > -int ppm_save(const char *filename, struct DisplaySurface *ds);
> > +int ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp);
> >
> > int vga_ioport_invalid(VGACommonState *s, uint32_t addr);
> > void vga_init_vbe(VGACommonState *s, MemoryRegion *address_space);
> > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> > index 6868778..0769652 100644
> > --- a/hw/vmware_vga.c
> > +++ b/hw/vmware_vga.c
> > @@ -1016,7 +1016,7 @@ static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch,
> > if (s->depth == 32) {
> > DisplaySurface *ds = qemu_create_displaysurface_from(s->width,
> > s->height, 32, ds_get_linesize(s->vga.ds), s->vga.vram_ptr);
> > - ppm_save(filename, ds);
> > + ppm_save(filename, ds, errp);
> > g_free(ds);
> > }
> > }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qapi: Convert screendump
2012-03-14 15:55 [Qemu-devel] [PATCH 1/2] qapi: Convert screendump Alon Levy
2012-03-14 15:55 ` [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure Alon Levy
@ 2012-03-14 17:33 ` Luiz Capitulino
2012-03-14 18:03 ` Alon Levy
1 sibling, 1 reply; 6+ messages in thread
From: Luiz Capitulino @ 2012-03-14 17:33 UTC (permalink / raw)
To: Alon Levy; +Cc: mlureau, qemu-devel
On Wed, 14 Mar 2012 17:55:32 +0200
Alon Levy <alevy@redhat.com> wrote:
> From: Luiz Capitulino <lcapitulino@redhat.com>
>
> Also, makes it possible for devices to report errors in the
> hw_screen_dump() callback.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Please, split this into two patches. The first one just adds the Error parameter
to the hw_screen_dump() callback, the second does the actual conversion.
And please, add your signed-off-by so that I don't feel I'm talking to myself :)
and an intro email is also a good thing (you'll need a changelog).
> ---
> console.c | 5 +++--
> console.h | 6 ++++--
> hmp-commands.hx | 3 +--
> hmp.c | 8 ++++++++
> hmp.h | 1 +
> hw/blizzard.c | 3 ++-
> hw/g364fb.c | 4 +++-
> hw/omap_lcdc.c | 4 +++-
> hw/qxl.c | 6 ++++--
> hw/tcx.c | 13 +++++++++----
> hw/vga.c | 7 +++++--
> hw/vmware_vga.c | 6 ++++--
> monitor.c | 6 ------
> qapi-schema.json | 13 +++++++++++++
> qmp-commands.hx | 5 +----
> qmp.c | 5 +++++
> 16 files changed, 66 insertions(+), 29 deletions(-)
>
> diff --git a/console.c b/console.c
> index 6a463f5..d3fccf3 100644
> --- a/console.c
> +++ b/console.c
> @@ -24,6 +24,7 @@
> #include "qemu-common.h"
> #include "console.h"
> #include "qemu-timer.h"
> +#include "error.h"
>
> //#define DEBUG_CONSOLE
> #define DEFAULT_BACKSCROLL 512
> @@ -173,7 +174,7 @@ void vga_hw_invalidate(void)
> active_console->hw_invalidate(active_console->hw);
> }
>
> -void vga_hw_screen_dump(const char *filename)
> +void vga_hw_screen_dump(const char *filename, Error **errp)
> {
> TextConsole *previous_active_console;
> bool cswitch;
> @@ -187,7 +188,7 @@ 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, cswitch);
> + consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch, errp);
> } else {
> error_report("screen dump not implemented");
> }
> diff --git a/console.h b/console.h
> index 4334db5..caf13f5 100644
> --- a/console.h
> +++ b/console.h
> @@ -6,6 +6,7 @@
> #include "notify.h"
> #include "monitor.h"
> #include "trace.h"
> +#include "error.h"
>
> /* keyboard/mouse support */
>
> @@ -343,7 +344,8 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
>
> typedef void (*vga_hw_update_ptr)(void *);
> typedef void (*vga_hw_invalidate_ptr)(void *);
> -typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch);
> +typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch,
> + Error **errp);
> typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *);
>
> DisplayState *graphic_console_init(vga_hw_update_ptr update,
> @@ -354,7 +356,7 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update,
>
> void vga_hw_update(void);
> void vga_hw_invalidate(void);
> -void vga_hw_screen_dump(const char *filename);
> +void vga_hw_screen_dump(const char *filename, Error **errp);
> void vga_hw_text_update(console_ch_t *chardata);
>
> int is_graphic_console(void);
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 6980214..d26421a 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -194,8 +194,7 @@ ETEXI
> .args_type = "filename:F",
> .params = "filename",
> .help = "save screen into PPM image 'filename'",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_screen_dump,
> + .mhandler.cmd = hmp_screendump,
> },
>
> STEXI
> diff --git a/hmp.c b/hmp.c
> index 290c43d..42dc79a 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -860,3 +860,11 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
>
> hmp_handle_error(mon, &error);
> }
> +
> +void hmp_screendump(Monitor *mon, const QDict *qdict)
> +{
> + Error *err = NULL;
> +
> + qmp_screendump(qdict_get_str(qdict, "filename"), &err);
> + hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 5409464..25d123f 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -59,5 +59,6 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
> void hmp_block_stream(Monitor *mon, const QDict *qdict);
> void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
> void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
> +void hmp_screendump(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/hw/blizzard.c b/hw/blizzard.c
> index c7d844d..76df78c 100644
> --- a/hw/blizzard.c
> +++ b/hw/blizzard.c
> @@ -23,6 +23,7 @@
> #include "devices.h"
> #include "vga_int.h"
> #include "pixel_ops.h"
> +#include "error.h"
>
> typedef void (*blizzard_fn_t)(uint8_t *, const uint8_t *, unsigned int);
>
> @@ -933,7 +934,7 @@ static void blizzard_update_display(void *opaque)
> }
>
> static void blizzard_screen_dump(void *opaque, const char *filename,
> - bool cswitch)
> + bool cswitch, Error **errp)
> {
> BlizzardState *s = (BlizzardState *) opaque;
>
> diff --git a/hw/g364fb.c b/hw/g364fb.c
> index 3a0b68f..7774d05 100644
> --- a/hw/g364fb.c
> +++ b/hw/g364fb.c
> @@ -22,6 +22,7 @@
> #include "pixel_ops.h"
> #include "trace.h"
> #include "sysbus.h"
> +#include "error.h"
>
> typedef struct G364State {
> /* hardware */
> @@ -289,7 +290,8 @@ static void g364fb_reset(G364State *s)
> g364fb_invalidate_display(s);
> }
>
> -static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch)
> +static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error **errp)
> {
> G364State *s = opaque;
> int y, x;
> diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
> index f172093..aec7210 100644
> --- a/hw/omap_lcdc.c
> +++ b/hw/omap_lcdc.c
> @@ -20,6 +20,7 @@
> #include "console.h"
> #include "omap.h"
> #include "framebuffer.h"
> +#include "error.h"
>
> struct omap_lcd_panel_s {
> MemoryRegion *sysmem;
> @@ -264,7 +265,8 @@ static int ppm_save(const char *filename, uint8_t *data,
> return 0;
> }
>
> -static void omap_screen_dump(void *opaque, const char *filename, bool cswitch)
> +static void omap_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error **errp)
> {
> struct omap_lcd_panel_s *omap_lcd = opaque;
> if (cswitch) {
> diff --git a/hw/qxl.c b/hw/qxl.c
> index e17b0e3..27f27f5 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -23,6 +23,7 @@
> #include "qemu-queue.h"
> #include "monitor.h"
> #include "sysemu.h"
> +#include "error.h"
>
> #include "qxl.h"
>
> @@ -1492,7 +1493,8 @@ static void qxl_hw_invalidate(void *opaque)
> vga->invalidate(vga);
> }
>
> -static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
> +static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error **errp)
> {
> PCIQXLDevice *qxl = opaque;
> VGACommonState *vga = &qxl->vga;
> @@ -1504,7 +1506,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
> ppm_save(filename, qxl->ssd.ds->surface);
> break;
> case QXL_MODE_VGA:
> - vga->screen_dump(vga, filename, cswitch);
> + vga->screen_dump(vga, filename, cswitch, errp);
> break;
> default:
> break;
> diff --git a/hw/tcx.c b/hw/tcx.c
> index ac7dcb4..f342a5d 100644
> --- a/hw/tcx.c
> +++ b/hw/tcx.c
> @@ -26,6 +26,7 @@
> #include "pixel_ops.h"
> #include "sysbus.h"
> #include "qdev-addr.h"
> +#include "error.h"
>
> #define MAXX 1024
> #define MAXY 768
> @@ -56,8 +57,10 @@ typedef struct TCXState {
> uint8_t dac_index, dac_state;
> } TCXState;
>
> -static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch);
> -static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch);
> +static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error *errp);
> +static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error *errp);
>
> static void tcx_set_dirty(TCXState *s)
> {
> @@ -574,7 +577,8 @@ static int tcx_init1(SysBusDevice *dev)
> return 0;
> }
>
> -static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
> +static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error *errp)
> {
> TCXState *s = opaque;
> FILE *f;
> @@ -601,7 +605,8 @@ static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
> return;
> }
>
> -static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch)
> +static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error *errp)
> {
> TCXState *s = opaque;
> FILE *f;
> diff --git a/hw/vga.c b/hw/vga.c
> index 6dc98f6..b80a079 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -31,6 +31,7 @@
> #include "qemu-timer.h"
> #include "xen.h"
> #include "trace.h"
> +#include "error.h"
>
> //#define DEBUG_VGA
> //#define DEBUG_VGA_MEM
> @@ -163,7 +164,8 @@ static uint32_t expand4[256];
> static uint16_t expand2[256];
> static uint8_t expand4to8[16];
>
> -static void vga_screen_dump(void *opaque, const char *filename, bool cswitch);
> +static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error *errp);
>
> static void vga_update_memory_access(VGACommonState *s)
> {
> @@ -2409,7 +2411,8 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
>
> /* save the vga display in a PPM image even if no display is
> available */
> -static void vga_screen_dump(void *opaque, const char *filename, bool cswitch)
> +static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error **errp)
> {
> VGACommonState *s = opaque;
>
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index 142d9f4..6868778 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -26,6 +26,7 @@
> #include "console.h"
> #include "pci.h"
> #include "vmware_vga.h"
> +#include "error.h"
>
> #undef VERBOSE
> #define HW_RECT_ACCEL
> @@ -1003,11 +1004,12 @@ static void vmsvga_invalidate_display(void *opaque)
>
> /* save the vga display in a PPM image even if no display is
> available */
> -static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch)
> +static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch,
> + Error **errp)
> {
> struct vmsvga_state_s *s = opaque;
> if (!s->enable) {
> - s->vga.screen_dump(&s->vga, filename, cswitch);
> + s->vga.screen_dump(&s->vga, filename, cswitch, errp);
> return;
> }
>
> diff --git a/monitor.c b/monitor.c
> index cbdfbad..f79ce9a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -893,12 +893,6 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
> return -1;
> }
>
> -static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data)
> -{
> - vga_hw_screen_dump(qdict_get_str(qdict, "filename"));
> - return 0;
> -}
> -
> static void do_logfile(Monitor *mon, const QDict *qdict)
> {
> cpu_set_log_filename(qdict_get_str(qdict, "filename"));
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 04fa84f..4f251ca 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1663,3 +1663,16 @@
> { 'command': 'qom-list-types',
> 'data': { '*implements': 'str', '*abstract': 'bool' },
> 'returns': [ 'ObjectTypeInfo' ] }
> +
> +##
> +# @screendump:
> +#
> +# Write a PPM of the VGA screen to a file.
> +#
> +# @filename: the name of a new PPM file to create to store the image
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.1
> +##
> +{ 'command': 'screendump', 'data': {'filename': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index dfe8a5b..5fe57fd 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -146,10 +146,7 @@ EQMP
> {
> .name = "screendump",
> .args_type = "filename:F",
> - .params = "filename",
> - .help = "save screen into PPM image 'filename'",
> - .user_print = monitor_user_noop,
> - .mhandler.cmd_new = do_screen_dump,
> + .mhandler.cmd_new = qmp_marshal_input_screendump,
> },
>
> SQMP
> diff --git a/qmp.c b/qmp.c
> index a182b51..086cec8 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -415,3 +415,8 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
>
> return ret;
> }
> +
> +void qmp_screendump(const char *filename, Error **errp)
> +{
> + vga_hw_screen_dump(filename, errp);
> +}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] qapi: Convert screendump
2012-03-14 17:33 ` [Qemu-devel] [PATCH 1/2] qapi: Convert screendump Luiz Capitulino
@ 2012-03-14 18:03 ` Alon Levy
0 siblings, 0 replies; 6+ messages in thread
From: Alon Levy @ 2012-03-14 18:03 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mlureau, qemu-devel
On Wed, Mar 14, 2012 at 02:33:53PM -0300, Luiz Capitulino wrote:
> On Wed, 14 Mar 2012 17:55:32 +0200
> Alon Levy <alevy@redhat.com> wrote:
>
> > From: Luiz Capitulino <lcapitulino@redhat.com>
> >
> > Also, makes it possible for devices to report errors in the
> > hw_screen_dump() callback.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>
> Please, split this into two patches. The first one just adds the Error parameter
> to the hw_screen_dump() callback, the second does the actual conversion.
OK.
>
> And please, add your signed-off-by so that I don't feel I'm talking to myself :)
> and an intro email is also a good thing (you'll need a changelog).
I assumed I could save myself one cover letter from v1 :)
>
> > ---
> > console.c | 5 +++--
> > console.h | 6 ++++--
> > hmp-commands.hx | 3 +--
> > hmp.c | 8 ++++++++
> > hmp.h | 1 +
> > hw/blizzard.c | 3 ++-
> > hw/g364fb.c | 4 +++-
> > hw/omap_lcdc.c | 4 +++-
> > hw/qxl.c | 6 ++++--
> > hw/tcx.c | 13 +++++++++----
> > hw/vga.c | 7 +++++--
> > hw/vmware_vga.c | 6 ++++--
> > monitor.c | 6 ------
> > qapi-schema.json | 13 +++++++++++++
> > qmp-commands.hx | 5 +----
> > qmp.c | 5 +++++
> > 16 files changed, 66 insertions(+), 29 deletions(-)
> >
> > diff --git a/console.c b/console.c
> > index 6a463f5..d3fccf3 100644
> > --- a/console.c
> > +++ b/console.c
> > @@ -24,6 +24,7 @@
> > #include "qemu-common.h"
> > #include "console.h"
> > #include "qemu-timer.h"
> > +#include "error.h"
> >
> > //#define DEBUG_CONSOLE
> > #define DEFAULT_BACKSCROLL 512
> > @@ -173,7 +174,7 @@ void vga_hw_invalidate(void)
> > active_console->hw_invalidate(active_console->hw);
> > }
> >
> > -void vga_hw_screen_dump(const char *filename)
> > +void vga_hw_screen_dump(const char *filename, Error **errp)
> > {
> > TextConsole *previous_active_console;
> > bool cswitch;
> > @@ -187,7 +188,7 @@ 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, cswitch);
> > + consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch, errp);
> > } else {
> > error_report("screen dump not implemented");
> > }
> > diff --git a/console.h b/console.h
> > index 4334db5..caf13f5 100644
> > --- a/console.h
> > +++ b/console.h
> > @@ -6,6 +6,7 @@
> > #include "notify.h"
> > #include "monitor.h"
> > #include "trace.h"
> > +#include "error.h"
> >
> > /* keyboard/mouse support */
> >
> > @@ -343,7 +344,8 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
> >
> > typedef void (*vga_hw_update_ptr)(void *);
> > typedef void (*vga_hw_invalidate_ptr)(void *);
> > -typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch);
> > +typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch,
> > + Error **errp);
> > typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *);
> >
> > DisplayState *graphic_console_init(vga_hw_update_ptr update,
> > @@ -354,7 +356,7 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update,
> >
> > void vga_hw_update(void);
> > void vga_hw_invalidate(void);
> > -void vga_hw_screen_dump(const char *filename);
> > +void vga_hw_screen_dump(const char *filename, Error **errp);
> > void vga_hw_text_update(console_ch_t *chardata);
> >
> > int is_graphic_console(void);
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 6980214..d26421a 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -194,8 +194,7 @@ ETEXI
> > .args_type = "filename:F",
> > .params = "filename",
> > .help = "save screen into PPM image 'filename'",
> > - .user_print = monitor_user_noop,
> > - .mhandler.cmd_new = do_screen_dump,
> > + .mhandler.cmd = hmp_screendump,
> > },
> >
> > STEXI
> > diff --git a/hmp.c b/hmp.c
> > index 290c43d..42dc79a 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -860,3 +860,11 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
> >
> > hmp_handle_error(mon, &error);
> > }
> > +
> > +void hmp_screendump(Monitor *mon, const QDict *qdict)
> > +{
> > + Error *err = NULL;
> > +
> > + qmp_screendump(qdict_get_str(qdict, "filename"), &err);
> > + hmp_handle_error(mon, &err);
> > +}
> > diff --git a/hmp.h b/hmp.h
> > index 5409464..25d123f 100644
> > --- a/hmp.h
> > +++ b/hmp.h
> > @@ -59,5 +59,6 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
> > void hmp_block_stream(Monitor *mon, const QDict *qdict);
> > void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
> > void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
> > +void hmp_screendump(Monitor *mon, const QDict *qdict);
> >
> > #endif
> > diff --git a/hw/blizzard.c b/hw/blizzard.c
> > index c7d844d..76df78c 100644
> > --- a/hw/blizzard.c
> > +++ b/hw/blizzard.c
> > @@ -23,6 +23,7 @@
> > #include "devices.h"
> > #include "vga_int.h"
> > #include "pixel_ops.h"
> > +#include "error.h"
> >
> > typedef void (*blizzard_fn_t)(uint8_t *, const uint8_t *, unsigned int);
> >
> > @@ -933,7 +934,7 @@ static void blizzard_update_display(void *opaque)
> > }
> >
> > static void blizzard_screen_dump(void *opaque, const char *filename,
> > - bool cswitch)
> > + bool cswitch, Error **errp)
> > {
> > BlizzardState *s = (BlizzardState *) opaque;
> >
> > diff --git a/hw/g364fb.c b/hw/g364fb.c
> > index 3a0b68f..7774d05 100644
> > --- a/hw/g364fb.c
> > +++ b/hw/g364fb.c
> > @@ -22,6 +22,7 @@
> > #include "pixel_ops.h"
> > #include "trace.h"
> > #include "sysbus.h"
> > +#include "error.h"
> >
> > typedef struct G364State {
> > /* hardware */
> > @@ -289,7 +290,8 @@ static void g364fb_reset(G364State *s)
> > g364fb_invalidate_display(s);
> > }
> >
> > -static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch)
> > +static void g364fb_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error **errp)
> > {
> > G364State *s = opaque;
> > int y, x;
> > diff --git a/hw/omap_lcdc.c b/hw/omap_lcdc.c
> > index f172093..aec7210 100644
> > --- a/hw/omap_lcdc.c
> > +++ b/hw/omap_lcdc.c
> > @@ -20,6 +20,7 @@
> > #include "console.h"
> > #include "omap.h"
> > #include "framebuffer.h"
> > +#include "error.h"
> >
> > struct omap_lcd_panel_s {
> > MemoryRegion *sysmem;
> > @@ -264,7 +265,8 @@ static int ppm_save(const char *filename, uint8_t *data,
> > return 0;
> > }
> >
> > -static void omap_screen_dump(void *opaque, const char *filename, bool cswitch)
> > +static void omap_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error **errp)
> > {
> > struct omap_lcd_panel_s *omap_lcd = opaque;
> > if (cswitch) {
> > diff --git a/hw/qxl.c b/hw/qxl.c
> > index e17b0e3..27f27f5 100644
> > --- a/hw/qxl.c
> > +++ b/hw/qxl.c
> > @@ -23,6 +23,7 @@
> > #include "qemu-queue.h"
> > #include "monitor.h"
> > #include "sysemu.h"
> > +#include "error.h"
> >
> > #include "qxl.h"
> >
> > @@ -1492,7 +1493,8 @@ static void qxl_hw_invalidate(void *opaque)
> > vga->invalidate(vga);
> > }
> >
> > -static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
> > +static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error **errp)
> > {
> > PCIQXLDevice *qxl = opaque;
> > VGACommonState *vga = &qxl->vga;
> > @@ -1504,7 +1506,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, bool cswitch)
> > ppm_save(filename, qxl->ssd.ds->surface);
> > break;
> > case QXL_MODE_VGA:
> > - vga->screen_dump(vga, filename, cswitch);
> > + vga->screen_dump(vga, filename, cswitch, errp);
> > break;
> > default:
> > break;
> > diff --git a/hw/tcx.c b/hw/tcx.c
> > index ac7dcb4..f342a5d 100644
> > --- a/hw/tcx.c
> > +++ b/hw/tcx.c
> > @@ -26,6 +26,7 @@
> > #include "pixel_ops.h"
> > #include "sysbus.h"
> > #include "qdev-addr.h"
> > +#include "error.h"
> >
> > #define MAXX 1024
> > #define MAXY 768
> > @@ -56,8 +57,10 @@ typedef struct TCXState {
> > uint8_t dac_index, dac_state;
> > } TCXState;
> >
> > -static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch);
> > -static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch);
> > +static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error *errp);
> > +static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error *errp);
> >
> > static void tcx_set_dirty(TCXState *s)
> > {
> > @@ -574,7 +577,8 @@ static int tcx_init1(SysBusDevice *dev)
> > return 0;
> > }
> >
> > -static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
> > +static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error *errp)
> > {
> > TCXState *s = opaque;
> > FILE *f;
> > @@ -601,7 +605,8 @@ static void tcx_screen_dump(void *opaque, const char *filename, bool cswitch)
> > return;
> > }
> >
> > -static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch)
> > +static void tcx24_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error *errp)
> > {
> > TCXState *s = opaque;
> > FILE *f;
> > diff --git a/hw/vga.c b/hw/vga.c
> > index 6dc98f6..b80a079 100644
> > --- a/hw/vga.c
> > +++ b/hw/vga.c
> > @@ -31,6 +31,7 @@
> > #include "qemu-timer.h"
> > #include "xen.h"
> > #include "trace.h"
> > +#include "error.h"
> >
> > //#define DEBUG_VGA
> > //#define DEBUG_VGA_MEM
> > @@ -163,7 +164,8 @@ static uint32_t expand4[256];
> > static uint16_t expand2[256];
> > static uint8_t expand4to8[16];
> >
> > -static void vga_screen_dump(void *opaque, const char *filename, bool cswitch);
> > +static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error *errp);
> >
> > static void vga_update_memory_access(VGACommonState *s)
> > {
> > @@ -2409,7 +2411,8 @@ int ppm_save(const char *filename, struct DisplaySurface *ds)
> >
> > /* save the vga display in a PPM image even if no display is
> > available */
> > -static void vga_screen_dump(void *opaque, const char *filename, bool cswitch)
> > +static void vga_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error **errp)
> > {
> > VGACommonState *s = opaque;
> >
> > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> > index 142d9f4..6868778 100644
> > --- a/hw/vmware_vga.c
> > +++ b/hw/vmware_vga.c
> > @@ -26,6 +26,7 @@
> > #include "console.h"
> > #include "pci.h"
> > #include "vmware_vga.h"
> > +#include "error.h"
> >
> > #undef VERBOSE
> > #define HW_RECT_ACCEL
> > @@ -1003,11 +1004,12 @@ static void vmsvga_invalidate_display(void *opaque)
> >
> > /* save the vga display in a PPM image even if no display is
> > available */
> > -static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch)
> > +static void vmsvga_screen_dump(void *opaque, const char *filename, bool cswitch,
> > + Error **errp)
> > {
> > struct vmsvga_state_s *s = opaque;
> > if (!s->enable) {
> > - s->vga.screen_dump(&s->vga, filename, cswitch);
> > + s->vga.screen_dump(&s->vga, filename, cswitch, errp);
> > return;
> > }
> >
> > diff --git a/monitor.c b/monitor.c
> > index cbdfbad..f79ce9a 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -893,12 +893,6 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
> > return -1;
> > }
> >
> > -static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data)
> > -{
> > - vga_hw_screen_dump(qdict_get_str(qdict, "filename"));
> > - return 0;
> > -}
> > -
> > static void do_logfile(Monitor *mon, const QDict *qdict)
> > {
> > cpu_set_log_filename(qdict_get_str(qdict, "filename"));
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 04fa84f..4f251ca 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1663,3 +1663,16 @@
> > { 'command': 'qom-list-types',
> > 'data': { '*implements': 'str', '*abstract': 'bool' },
> > 'returns': [ 'ObjectTypeInfo' ] }
> > +
> > +##
> > +# @screendump:
> > +#
> > +# Write a PPM of the VGA screen to a file.
> > +#
> > +# @filename: the name of a new PPM file to create to store the image
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 1.1
> > +##
> > +{ 'command': 'screendump', 'data': {'filename': 'str'} }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index dfe8a5b..5fe57fd 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -146,10 +146,7 @@ EQMP
> > {
> > .name = "screendump",
> > .args_type = "filename:F",
> > - .params = "filename",
> > - .help = "save screen into PPM image 'filename'",
> > - .user_print = monitor_user_noop,
> > - .mhandler.cmd_new = do_screen_dump,
> > + .mhandler.cmd_new = qmp_marshal_input_screendump,
> > },
> >
> > SQMP
> > diff --git a/qmp.c b/qmp.c
> > index a182b51..086cec8 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -415,3 +415,8 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
> >
> > return ret;
> > }
> > +
> > +void qmp_screendump(const char *filename, Error **errp)
> > +{
> > + vga_hw_screen_dump(filename, errp);
> > +}
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-14 18:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-14 15:55 [Qemu-devel] [PATCH 1/2] qapi: Convert screendump Alon Levy
2012-03-14 15:55 ` [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure Alon Levy
2012-03-14 17:40 ` Luiz Capitulino
2012-03-14 18:02 ` Alon Levy
2012-03-14 17:33 ` [Qemu-devel] [PATCH 1/2] qapi: Convert screendump Luiz Capitulino
2012-03-14 18:03 ` Alon Levy
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).