From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:42301) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTuQ4-0006qw-AP for qemu-devel@nongnu.org; Fri, 25 Nov 2011 06:58:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RTuQ3-0003dt-73 for qemu-devel@nongnu.org; Fri, 25 Nov 2011 06:58:16 -0500 Received: from cantor2.suse.de ([195.135.220.15]:47921 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTuQ2-0003do-Qw for qemu-devel@nongnu.org; Fri, 25 Nov 2011 06:58:15 -0500 Message-ID: <4ECF82B3.5090705@suse.de> Date: Fri, 25 Nov 2011 12:57:39 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1322044805-3687-1-git-send-email-kwolf@redhat.com> <4ECCD7B5.6010607@redhat.com> In-Reply-To: <4ECCD7B5.6010607@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] vpc: Add missing error handling in alloc_block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Stefan Hajnoczi , qemu-devel@nongnu.org Am 23.11.2011 12:23, schrieb Kevin Wolf: > Am 23.11.2011 12:01, schrieb Stefan Hajnoczi: >> On Wed, Nov 23, 2011 at 10:40 AM, Kevin Wolf wrote: >>> Signed-off-by: Kevin Wolf >>> --- >>> block/vpc.c | 5 ++++- >>> 1 files changed, 4 insertions(+), 1 deletions(-) >>> >>> diff --git a/block/vpc.c b/block/vpc.c >>> index 75d7d4a..89a5ee2 100644 >>> --- a/block/vpc.c >>> +++ b/block/vpc.c >>> @@ -362,8 +362,11 @@ static int64_t alloc_block(BlockDriverState* bs,= int64_t sector_num) >>> >>> // Initialize the block's bitmap >>> memset(bitmap, 0xff, s->bitmap_size); >>> - bdrv_pwrite_sync(bs->file, s->free_data_block_offset, bitmap, >>> + ret =3D bdrv_pwrite_sync(bs->file, s->free_data_block_offset, bi= tmap, >>> s->bitmap_size); >>> + if (ret < 0) { >>> + return ret; >>> + } >> >> I notice that s->pagetable[index] is left modified when the function >> fails. But this is a larger issue and could be addressed in a later >> patch which also looks at the other failure cases in this function. >=20 > Error handling in vpc needs some work anyway. For example, almost all > places return -1 instead of the real error number. Probably even the > order of operations is unsafe, didn't check that yet. As a note, as part of QEMU Test Day(s) I tested VHD images (cf. Wiki) - booting, installing software, etc. and haven't encountered any problem. Doesn't mean it's entirely safe though, obviously. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg