From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1NeVP0-0000d9-Vt
	for qemu-devel@nongnu.org; Mon, 08 Feb 2010 10:19:55 -0500
Received: from [199.232.76.173] (port=58273 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1NeVP0-0000cI-Dk
	for qemu-devel@nongnu.org; Mon, 08 Feb 2010 10:19:54 -0500
Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim
	4.60) (envelope-from <kwolf@redhat.com>) id 1NeVOx-0002KI-VX
	for qemu-devel@nongnu.org; Mon, 08 Feb 2010 10:19:53 -0500
Received: from mx1.redhat.com ([209.132.183.28]:19865)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kwolf@redhat.com>) id 1NeVOw-0002JG-89
	for qemu-devel@nongnu.org; Mon, 08 Feb 2010 10:19:50 -0500
Message-ID: <4B702B60.4000904@redhat.com>
Date: Mon, 08 Feb 2010 16:18:56 +0100
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <87hbprsrei.fsf@meyering.net>
In-Reply-To: <87hbprsrei.fsf@meyering.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH] qcow2: don't ignore failed update_refcount
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jim Meyering <jim@meyering.net>
Cc: qemu-devel@nongnu.org

Am 08.02.2010 16:01, schrieb Jim Meyering:
> update_refcount is marked as a function for which we must use its result,
> 
>     static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
> 
> and rightly so, since doing otherwise would amount to ignoring write failure.
> However, there are two cases in which the return value is currently ignored.
> This fixes them:
> 
> From 107940556a2d0ef1de1d59a5da0c6c3086246817 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@redhat.com>
> Date: Mon, 8 Feb 2010 11:50:59 +0100
> Subject: [PATCH] qcow2: don't ignore failed update_refcount
> 
> * block/qcow2-refcount.c (grow_refcount_table): When update_refcount
> fails, return its negative return code to our caller.
> (alloc_refcount_block): Likewise.

I'm currently working on fixing exactly this, and unfortunaly, no, it's
not that easy. What you introduce looks like proper error handling at
first sight, but what happens in fact is that while the current write
request correctly fails now we're running with corrupted metadata for
all future requests (the new refcount table/block is already in use, but
it has a refcount of 0 and will be overwritten sooner or later).

Actually, I have found it impossible to fix the current approach, so the
fix I'm working on will be more of a rewrite of the two functions.

Kevin