[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for 2.7 1/1] qcow2: improve qcow2_co_write_zeroe

From: Denis V. Lunev
Subject: Re: [Qemu-devel] [PATCH for 2.7 1/1] qcow2: improve qcow2_co_write_zeroes()
Date: Wed, 27 Apr 2016 11:32:58 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1

On 04/27/2016 11:12 AM, Kevin Wolf wrote:
Am 27.04.2016 um 09:07 hat Denis V. Lunev geschrieben:
On 04/26/2016 01:19 PM, Kevin Wolf wrote:

Did you ever talk to the kernel people?

We can try to make the best out of suboptimal requests in qemu, but it
looks to me as if the real fix was in the kernel, and if we don't get it
fixed there, we'll see more and more of this kind of problems. I think
this is relevant not only for VMs, but probably on real hardware as
it is difficult and could be done partially only :(
Was there a discussion on some public mailing list? If so, do you have a
we have discussed this here a year ago but I am unable to see traces in the
LKML so far. It seems that the situation was resolved and patience
was lost. Though there were some efforts...

Here is the thread with Paolo and Dmitry Monakhov,
who is working with kernel block layer in Virtuozzo.


Here is the trick that I think will save us:

On a misaligned call, we call bdrv_get_block_status_above() for the
whole cluster that we're in. We know that it's only a single cluster
because bdrv_co_do_write_zeroes() splits things this way; only aligned
requests can be longer than a sector (we can even assert this).

If the result is that the cluster already reads as zero, instead of
doing nothing and possibly breaking backing chain manipulations, we
simply extend the write zeroes operation to the whole cluster and
continue as normal with an aligned request. This way we end up with a
zero cluster instead of an unallocated one, and that should be safe.

If the result is that the cluster isn't completed zeroed, return
-ENOTSUP as you did in the snippet above.

That approach should probably result in an (even) simpler patch, too.
let's start from the simple thing. I'll send a patch in a couple of minutes.
It really looks MUCH better than the original one. Thank you for this cool

Hm... Or actually, if we want something more complex that will help all
block drivers, extending the range of the request could even be done in
bdrv_co_do_write_zeroes(), I guess. I won't insist on it, though.
I do not think that this should be done now. This patch should go into
our own stable and thus it would better be as simple as possible.
As I said, I'm okay with the simple patch for now, but I must point out
that "your own stable" is an invalid argument here.

What we merge in upstream qemu must make sense for upstream qemu, and
for nothing else, even if it may be more inconvenient for downstreams
and if it may mean that downstreams have to carry non-upstream patches.
I have rejected patches upstream even though they made more sense for
RHEL, and I will reject patches that suit other downstreams if they
aren't doing the right thing for the upstream project.

So on qemu-devel, as a rule of thumb always argue with the merits that a
patch has for upstream, not with advantages for downstream.
ok, but I have not used this as an argument. You have said that "simple patch is OK", the purpose of my clause was to note "simple patch is better for me at the moment".
Sorry, if this has wrong intention.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]