From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:57365)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1RSRVD-0003uK-LL
	for qemu-devel@nongnu.org; Mon, 21 Nov 2011 05:53:32 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1RSRVB-0002EK-FS
	for qemu-devel@nongnu.org; Mon, 21 Nov 2011 05:53:31 -0500
Received: from mail-ww0-f53.google.com ([74.125.82.53]:62057)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1RSRVB-0002E8-4b
	for qemu-devel@nongnu.org; Mon, 21 Nov 2011 05:53:29 -0500
Received: by wwf27 with SMTP id 27so8988510wwf.10
	for <qemu-devel@nongnu.org>; Mon, 21 Nov 2011 02:53:28 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <4ECA2B78.7040400@redhat.com>
References: <1321607573-29744-1-git-send-email-zhihuili@linux.vnet.ibm.com>
	<CAJSP0QXfetSM8zewDtrYkL9F2790UXtcS1Bh761nL5UQxE8twQ@mail.gmail.com>
	<ja5eff$tdd$1@dough.gmane.org>
	<CAJSP0QUghLT5-bRFtwL2Yj0VwRp0PG3rV0hmhUB5D7jHn+3HWQ@mail.gmail.com>
	<4ECA2B78.7040400@redhat.com>
Date: Mon, 21 Nov 2011 10:53:28 +0000
Message-ID: <CAJSP0QUnmGv8sxJhzHV_5sWVird2+OFFvDdMDx0ViaZoCcTc2g@mail.gmail.com>
From: Stefan Hajnoczi <stefanha@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH] block: Use bdrv functions to replace file
 operation in qcow.c
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: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org

On Mon, Nov 21, 2011 at 10:44 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 18.11.2011 15:34, schrieb Stefan Hajnoczi:
>> On Fri, Nov 18, 2011 at 11:10 AM, Paolo Bonzini <pbonzini@redhat.com> wr=
ote:
>>> On 11/18/2011 11:59 AM, Stefan Hajnoczi wrote:
>>>>
>>>> + =A0 =A0tmp =3D g_malloc0(sizeof(uint64_t)*l1_size);
>>>>> =A0+ =A0 =A0ret =3D bdrv_pwrite(qcow_bs, header_size, tmp,
>>>>> sizeof(uint64_t)*l1_size);
>>>>> =A0+ =A0 =A0g_free(tmp);
>>>>> =A0+ =A0 =A0if (ret !=3D sizeof(uint64_t)*l1_size) {
>>>>> =A0+ =A0 =A0 =A0 =A0goto exit;
>>>>> =A0 =A0 =A0 }
>>>>
>>>> That means 400 MB of RAM for the zero L1 table for a 100 TB image.
>>>> Since qcow is a legacy format this probably doesn't matter in practice
>>>> but in theory this approach can require a noticable amount of RAM.
>>>
>>> 4 MB / TB is not a big deal (you probably would like the L1 table to be=
 in
>>> memory all the time), but why write the L1 table at all? =A0Since the f=
ile was
>>> CREATed, it is already zero and you can just leave a hole in the file.
>>
>> I thought the same thing then remember sometimes people want to use
>> image formats on block devices. =A0I think at least making image
>> creation not depend on has_zero_init is a good idea.
>
> qcow1 doesn't work on block devices anyway.

Okay, both of my original points were moot, Kevin and Paolo have explained =
why:

The L1 RAM size issue doesn't really matter since we hold the entire
L1 in RAM during normal operation anyway.  Holding it in RAM during
creation is no worse.

The zero initialization could be optimized as Paolo suggested with
truncate since qcow1 always works on image files (which have automatic
zero initialization).

I'm happy with this patch.

Stefan