On 07/30/2014 10:14 AM, Eric Blake wrote: > On 07/25/2014 12:07 PM, Max Reitz wrote: >> Actually, we do not need to allocate a new data cluster for every zero >> cluster to be expanded: It is completely sufficient to rely on qcow2's >> COW part and instead create a single zero cluster and reuse it as much >> as possible. > Also, I have to wonder - since the all-zero cluster is the most likely > cluster to have a large refcount, even during normal runtime, should we > special case the normal qcow2 write code to track the current all-zero > cluster (if any), and merely increase its refcount rather than allocate > a new cluster any time it is detected that an all-zero cluster is > needed? [Of course, the tracking would be runtime only, since > compat=0.10 header doesn't provide any way to track the location of an > all-zero cluster across file reloads. Each new runtime would probably > settle on a new location for the all-zero cluster used during that run, > rather than trying to find an existing one. And there's really no point > to adding a header to track an all-zero cluster in compat=1.1 images, > since those images already have the ability to track zero clusters > without needing one allocated.] >> + ret = bdrv_write_zeroes(bs->file, offset / BDRV_SECTOR_SIZE, >> + s->cluster_sectors, 0); > > That is, if bdrv_write_zeroes knows how to take advantage of an already > existing all-zero cluster, it would be less special casing in this code, > but still get the same benefits of maximizing refcount during the amend > operation, if all expanded clusters go through bdrv_write_zeroes. Now that I've looked through both variants, I'm leaning towards the simplicity of your alternate series, rather than the complexity of this one, if we can (independently?) optimize bdrv_write_zeroes to reuse a known-all-zeroes cluster when possible. Of course, you may want to get other opinions than just mine before posting your next round of these patches. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org