From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:46033)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1QeMst-0000DZ-Tc
	for qemu-devel@nongnu.org; Wed, 06 Jul 2011 03:51:00 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1QeMss-0002L7-98
	for qemu-devel@nongnu.org; Wed, 06 Jul 2011 03:50:59 -0400
Received: from mx1.redhat.com ([209.132.183.28]:1027)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1QeMsr-0002Ks-L0
	for qemu-devel@nongnu.org; Wed, 06 Jul 2011 03:50:58 -0400
Message-ID: <4E14147C.4070600@redhat.com>
Date: Wed, 06 Jul 2011 09:53:32 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <1305981319-32508-1-git-send-email-morita.kazutaka@lab.ntt.co.jp>	<4E0D8555.40102@redhat.com>
	<87iprgegh1.wl%morita.kazutaka@lab.ntt.co.jp>
In-Reply-To: <87iprgegh1.wl%morita.kazutaka@lab.ntt.co.jp>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] sheepdog: add data preallocation support
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: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Cc: sheepdog@lists.wpkg.org, qemu-devel@nongnu.org

Am 05.07.2011 20:21, schrieb MORITA Kazutaka:
>>> +
>>> +    max_idx = (vdi_size + SD_DATA_OBJ_SIZE - 1) / SD_DATA_OBJ_SIZE;
>>> +
>>> +    for (idx = 0; idx < max_idx; idx++) {
>>> +        uint64_t oid;
>>> +        oid = vid_to_data_oid(vid, idx);
>>> +
>>> +        if (inode->data_vdi_id[idx]) {
>>> +            ret = read_object(fd, buf, vid_to_vdi_oid(inode->data_vdi_id[idx]),
>>> +                              1, SD_DATA_OBJ_SIZE, 0);
>>> +            if (ret)
>>> +                goto out;
>>
>> Missing braces.
>>
>> Also, what is this if branch doing? Is it to ensure that we don't
>> overwrite existing data? But then, isn't an image always empty when we
>> preallocate it?
> 
> This branch is for handling a cloned image, which is created with -b
> option.  This branch reads data from the backing file (read_object
> returns zero when it succeeds) instead of filling buffer with zero.

Oh, I see. You support preallocation even with backing files. And
suddenly it makes perfect sense. :-)

(Although after completing preallocation, you won't need the backing
file any more as all of it has been copied into the image. Maybe we
should drop the reference then?)

>> In my RFC for qcow2, I called this preallocation mode "full" instead of
>> "data". I don't mind much which we pick, but we should keep it
>> consistent. Any preferences?
> 
> I don't mind too.  I'll use "full" in the v2 patch.

Great, thanks.

Kevin