Re: [Qemu-devel] [PATCH] block: Use bdrv functions to replace file operation in qcow.c

[Stefan Hajnoczi <stefanha@gmail.com>]

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> wrote:
>>> On 11/18/2011 11:59 AM, Stefan Hajnoczi wrote:
>>>>
>>>> +    tmp = g_malloc0(sizeof(uint64_t)*l1_size);
>>>>>  +    ret = bdrv_pwrite(qcow_bs, header_size, tmp,
>>>>> sizeof(uint64_t)*l1_size);
>>>>>  +    g_free(tmp);
>>>>>  +    if (ret != sizeof(uint64_t)*l1_size) {
>>>>>  +        goto exit;
>>>>>       }
>>>>
>>>> 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?  Since the file 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.  I 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