[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v2 16/32] qcow2: Update l2_allocate

From: Alberto Garcia
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v2 16/32] qcow2: Update l2_allocate() to support L2 slices
Date: Wed, 17 Jan 2018 17:13:45 +0100
User-agent: Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu)

On Wed 17 Jan 2018 05:06:04 PM CET, Eric Blake wrote:
>>>>      /* allocate a new entry in the l2 cache */
>>>> +    slice_size = s->l2_slice_size * sizeof(uint64_t);
>>> Would this read any better if the earlier patch named it
>>> s->l2_slice_entries?
>> I had doubts with this. Like you, when I see size I tend to think about
>> bytes. However both s->l1_size and s->l2_size indicate entries, and the
>> documentation of the qcow2 format even describes the header field like
>> this:
>>          36 - 39:   l1_size
>>                     Number of entries in the active L1 table
> We're free to rename the field in the qcow2 format specification if it
> makes things easier to understand.  If l1_entries reads better than
> l1_size, maybe it's worth doing.

In my opinion it reads much better. We mix both kinds of meanings in the
BDRVQcow2State structure already, there's for example cluster_size
(bytes), and refcount_table_size (entries). With the latter we actually
have the exact same problem we're discussing here:

    refcount_table_size2 = s->refcount_table_size * sizeof(uint64_t);
    s->refcount_table = g_try_malloc(refcount_table_size2);

So yeah, we can change those variables but it won't be a small patch. I
can do that if everyone thinks that it's a good idea.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]