| Message ID | 1419934032-24216-9-git-send-email-den@openvz.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 30, 2014 at 01:07:11PM +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. > > This patch just adds proper parameters into BDRVParallelsState and > performs options parsing in parallels_open. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/parallels.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/block/parallels.c b/block/parallels.c > index 18b9267..12a9cea 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -30,6 +30,7 @@ > #include "qemu-common.h" > #include "block/block_int.h" > #include "qemu/module.h" > +#include "qapi/util.h" > > /**************************************************************/ > > @@ -54,6 +55,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, There is already generic "preallocaton" option, BLOCK_OPT_PREALLOC, which is handled by qcow2 and raw-posix. It means slightly different thing: the *whole* image is preallocated using the method specified. I think it would make sense to consolidate that option with this new batched allocation in the generic block code. I guess qcow2 and raw-posix would benefit from it, too. At any rate I think it's a matter for a separate patchset. Roman.
On 14/01/15 17:26, Roman Kagan wrote: > On Tue, Dec 30, 2014 at 01:07:11PM +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. >> >> This patch just adds proper parameters into BDRVParallelsState and >> performs options parsing in parallels_open. >> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Kevin Wolf <kwolf@redhat.com> >> CC: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> block/parallels.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 72 insertions(+) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index 18b9267..12a9cea 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -30,6 +30,7 @@ >> #include "qemu-common.h" >> #include "block/block_int.h" >> #include "qemu/module.h" >> +#include "qapi/util.h" >> >> /**************************************************************/ >> >> @@ -54,6 +55,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, > There is already generic "preallocaton" option, BLOCK_OPT_PREALLOC, > which is handled by qcow2 and raw-posix. It means slightly different > thing: the *whole* image is preallocated using the method specified. > > I think it would make sense to consolidate that option with this new > batched allocation in the generic block code. I guess qcow2 and > raw-posix would benefit from it, too. > > At any rate I think it's a matter for a separate patchset. > > Roman. it is too early :) I think that I should provide the rationale for the preallocation in general. I am working on this with CEPH :)
On Wed, Jan 14, 2015 at 05:31:20PM +0300, Denis V. Lunev wrote: > On 14/01/15 17:26, Roman Kagan wrote: > >On Tue, Dec 30, 2014 at 01:07:11PM +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. > >> > >>This patch just adds proper parameters into BDRVParallelsState and > >>performs options parsing in parallels_open. > >> > >>Signed-off-by: Denis V. Lunev <den@openvz.org> > >>CC: Kevin Wolf <kwolf@redhat.com> > >>CC: Stefan Hajnoczi <stefanha@redhat.com> > >>--- > >> block/parallels.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 72 insertions(+) > >> > >>diff --git a/block/parallels.c b/block/parallels.c > >>index 18b9267..12a9cea 100644 > >>--- a/block/parallels.c > >>+++ b/block/parallels.c > >>@@ -30,6 +30,7 @@ > >> #include "qemu-common.h" > >> #include "block/block_int.h" > >> #include "qemu/module.h" > >>+#include "qapi/util.h" > >> /**************************************************************/ > >>@@ -54,6 +55,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, > >There is already generic "preallocaton" option, BLOCK_OPT_PREALLOC, > >which is handled by qcow2 and raw-posix. It means slightly different > >thing: the *whole* image is preallocated using the method specified. > > > >I think it would make sense to consolidate that option with this new > >batched allocation in the generic block code. I guess qcow2 and > >raw-posix would benefit from it, too. > > > >At any rate I think it's a matter for a separate patchset. > > > >Roman. > it is too early :) I think that I should provide the rationale for > the preallocation in general. I am working on this with CEPH :) OK then, maybe indeed it should land in parallels driver first, and move into the generic code if found appropriate... Then my only problem with this patch is the naming of the options: they look too similar to the generic one while the meaning is different. Maybe "batched_alloc" reflects the meaning better? Roman.
diff --git a/block/parallels.c b/block/parallels.c index 18b9267..12a9cea 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -30,6 +30,7 @@ #include "qemu-common.h" #include "block/block_int.h" #include "qemu/module.h" +#include "qapi/util.h" /**************************************************************/ @@ -54,6 +55,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 { CoMutex lock; @@ -67,12 +82,40 @@ typedef struct BDRVParallelsState { int bat_cache_off; int data_off; + uint64_t prealloc_size; + ParallelsPreallocMode prealloc_mode; + unsigned int tracks; unsigned int off_multiplier; } 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 uint32_t bat_offset(uint32_t index) { return sizeof(ParallelsHeader) + sizeof(uint32_t) * index; @@ -99,6 +142,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, BDRVParallelsState *s = bs->opaque; int i; int ret; + QemuOpts *opts = NULL; + Error *local_err = NULL; + char *buf; ret = bdrv_pread(bs->file, 0, &s->ph, sizeof(s->ph)); if (ret < 0) { @@ -149,6 +195,27 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } + opts = qemu_opts_create(¶llels_runtime_opts, NULL, 0, &local_err); + if (local_err != NULL) { + goto fail_options; + } + + qemu_opts_absorb_qdict(opts, options, &local_err); + if (local_err != NULL) { + goto fail_options; + } + + s->prealloc_size = + qemu_opt_get_size_del(opts, PARALLELS_OPT_PREALLOC_SIZE, 0); + s->prealloc_size = MAX(s->tracks, s->prealloc_size >> BDRV_SECTOR_BITS); + buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE); + s->prealloc_mode = qapi_enum_parse(prealloc_mode_lookup, buf, + PRL_PREALLOC_MODE_MAX, PRL_PREALLOC_MODE_FALLOCATE, &local_err); + g_free(buf); + if (local_err != NULL) { + goto fail_options; + } + for (i = 0; i < s->bat_size; i++) le32_to_cpus(&s->bat[i]); @@ -172,6 +239,11 @@ fail_format: fail: g_free(s->bat); return ret; + +fail_options: + error_propagate(errp, local_err); + ret = -EINVAL; + goto fail; } static int64_t seek_to_sector(BDRVParallelsState *s, int64_t sector_num)
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. This patch just adds proper parameters into BDRVParallelsState and performs options parsing in parallels_open. Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Kevin Wolf <kwolf@redhat.com> CC: Stefan Hajnoczi <stefanha@redhat.com> --- block/parallels.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)