qemu-devel
[Top][All Lists]
Advanced

[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: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH for 2.7 1/1] qcow2: improve qcow2_co_write_zeroes()
Date: Wed, 27 Apr 2016 10:12:04 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 27.04.2016 um 09:07 hat Denis V. Lunev geschrieben:
> On 04/26/2016 01:19 PM, Kevin Wolf wrote:
> 
> [skipped]
> >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
> >well.
> it is difficult and could be done partially only :(

Was there a discussion on some public mailing list? If so, do you have a
pointer?

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

Kevin



reply via email to

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