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 <d...@openvz.org> > CC: Roman Kagan <rka...@parallels.com> > CC: Kevin Wolf <kw...@redhat.com> > CC: Stefan Hajnoczi <stefa...@redhat.com> > --- > block/parallels.c | 83 > +++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 77 insertions(+), 6 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index 05fe030..440938e 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -31,6 +31,7 @@ > #include "block/block_int.h" > #include "qemu/module.h" > #include "qemu/bitmap.h" > +#include "qapi/util.h" > > /**************************************************************/ > > @@ -56,6 +57,20 @@ typedef struct ParallelsHeader { > char padding[12]; > } QEMU_PACKED ParallelsHeader; > > + > +typedef enum ParallelsPreallocMode { > + PRL_PREALLOC_MODE_FALLOCATE = 0, > + PRL_PREALLOC_MODE_TRUNCATE = 1, > + PRL_PREALLOC_MODE_MAX = 2, > +} ParallelsPreallocMode; > + > +static const char *prealloc_mode_lookup[] = { > + "falloc", > + "truncate", > + NULL, > +}; > + > + > typedef struct BDRVParallelsState { > /** Locking is conservative, the lock protects > * - image file extending (truncate, fallocate) > @@ -73,14 +88,40 @@ typedef struct BDRVParallelsState { > uint32_t *bat_bitmap; > unsigned int bat_size; > > + uint64_t prealloc_size; > + ParallelsPreallocMode prealloc_mode; > + > unsigned int tracks; > > unsigned int off_multiplier; > - > - bool has_truncate; > } BDRVParallelsState; > > > +#define PARALLELS_OPT_PREALLOC_MODE "prealloc-mode" > +#define PARALLELS_OPT_PREALLOC_SIZE "prealloc-size" > + > +static QemuOptsList parallels_runtime_opts = { > + .name = "parallels", > + .head = QTAILQ_HEAD_INITIALIZER(parallels_runtime_opts.head), > + .desc = { > + { > + .name = PARALLELS_OPT_PREALLOC_SIZE, > + .type = QEMU_OPT_SIZE, > + .help = "Preallocation size on image expansion", > + .def_value_str = "128MiB", > + }, > + { > + .name = PARALLELS_OPT_PREALLOC_MODE, > + .type = QEMU_OPT_STRING, > + .help = "Preallocation mode on image expansion " > + "(allowed values: falloc, truncate)", > + .def_value_str = "falloc", > + }, > + { /* end of list */ }, > + }, > +}; > + > + > static int64_t bat2sect(BDRVParallelsState *s, uint32_t idx) > { > return (uint64_t)le32_to_cpu(s->bat_bitmap[idx]) * s->off_multiplier; > @@ -159,7 +200,7 @@ static int64_t allocate_cluster(BlockDriverState *bs, > int64_t sector_num) > } > > pos = bdrv_getlength(bs->file) >> BDRV_SECTOR_BITS; > - if (s->has_truncate) { > + if (s->prealloc_mode == PRL_PREALLOC_MODE_TRUNCATE) { > ret = bdrv_truncate(bs->file, (pos + s->tracks) << BDRV_SECTOR_BITS); > } else { > ret = bdrv_write_zeroes(bs->file, pos, s->tracks, 0); > @@ -509,6 +550,9 @@ static int parallels_open(BlockDriverState *bs, QDict > *options, int flags, > BDRVParallelsState *s = bs->opaque; > ParallelsHeader ph; > int ret, size; > + QemuOpts *opts = NULL; > + Error *local_err = NULL; > + char *buf; > > ret = bdrv_pread(bs->file, 0, &ph, sizeof(ph)); > if (ret < 0) { > @@ -567,9 +611,6 @@ static int parallels_open(BlockDriverState *bs, QDict > *options, int flags, > } > s->bat_bitmap = (uint32_t *)(s->header + 1); > > - s->has_truncate = bdrv_has_zero_init(bs->file) && > - bdrv_truncate(bs->file, bdrv_getlength(bs->file)) == 0; > - > if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) { > /* Image was not closed correctly. The check is mandatory */ > s->header_unclean = true;
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. Roman.