Re: [Qemu-devel] [PATCH 25/27] block/parallels: add prealloc-mode and prealloc-size open paramemets

[Roman Kagan]

On Tue, Apr 28, 2015 at 01:59:56PM +0300, Roman Kagan wrote:
> On Tue, Apr 28, 2015 at 10:46:58AM +0300, Denis V. Lunev wrote:
> > This is preparational commit for tweaks in Parallels image expansion.
> > The idea is that enlarge via truncate by one data block is slow. It
> > would be much better to use fallocate via bdrv_write_zeroes and
> > expand by some significant amount at once.
> > 
> > Original idea with sequential file writing to the end of the file without
> > fallocate/truncate would be slower than this approach if the image is
> > expanded with several operations:
> > - each image expanding means file metadata update, i.e. filesystem
> >   journal write. Truncate/write to newly truncated space update file
> >   metadata twice thus truncate removal helps. With fallocate call
> >   inside bdrv_write_zeroes file metadata is updated only once and
> >   this should happen infrequently thus this approach is the best one
> >   for the image expansion
> > - tail writes are ordered, i.e. the guest IO queue could not be sent
> >   immediately to the host introducing additional IO delays
> > 
> > This patch just adds proper parameters into BDRVParallelsState and
> > performs options parsing in parallels_open.
> > 
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Roman Kagan <address@hidden>
> > CC: Kevin Wolf <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > ---
> >  block/parallels.c | 83 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 77 insertions(+), 6 deletions(-)
> 
> It may be slightly more logical to leave truncate support out of patch
> 09 sticking with bdrv_write_zeros there, and introduce it all in this
> patch.
> 
> Otherwise looks good to me.

I mean,

Reviewed-by: Roman Kagan <address@hidden>

Roman.