From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:60738)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1RuRjJ-0004VF-4m
	for qemu-devel@nongnu.org; Mon, 06 Feb 2012 11:47:50 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1RuRjD-00060C-Nw
	for qemu-devel@nongnu.org; Mon, 06 Feb 2012 11:47:49 -0500
Received: from mx1.redhat.com ([209.132.183.28]:33456)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1RuRjD-0005zy-Dy
	for qemu-devel@nongnu.org; Mon, 06 Feb 2012 11:47:43 -0500
Message-ID: <4F3004FC.7070802@redhat.com>
Date: Mon, 06 Feb 2012 17:51:08 +0100
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <4F27D8A00200009100079C3A@novprvoes0310.provo.novell.com>
	<4F286659.6060300@suse.de>
	<4F2811220200009100079C4A@novprvoes0310.provo.novell.com>
	<4F292CD0.20707@redhat.com>
	<4F290B1A0200009100079CC1@novprvoes0310.provo.novell.com>
	<4F2A5021.4090505@redhat.com>
	<4F2AC4D30200009100079D96@novprvoes0310.provo.novell.com>
	<4F2FF5B9.9090404@redhat.com>
	<4F2F9BD60200009100079F2D@novprvoes0310.provo.novell.com>
In-Reply-To: <4F2F9BD60200009100079F2D@novprvoes0310.provo.novell.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] block: Add support for vpc Fixed Disk type
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Charles Arnold <carnold@suse.com>
Cc: qemu-devel@nongnu.org, afaerber@suse.de

Am 06.02.2012 17:22, schrieb Charles Arnold:
>>>> On 2/6/2012 at 08:46 AM, in message <4F2FF5B9.9090404@redhat.com>, Kevin Wolf
> <kwolf@redhat.com> wrote: 
>>
>> Somehow you lost the ret = -EFBIG here.
>>
>> Otherwise the patch looks good enough for me.
>>
>> Kevin
> 
> Thanks Kevin.  Here is the revised patch with just this fix.
> - Charles

Thanks, applied to the block branch.

I have one question left, though:

> +    total_sectors = total_size / BDRV_SECTOR_SIZE;
> +    if (disk_type == VHD_DYNAMIC) {
> +        /* Calculate matching total_size and geometry. Increase the number of
> +           sectors requested until we get enough (or fail). */
> +        for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl;
> +             i++) {
> +            if (calculate_geometry(total_sectors + i,
> +                                   &cyls, &heads, &secs_per_cyl)) {
> +                ret = -EFBIG;
> +                goto fail;
> +            }
> +        }
> +    } else {
> +        if (calculate_geometry(total_sectors, &cyls, &heads, &secs_per_cyl)) {
> +            ret = -EFBIG;
> +            goto fail;
> +        }
> +    }

What's the reason that we need to do things differently here depending
on the subformat? Dynamic disks round up the size so that during image
conversion images won't be truncated. For fixed images,
calculate_geometry can round down, so don't fixed image have the same
problem?

Kevin