On Mon, Sep 04, 2017 at 04:17:42PM +0300, Anton Nefedov wrote: > > > On 31/8/2017 9:55 AM, Liu Qing wrote: > >On Wed, Aug 30, 2017 at 01:15:33PM +0300, Anton Nefedov wrote: > >> > >>On 29/08/2017 05:56, Liu Qing wrote: > >>>On Mon, Aug 28, 2017 at 10:46:34AM -0500, Eric Blake wrote: > >>>>[adding qemu-block] > >>>> > >>>>On 08/28/2017 12:56 AM, Liu Qing wrote: > >>>>>Dear list, > >>>>> Recently I used fio to test qcow2 driver in the guest os, and found > >>>>> out > >>>>>that when a new cluster is allocated the 4K IO will occupy 64K(default > >>>>>cluster > >>>>>size) bandwith. > >>>>> From the code qcow2 driver will fill the unused part of new allocated > >>>>>cluster with 0 in perform_cow. These 0s are set in qcow2_co_readv when > >>>>>the read > >>>>>destination is not allocated and it has no backing file. Could I > >>>>>forbidden any > >>>>>further write in copy_sectors if the copy source is not allocated and it > >>>>>has > >>>>>no backing file? So only the requested data is written to the cluster. > >>>>>Function > >>>>>copy_sectors is only used by perform_cow in the master branch. > >>>> > >>>>There have already been discussions on optimizing COW writes in a manner > >>>>similar to what you are describing; for example, > >>>> > >>>>https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00109.html > >>>Thanks Eric, this is what I am looking for. > >>>The only concern I have is in patch '[Qemu-devel] [PATCH v4 12/15] qcow2: > >>>skip > >>>writing zero buffers to empty' it says: > >>> > >>>It can be detected that > >>> 1. COW alignment of a write request is zeroes > >>> 2. Respective areas on the underlying BDS already read as zeroes > >>> after being preallocated previously > >>> If both of these true, COW may be skipped > >>> > >>>Will writing zero be skipped if the disk is not preallocated? @Anton > >>> > >> > >>Hi, > >> > >>In short, no, it will not (with my patches), but there might be some way > >>if that's what you really need. > >> > >> > >>First of all, this might be undesirable as you lose the cluster-size > >>data locality: now the whole cluster is written at once and is expected > >>to reside in the contiguous area on the physical drive. > >> > >>Secondly, I think there is no guarantee that the underlying bs->file > >>image reads back as zeroes if the cluster is unallocated on qcow2 level. > >Why we need this guarantee? If the cluster is unallocated, it means no > >one used these clusters previously. So why should these unallocated > >clusters be read back as zeroes? > > Hi, sorry I missed your mail; > > I'm actually not sure if this is fixed in some spec or smth, that we > must read 0 from the never-written-to areas. > > I can guess why it looks quite desirable - suppose we had a guest offset > X which mapped to the image offset Y, then the cluster got discarded and > the new guest offset Z mapped to the image offset Y - then the guest can > read old data from the other offset. But of course the sensitive data at > X should be explicitly overwritten by guest means, rather than just > discarded. > > /Anton I found the following article is disscussing similar thing. https://lwn.net/Articles/492959/ The author added a new a flag in fallocate, and uninitialized file block will not be filled with zero when only a small part of the block is written.
As you mentioned there should be some security issues. There is trade off on this. Thanks for the reply. Qing. > > >> > >>For example, the unallocated cluster could have been used earlier but > >>then discarded. Discard passthrough is configurable so discard may not > >>be passed down to the underlying image. And I guess that in general, > >>even if it is passed, there is no strong requirement on reading back as > >>zeroes - look at qcow2 discard handling - discard head and tail which do > >>not cover full clusters are ignored. > >> > >>_perhaps_, one may expect that there will be zeroes if the cluster is > >>allocated at the end of file > >>(see 'clusters_are_trailing' detection here > >>https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00122.html) > >> > >>but I haven't thought about all corner cases here. > >> > >> > >>/Anton > >> > >>>BTW: why the code in the patch is a little different than the latest > >>>master branch? For example I don't have the is_zero function but only > >>>get is_zero_sectors. Is there something wrong with my settings? > >>> > >>>My repo: > >>># git remote -v > >>>origin git://git.qemu-project.org/qemu.git (fetch) > >>>origin git://git.qemu-project.org/qemu.git (push) > >>> > >>>Thanks. > >>>> > >>>>-- > >>>>Eric Blake, Principal Software Engineer > >>>>Red Hat, Inc. +1-919-301-3266 > >>>>Virtualization: qemu.org | libvirt.org > >>>> > >>> > >>>
