From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36123) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTAsR-0006Qu-5W for qemu-devel@nongnu.org; Wed, 23 Nov 2011 06:20:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RTAsN-0004df-8n for qemu-devel@nongnu.org; Wed, 23 Nov 2011 06:20:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39873) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RTAsN-0004dV-0Q for qemu-devel@nongnu.org; Wed, 23 Nov 2011 06:20:27 -0500 Message-ID: <4ECCD7B5.6010607@redhat.com> Date: Wed, 23 Nov 2011 12:23:33 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1322044805-3687-1-git-send-email-kwolf@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: Stefan Hajnoczi Cc: qemu-devel@nongnu.org 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 = bdrv_pwrite_sync(bs->file, s->free_data_block_offset, bitmap, >> 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. 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. Kevin