From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MLuOx-0005oU-RN
	for qemu-devel@nongnu.org; Wed, 01 Jul 2009 03:38:43 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MLuOs-0005nO-PX
	for qemu-devel@nongnu.org; Wed, 01 Jul 2009 03:38:43 -0400
Received: from [199.232.76.173] (port=49182 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MLuOs-0005nL-MS
	for qemu-devel@nongnu.org; Wed, 01 Jul 2009 03:38:38 -0400
Received: from mx2.redhat.com ([66.187.237.31]:35897)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kwolf@redhat.com>) id 1MLuOs-0004at-3S
	for qemu-devel@nongnu.org; Wed, 01 Jul 2009 03:38:38 -0400
Message-ID: <4A4B1238.8060302@redhat.com>
Date: Wed, 01 Jul 2009 09:37:28 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH] block-raw: Allow pread beyond the end of
	growable images
References: <1246038684-3702-1-git-send-email-kwolf@redhat.com>
	<20090630180940.GA4205@lst.de>
In-Reply-To: <20090630180940.GA4205@lst.de>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
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: Christoph Hellwig <hch@lst.de>
Cc: qemu-devel@nongnu.org

Christoph Hellwig schrieb:
> On Fri, Jun 26, 2009 at 07:51:24PM +0200, Kevin Wolf wrote:
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index fa1a394..985bf69 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -117,6 +117,7 @@ typedef struct BDRVRawState {
>>  static int posix_aio_init(void);
>>  
>>  static int fd_open(BlockDriverState *bs);
>> +static int64_t raw_getlength(BlockDriverState *bs);
>>  
>>  #if defined(__FreeBSD__)
>>  static int cdrom_reopen(BlockDriverState *bs);
>> @@ -231,6 +232,16 @@ static int raw_pread_aligned(BlockDriverState *bs, int64_t offset,
>>      if (ret == count)
>>          goto label__raw_read__success;
>>  
>> +    /* Allow reads beyond the end (needed for pwrite) */
>> +    if ((ret == 0) && bs->growable) {
>> +        int64_t size = raw_getlength(bs);
>> +        if (offset >= size) {
>> +            memset(buf, 0, count);
>> +            ret = count;
>> +            goto label__raw_read__success;
>> +        }
>> +    }
> 
> I really don't like doing this inside the lowelevel read handler.  If
> this is indeed only needed for pwrite we might be better off doing
> the right thing in the place that needs it, bdrv_pwrite.

If you feel like posting a better patch, go ahead. I'm feeling that it's
going to be a whole lot uglier in bdrv_pwrite, but that might prove
wrong. And after all it is a problem with raw and not with the generic
block code - qcow2 for example was already reading zeros.

For the time being, this patch fixes a bug and should stay.

Kevin