qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on c


From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert
Date: Thu, 18 Jul 2013 13:04:40 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130623 Thunderbird/17.0.7

On 18.07.2013 12:56, Paolo Bonzini wrote:
Il 18/07/2013 12:44, Peter Lieven ha scritto:
On 18.07.2013 12:24, Paolo Bonzini wrote:
Il 18/07/2013 11:23, Kevin Wolf ha scritto:
Am 17.07.2013 um 19:48 hat Peter Lieven geschrieben:
Am 17.07.2013 um 19:04 schrieb Paolo Bonzini <address@hidden>:

Il 17/07/2013 19:02, Peter Lieven ha scritto:
For Disks we always use read/write16 so i think we Should also use
writesame16. Or not?
Yes.

Remember you can still use UNMAP if LBPRZ=0.
I can always use it if writesame is not available, but in this case
bdi->discard_zeroes must be 0.

Maybe we should call it discard_writes_zeroes or similar.

Discard_zeroes is sth that should only indicate if lbprz == 1. At
least if we refer to the Linux ioctl. We could include both in BDI.
Maybe what we really should do is to define different operations (with
an exact behaviour) instead of having one bdrv_discard() and then adding
flags everywhere to tell what the operation is doing exactly.
A BDRV_MAY_UNMAP flag for bdrv_write_zeroes?
I thought that we wanted to add a paramter to the BDI (call it
write_zeroes_w_discard).
I also thought so, but I like Kevin's idea of not shoehorning it in
bdrv_discard().  Extending bdrv_write_zeroes is a better API.

So far we've avoided "discard zeroes" semantics in QEMU (device models
are also careful not to expose that).  Since the only sane way to
implement what you want is to use the SCSI WRITE SAME command, adding
flags to bdrv_write_zeroes will even be easier because the mapping with
SCSI is natural: discard = UNMAP, write_zeroes = WRITE SAME (either
without or with the UNMAP bit).

If this is set the bdrv MUST accept a flag to bdrv_discard() lets call
it BDRV_DISCARD_WRITE_ZEROES
and he has to ensure that all sectors specified in bdrv_discard() read
as zero after the operation.

If this flag is not set (e.g. when the OS issues a normal discard) the
operation may still silently fail with
undefined provisioning state and content of the specified sectors.
But if you set BDRV_DISCARD_WRITE_ZEROES, then you always need a
fallback to bdrv_write_zeroes.  Why not just call bdrv_write_zeroes to
begin with?  That's why extending bdrv_write_zeroes is preferable.
In this case wo do not need a flag to the function at all. If the
driver sets bdi->write_zeroes_w_discard = 1 then bdrv_write_zeroes
can use bdrv_discard to write zeroes and the driver has to
ensure that all is zero afterwards.

If the driver would have a better method of writing zeroes than
discard it simply should not set bdi->write_zeroes_w_discard = 1.

Peter



reply via email to

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