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. Berto