From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45900) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7sBq-0004H1-Qb for qemu-devel@nongnu.org; Wed, 14 Mar 2012 13:41:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7sBl-0008BC-GU for qemu-devel@nongnu.org; Wed, 14 Mar 2012 13:40:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26036) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7sBl-0008Ax-7n for qemu-devel@nongnu.org; Wed, 14 Mar 2012 13:40:41 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2EHedUl004094 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 14 Mar 2012 13:40:39 -0400 Date: Wed, 14 Mar 2012 14:40:42 -0300 From: Luiz Capitulino Message-ID: <20120314144042.313508d5@doriath.home> In-Reply-To: <1331740533-26319-2-git-send-email-alevy@redhat.com> References: <1331740533-26319-1-git-send-email-alevy@redhat.com> <1331740533-26319-2-git-send-email-alevy@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] vga: ppm_save(): Return error on failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alon Levy Cc: mlureau@redhat.com, qemu-devel@nongnu.org On Wed, 14 Mar 2012 17:55:33 +0200 Alon Levy wrote: > From: Luiz Capitulino > > 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 > --- > 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 > #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); > } > }