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: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert
Date: Thu, 12 Sep 2013 10:55:10 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8

Il 12/09/2013 10:52, Peter Lieven ha scritto:
> On 18.07.2013 16:14, Paolo Bonzini wrote:
>> Il 18/07/2013 15:55, ronnie sahlberg ha scritto:
>>>>> bdrv->write_zeroes will use writesame16 and set the unmap flag only if
>>>>> BDRV_MAY_DISCARD == 1 and BDRV_O_UNMAP == 1 and lbprz == 1.
>>> When you use WRITESAME16 you can ignore the lbprz flag.
>>> Just send a WRITESAME16 command with one block of data that is all
>>> set to zero.
>>> If the unmap flag is set and if unmapped blocks read back the same as
>>> the block you provided (all zero)
>>> then it will unmap those blocks, where possible.
>> True, so the unmap flag can be set iff BDRV_MAY_DISCARD == 1.  block.c
>> can take care of checking BDRV_O_UNMAP.
> Would you add BDRV_MAY_DISCARD to BdrvRequestFlags?
> 
> This would require making BdrvRequestFlags public.
> 
> Before I start implementation I would still like to verify if we need this
> flag. E.g. in which case would we do not want to optimize writing zeroes
> via discard if the user has set BDRV_O_UNMAP?

For example if the bdrv_write_zeroes is coming from a WRITE SAME command
(sent by the guest to an emulated SCSI disk).  In this case, we should
not discard if the UNMAP bit is zero in the request.

The WRITE SAME implementation in scsi-disk.c is old and broken, but the
block layer should provide enough API to make it right.

Paolo

> One case I would think of is sanitizing data. But in this case shouldn't
> this
> be a different function? In case of a container format its also not
> guaranteed that the
> contents are erased from the underlying media. Looking at the commit
> message
> for f08f2ddae078e8a7683f8b16da8e0cc3029c7b89 it sounds that
> bdrv_write_zeroes
> was intended to provide an efficient way to zero contents, but only in
> the sense
> that it is guaranteed that they read back as zero (compared to
> bdrv_discard).
> 
> Peter
> 




reply via email to

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