[Qemu-devel] Re: [PATCH 1/5] block: add discard support

2010-12-13 Thread Paolo Bonzini

On 12/10/2010 02:38 PM, Christoph Hellwig wrote:

if it's smaller than the block size we'll zero out the remainder of
the block.


I think it should fail at VM startup time, or even better do nothing at all.

When you write in the middle of an absent block, and a partially-zero 
block is created, this is not visible: a read cannot see the difference 
between all zeros because it's sparse and all zeros because it's zero.


If I ask you to (optionally) punch a 1kb hole but all you can do is 
punch a 2kb hole, I do care about the second kilobyte of data.  Since 
the hole punching of bdrv_discard is completely optional, it should not 
be done in this case.


Paolo



[Qemu-devel] Re: [PATCH 1/5] block: add discard support

2010-12-13 Thread Christoph Hellwig
On Mon, Dec 13, 2010 at 05:07:27PM +0100, Paolo Bonzini wrote:
 On 12/10/2010 02:38 PM, Christoph Hellwig wrote:
 if it's smaller than the block size we'll zero out the remainder of
 the block.
 
 I think it should fail at VM startup time, or even better do nothing at all.

What should fail?

 When you write in the middle of an absent block, and a partially-zero 
 block is created, this is not visible: a read cannot see the difference 
 between all zeros because it's sparse and all zeros because it's zero.

You can not see from a VM if a block is not allocated or zeroed.  Then
again we'll never create a fully zeroed block anyway unless we get
really stupid discard patterns from the guest OS.

 If I ask you to (optionally) punch a 1kb hole but all you can do is 
 punch a 2kb hole, I do care about the second kilobyte of data.  Since 
 the hole punching of bdrv_discard is completely optional, it should not 
 be done in this case.

Of course we do not discard the second KB in that case.  If you issue
a 1k UNRSVSP ioctl on a 2k block size XFS filesystem it will zero
exactly the 1k you specified, which is required for the semantics of the
ioctl.  Yes, it's not optimal, but qemu can't easily know what block
size the underlying filesystem has.




[Qemu-devel] Re: [PATCH 1/5] block: add discard support

2010-12-13 Thread Paolo Bonzini

On 12/13/2010 05:15 PM, Christoph Hellwig wrote:

On Mon, Dec 13, 2010 at 05:07:27PM +0100, Paolo Bonzini wrote:

On 12/10/2010 02:38 PM, Christoph Hellwig wrote:

if it's smaller than the block size we'll zero out the remainder of
the block.


I think it should fail at VM startup time, or even better do nothing at all.


What should fail?


Nothing -- you wrote if it's smaller than the block size we'll zero out 
the remainder of the block which I interpreted the wrong way, i.e. as 
XFS will round up the size to the remainder of the block and zero that 
part out as well.


Thanks for the clarification.

Paolo