[Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Liu Qing]

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.
    Do you think this change is reasonable? Thanks.

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

[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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[John Snow]

On 08/28/2017 01: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.
>     Do you think this change is reasonable? Thanks.
> 
> 

CCing qemu-block.

If I am understanding you correctly, you're noticing an adverse
performance impact from initial writes to unallocated clusters through
the qcow2 driver, when qcow2 performs zero-fill of uninitiated clusters.

First, can you look at sources for current master branch? it hasn't been
named "copy_sectors" since June 2016, and the function looks a bit
differently than it did in the version you're looking at, I think.

John

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Liu Qing]

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

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
> 

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Liu Qing]

On Mon, Aug 28, 2017 at 05:40:48PM -0400, John Snow wrote:
> 
> 
> On 08/28/2017 01: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.
> >     Do you think this change is reasonable? Thanks.
> > 
> > 
> 
> CCing qemu-block.
> 
> If I am understanding you correctly, you're noticing an adverse
> performance impact from initial writes to unallocated clusters through
> the qcow2 driver, when qcow2 performs zero-fill of uninitiated clusters.
Yes, that's it.
> 
> First, can you look at sources for current master branch? it hasn't been
> named "copy_sectors" since June 2016, and the function looks a bit
> differently than it did in the version you're looking at, I think.
I checkout the latest master branch and the code path has some changes, but the
zero-fill process still exists. Eric has replied a patch from Anton
which could improve the write performance. Thanks all.
> 
> John
> 

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Anton Nefedov]

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.

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

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Liu Qing]

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?
> 
> 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
> >>
> >
> >

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Anton Nefedov]

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

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

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Liu Qing]

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