On Wed 30 Oct 2019 01:04:02 PM CET, Max Reitz wrote: > (I intended not to comment on such things on an RFC, but here I am...)
No problem with that :-) > I’d call it host_cluster_offset to make clear that it points to a > cluster and isn’t the host offset for @guest_offset. Sure, why not. We can also accept the exact offset within the cluster and then use start_of_cluster(), but I prefer this one. > And now that I’ve gone this far already, I might as well say that I’d > like if it the comment noted that this function not only creates the > L2Meta structure but also adds it to the cluster_allocs list. I'll update the comment. >> + * @guest_offset and @bytes indicate the offset and length of the >> + * request. >> + * >> + * If @keep_old is true it means that the clusters were already >> + * allocated and will be overwritten. If false then the clusters are >> + * new and we have to decrease the reference count of the old ones. >> + */ >> +static void calculate_l2_meta(BlockDriverState *bs, uint64_t host_offset, >> + uint64_t guest_offset, uint64_t bytes, > > And now I’m so deep into non-RFC-level review territory, I might as > well say I’d prefer @bytes to be an unsigned (or maybe even a plain > int), because anything more wouldn’t work. (Not least because > cow_end_to is an unsigned). True, I'll correct that too. Thanks! Berto
