On Thu 09 Apr 2020 09:50:52 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> case QCOW2_CLUSTER_ZERO_PLAIN: >> case QCOW2_CLUSTER_UNALLOCATED: >> /* how many empty clusters ? */ >> c = count_contiguous_clusters_unallocated(bs, nb_clusters, >> &l2_slice[l2_index], >> type); >> - *cluster_offset = 0; >> + *host_offset = 0; > > Actually, dead assignment now.. But I feel that better to keep it. > > Hmm. May be, drop the first assignment of zero to host_offset? We > actually don't need it, user should not rely on host_offset if we > return an error.
Yeah, I'll drop the first one and keep this one. >> @@ -3735,7 +3726,7 @@ static coroutine_fn int >> qcow2_co_pwrite_zeroes(BlockDriverState *bs, >> offset = QEMU_ALIGN_DOWN(offset, s->cluster_size); >> bytes = s->cluster_size; > > Unrelated to the patch, but.. Why we change bytes?? So, we can finish > with success, but zero-out only first cluster? > > Ah, found, generic block-layer take care of it and never issue > unaligned requests crossing cluster boundary. That's right, hence the assert(head + bytes <= s->cluster_size); a few lines before. Berto