Re: [Qemu-devel] [PATCH] sheepdog: add data preallocation support

[Kevin Wolf <kwolf@redhat.com>]

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