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

[Kevin Wolf <kwolf@redhat.com>]

Am 21.11.2011 11:53, schrieb Stefan Hajnoczi:
> 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 didn't say this. :-)

At least in theory, block devices may not be the only protocols with
!has_zero_init. We have only covered raw-posix with this discussion. I
would prefer an explicit write of the table to avoid breaking other
protocols (though I don't think we have one today; curl would be a
candidate, but it is read-only).

Kevin