| Message ID | 20200918181951.21752-9-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | preallocate filter | expand |
On 18.09.20 20:19, Vladimir Sementsov-Ogievskiy wrote: > It's intended to be inserted between format and protocol nodes to > preallocate additional space (expanding protocol file) on writes > crossing EOF. It improves performance for file-systems with slow > allocation. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > docs/system/qemu-block-drivers.rst.inc | 26 ++ > qapi/block-core.json | 20 +- > block/preallocate.c | 556 +++++++++++++++++++++++++ > block/meson.build | 1 + > 4 files changed, 602 insertions(+), 1 deletion(-) > create mode 100644 block/preallocate.c Looks good to me in general. [...] > diff --git a/block/preallocate.c b/block/preallocate.c > new file mode 100644 > index 0000000000..6510ad0149 > --- /dev/null > +++ b/block/preallocate.c [...] > +/* > + * Handle reopen. > + * > + * We must implement reopen handlers, otherwise reopen just don't work. Handle > + * new options and don't care about preallocation state, as it is handled in > + * set/check permission handlers. > + */ > + > +static int preallocate_reopen_prepare(BDRVReopenState *reopen_state, > + BlockReopenQueue *queue, Error **errp) > +{ > + PreallocateOpts *opts = g_new0(PreallocateOpts, 1); > + > + if (!preallocate_absorb_opts(opts, reopen_state->options, > + reopen_state->bs->file->bs, errp)) { I tried to find out whether this refers to the old file child, or the post-reopen one. As far as I could find out, there is no generic implementation for changing the file child as part of x-blockdev-reopen: {"error": {"class": "GenericError", "desc": "Cannot change the option 'file'"}} Now that’s a shame. That means you can’t reasonably integrate a preallocate filter into an existing node graph unless the format driver checks for the respective child option and issues a replace_node on commit or something, right? I suppose any driver who’d want to implement child replacement would need to attach the new node in prepare() as some pseudo-child, and then drop the old one and rename the new one in commit(). I don’t think any driver does that yet, so I suppose no format driver allows replacement of children yet (except for quorum...). Does anyone know what the status on that is? Are there any plans for implementing child replacement in reopen, or did I just miss something? (It looks like the backing child can be replaced, but that’s probably not a child where the preallocate filter would be placed on top...). > + g_free(opts); > + return -EINVAL; > + } > + > + reopen_state->opaque = opts; > + > + return 0; > +} [...] > +/* > + * Call on each write. Returns true if @want_merge_zero is true and the region > + * [offset, offset + bytes) is zeroed (as a result of this call or earlier > + * preallocation). > + * > + * want_merge_zero is used to merge write-zero request with preallocation in > + * one bdrv_co_pwrite_zeroes() call. > + */ > +static bool coroutine_fn handle_write(BlockDriverState *bs, int64_t offset, > + int64_t bytes, bool want_merge_zero) > +{ > + BDRVPreallocateState *s = bs->opaque; > + int64_t end = offset + bytes; > + int64_t prealloc_start, prealloc_end; > + int ret; > + > + if (!has_prealloc_perms(bs)) { Took me a bit to figure out, because it takes a trip to preallocate_child_perm() to figure out exactly when we’re going to have the necessary permissions for this to pass. Then it turns out that this is going to be the case exactly when the parents collectively have the same permissions (WRITE+RESIZE) on the preallocate node. Then I had to wonder whether it’s appropriate not to preallocate if WRITE is taken, but RESIZE isn’t. But that seems entirely correct, yes. If noone is going to grow the file, then there is no need for preallocation. (Vice versa, if noone is going to write, but only resize, then there is no need for preallocation either.) So this seems correct, yes. (It could be argued that if one parent has WRITE, and another has RESIZE (but neither has both), then we probably don’t need preallocation either. But in such an arcane case (which is impossible to figure out in .bdrv_child_perm() anyway), we might as well just do preallocation. Won’t hurt.) > + /* We don't have state neither should try to recover it */ > + return false; > + } > + > + if (s->data_end < 0) { > + s->data_end = bdrv_getlength(bs->file->bs); > + if (s->data_end < 0) { > + return false; > + } > + > + if (s->file_end < 0) { > + s->file_end = s->data_end; > + } > + } > + > + if (end <= s->data_end) { > + return false; > + } > + > + /* We have valid s->data_end, and request writes beyond it. */ > + > + s->data_end = end; > + if (s->zero_start < 0 || !want_merge_zero) { > + s->zero_start = end; Skipping this on want_merge_zero == true means that zero writes can be cached; if you repeatedly perform zero writes into the preallocated area, then none of those will actually be executed. I legitimately don’t know whether that’s OK. I suppose until someone tells me it isn’t OK, I’ll believe it is. > + } > + > + if (s->file_end < 0) { > + s->file_end = bdrv_getlength(bs->file->bs); > + if (s->file_end < 0) { > + return false; > + } > + } > + > + /* Now s->data_end, s->zero_start and s->file_end are valid. */ > + > + if (end <= s->file_end) { > + /* No preallocation needed. */ > + return want_merge_zero && offset >= s->zero_start; > + } > + > + /* Now we want new preallocation, as request writes beyond s->data_end. */ s/data_end/file_end/ > + > + prealloc_start = want_merge_zero ? MIN(offset, s->file_end) : s->file_end; I suppose you intentionally use s->file_end here instead of @end, even if offset <= s->file_end. I just mention it because I wonder whether it’s really better to effectively write twice to the same area in such cases (once zeroes for preallocation, then immediately the data) instead of only writing the data and then preallocating past it. (Though if it were the same code just with @end instead of s->file_end for offset <= s->file_end, then the order would be to preallocate past @end, and then to write the data. Which might be suboptimal in terms of how the blocks are then ordered in the filesystem.) > + prealloc_end = QEMU_ALIGN_UP(offset + bytes + s->opts.prealloc_size, s/offset + bytes/end/? > + s->opts.prealloc_align); > + s->file_end = end; Why is this set here, when it’s always overwritten after bdrv_co_pwrite_zeroes() anyway? (It also seems a bit wrong, because at this point we don’t know yet whether the data write is going to succeed, so we don’t know for sure whether the file end is really going to be @end without the preallocation.) > + > + ret = bdrv_co_pwrite_zeroes( > + bs->file, prealloc_start, prealloc_end - prealloc_start, > + BDRV_REQ_NO_FALLBACK | BDRV_REQ_SERIALISING | BDRV_REQ_NO_WAIT); > + if (ret < 0) { > + s->file_end = ret; > + return false; > + } > + > + s->file_end = prealloc_end; > + return want_merge_zero; > +} > + > +static int coroutine_fn preallocate_co_pwrite_zeroes(BlockDriverState *bs, > + int64_t offset, int bytes, BdrvRequestFlags flags) > +{ > + bool want_merge_zero = > + (flags & (BDRV_REQ_ZERO_WRITE | BDRV_REQ_NO_FALLBACK)) == flags; Isn’t this the same as !(flags & ~(ZERO_WRITE | NO_FALLBACK))? (Maybe only I would find that simpler to understand, though.) > + if (handle_write(bs, offset, bytes, want_merge_zero)) { > + return 0; > + } > + > + return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags); > +} [...] > +static int coroutine_fn > +preallocate_co_truncate(BlockDriverState *bs, int64_t offset, > + bool exact, PreallocMode prealloc, > + BdrvRequestFlags flags, Error **errp) > +{ > + ERRP_GUARD(); > + BDRVPreallocateState *s = bs->opaque; > + int ret; > + > + if (s->data_end >= 0 && offset > s->data_end) { > + if (s->file_end < 0) { > + s->file_end = bdrv_getlength(bs->file->bs); > + if (s->file_end < 0) { > + error_setg(errp, "failed to get file length"); > + return s->file_end; > + } > + } > + > + if (prealloc == PREALLOC_MODE_FALLOC) { > + /* > + * If offset <= s->file_end, the task is already done, just > + * update s->file_end, to move part of "filter preallocation" s/file_end/data_end/ > + * to "preallocation requested by user". > + * Otherwise just proceed to preallocate missing part. > + */ > + if (offset <= s->file_end) { > + s->data_end = offset; > + return 0; > + } [...] > +static int preallocate_check_perm(BlockDriverState *bs, > + uint64_t perm, uint64_t shared, Error **errp) > +{ > + BDRVPreallocateState *s = bs->opaque; > + > + if (s->data_end >= 0 && !can_write_resize(perm)) { > + /* > + * Loose permissions. *Lose (I assume) > + * We should truncate in check_perm, as in set_perm bs->file->perm will > + * be already changed, and we should not violate it. > + */ > + if (s->file_end < 0) { > + s->file_end = bdrv_getlength(bs->file->bs); > + if (s->file_end < 0) { > + error_setg(errp, "Failed to get file length"); > + return s->file_end; > + } > + } > + > + if (s->data_end < s->file_end) { > + int ret = bdrv_truncate(bs->file, s->data_end, true, > + PREALLOC_MODE_OFF, 0, NULL); > + if (ret < 0) { > + error_setg(errp, "Failed to drop preallocation"); > + s->file_end = ret; > + return ret; > + } > + } > + /* > + * We will drop our permissions, as well as allow shared > + * permissions, anyone will be able to change the child, so mark > + * all states invalid. We'll regain control if get good permissions > + * back. > + */ > + s->data_end = s->file_end = s->zero_start = -EINVAL; Shouldn’t we clear these variables whenever !can_write_resize(perm), not just if also s->data_end >= 0? > + } > + > + return 0; > +} Max
24.09.2020 19:50, Max Reitz wrote: > On 18.09.20 20:19, Vladimir Sementsov-Ogievskiy wrote: >> It's intended to be inserted between format and protocol nodes to >> preallocate additional space (expanding protocol file) on writes >> crossing EOF. It improves performance for file-systems with slow >> allocation. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> docs/system/qemu-block-drivers.rst.inc | 26 ++ >> qapi/block-core.json | 20 +- >> block/preallocate.c | 556 +++++++++++++++++++++++++ >> block/meson.build | 1 + >> 4 files changed, 602 insertions(+), 1 deletion(-) >> create mode 100644 block/preallocate.c > > Looks good to me in general. > > [...] > >> diff --git a/block/preallocate.c b/block/preallocate.c >> new file mode 100644 >> index 0000000000..6510ad0149 >> --- /dev/null >> +++ b/block/preallocate.c > > [...] > >> +/* >> + * Handle reopen. >> + * >> + * We must implement reopen handlers, otherwise reopen just don't work. Handle >> + * new options and don't care about preallocation state, as it is handled in >> + * set/check permission handlers. >> + */ >> + >> +static int preallocate_reopen_prepare(BDRVReopenState *reopen_state, >> + BlockReopenQueue *queue, Error **errp) >> +{ >> + PreallocateOpts *opts = g_new0(PreallocateOpts, 1); >> + >> + if (!preallocate_absorb_opts(opts, reopen_state->options, >> + reopen_state->bs->file->bs, errp)) { > > I tried to find out whether this refers to the old file child, or the > post-reopen one. Note, that it's needed only to check request_alignment. Probably it's better to pass request_alignment to preallocate_absorb_opts, not the whole child. > As far as I could find out, there is no generic > implementation for changing the file child as part of x-blockdev-reopen: > > {"error": {"class": "GenericError", "desc": "Cannot change the option > 'file'"}} > > Now that’s a shame. That means you can’t reasonably integrate a > preallocate filter into an existing node graph unless the format driver > checks for the respective child option and issues a replace_node on > commit or something, right? I suppose any driver who’d want to > implement child replacement would need to attach the new node in > prepare() as some pseudo-child, and then drop the old one and rename the > new one in commit(). I don’t think any driver does that yet, so I > suppose no format driver allows replacement of children yet (except for > quorum...). > > Does anyone know what the status on that is? Are there any plans for > implementing child replacement in reopen, or did I just miss something? > > (It looks like the backing child can be replaced, but that’s probably > not a child where the preallocate filter would be placed on top...). Hm. I didn't care about it, because main use case is to insert the filter at start, specifying it in -drive or in -blockdev options. Probably, we need a separate API which will insert/remove filters like it is done in block jobs code, not reopening the block device. > >> + g_free(opts); >> + return -EINVAL; >> + } >> + >> + reopen_state->opaque = opts; >> + >> + return 0; >> +} > > [...] > >> +/* >> + * Call on each write. Returns true if @want_merge_zero is true and the region >> + * [offset, offset + bytes) is zeroed (as a result of this call or earlier >> + * preallocation). >> + * >> + * want_merge_zero is used to merge write-zero request with preallocation in >> + * one bdrv_co_pwrite_zeroes() call. >> + */ >> +static bool coroutine_fn handle_write(BlockDriverState *bs, int64_t offset, >> + int64_t bytes, bool want_merge_zero) >> +{ >> + BDRVPreallocateState *s = bs->opaque; >> + int64_t end = offset + bytes; >> + int64_t prealloc_start, prealloc_end; >> + int ret; >> + >> + if (!has_prealloc_perms(bs)) { > > Took me a bit to figure out, because it takes a trip to > preallocate_child_perm() to figure out exactly when we’re going to have > the necessary permissions for this to pass. > > Then it turns out that this is going to be the case exactly when the > parents collectively have the same permissions (WRITE+RESIZE) on the > preallocate node. > > Then I had to wonder whether it’s appropriate not to preallocate if > WRITE is taken, but RESIZE isn’t. But that seems entirely correct, yes. > If noone is going to grow the file, then there is no need for > preallocation. (Vice versa, if noone is going to write, but only > resize, then there is no need for preallocation either.) > > So this seems correct, yes. > > (It could be argued that if one parent has WRITE, and another has RESIZE > (but neither has both), then we probably don’t need preallocation > either. But in such an arcane case (which is impossible to figure out > in .bdrv_child_perm() anyway), we might as well just do preallocation. > Won’t hurt.) > >> + /* We don't have state neither should try to recover it */ >> + return false; >> + } >> + >> + if (s->data_end < 0) { >> + s->data_end = bdrv_getlength(bs->file->bs); >> + if (s->data_end < 0) { >> + return false; >> + } >> + >> + if (s->file_end < 0) { >> + s->file_end = s->data_end; >> + } >> + } >> + >> + if (end <= s->data_end) { >> + return false; >> + } >> + >> + /* We have valid s->data_end, and request writes beyond it. */ >> + >> + s->data_end = end; >> + if (s->zero_start < 0 || !want_merge_zero) { >> + s->zero_start = end; > > Skipping this on want_merge_zero == true means that zero writes can be > cached; if you repeatedly perform zero writes into the preallocated > area, then none of those will actually be executed. I legitimately > don’t know whether that’s OK. > > I suppose until someone tells me it isn’t OK, I’ll believe it is. Skipping zero-writes to preallocated area is significant for performance. If I remember correctly, handle_alloc_space() in qcow2 will otherwise do extra write-zeroes, which reduces performance. So, we assume that zero-write after EOF is a kind of preallocation. And avoid preallocating same region twice. Using zero_start instead of data_end is probably less significant, I'm not sure. > >> + } >> + >> + if (s->file_end < 0) { >> + s->file_end = bdrv_getlength(bs->file->bs); >> + if (s->file_end < 0) { >> + return false; >> + } >> + } >> + >> + /* Now s->data_end, s->zero_start and s->file_end are valid. */ >> + >> + if (end <= s->file_end) { >> + /* No preallocation needed. */ >> + return want_merge_zero && offset >= s->zero_start; >> + } >> + >> + /* Now we want new preallocation, as request writes beyond s->data_end. */ > > s/data_end/file_end/> >> + >> + prealloc_start = want_merge_zero ? MIN(offset, s->file_end) : s->file_end; > > I suppose you intentionally use s->file_end here instead of @end, even > if offset <= s->file_end. I just mention it because I wonder whether > it’s really better to effectively write twice to the same area in such > cases (once zeroes for preallocation, then immediately the data) instead > of only writing the data and then preallocating past it. > > (Though if it were the same code just with @end instead of s->file_end > for offset <= s->file_end, then the order would be to preallocate past > @end, and then to write the data. Which might be suboptimal in terms of > how the blocks are then ordered in the filesystem.) Yes, I think it's better to preallocate from file_end, in this way we do our best for good file blocks location in filesystem. And writing twice shouldn't really matter with big enough preallocation size (a lot larger than usual guest write operation). > >> + prealloc_end = QEMU_ALIGN_UP(offset + bytes + s->opts.prealloc_size, > > s/offset + bytes/end/? yes > >> + s->opts.prealloc_align); >> + s->file_end = end; > > Why is this set here, when it’s always overwritten after > bdrv_co_pwrite_zeroes() anyway? > > (It also seems a bit wrong, because at this point we don’t know yet > whether the data write is going to succeed, so we don’t know for sure > whether the file end is really going to be @end without the preallocation.) Agree, that's strange and seems wrong. Will drop. > >> + >> + ret = bdrv_co_pwrite_zeroes( >> + bs->file, prealloc_start, prealloc_end - prealloc_start, >> + BDRV_REQ_NO_FALLBACK | BDRV_REQ_SERIALISING | BDRV_REQ_NO_WAIT); >> + if (ret < 0) { >> + s->file_end = ret; >> + return false; >> + } >> + >> + s->file_end = prealloc_end; >> + return want_merge_zero; >> +} >> + >> +static int coroutine_fn preallocate_co_pwrite_zeroes(BlockDriverState *bs, >> + int64_t offset, int bytes, BdrvRequestFlags flags) >> +{ >> + bool want_merge_zero = >> + (flags & (BDRV_REQ_ZERO_WRITE | BDRV_REQ_NO_FALLBACK)) == flags; > > Isn’t this the same as !(flags & ~(ZERO_WRITE | NO_FALLBACK))? (Maybe > only I would find that simpler to understand, though.) Actually, I think yours is more usual notation and it uses "flags" once. Will change. > >> + if (handle_write(bs, offset, bytes, want_merge_zero)) { >> + return 0; >> + } >> + >> + return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags); >> +} > > [...] > >> +static int coroutine_fn >> +preallocate_co_truncate(BlockDriverState *bs, int64_t offset, >> + bool exact, PreallocMode prealloc, >> + BdrvRequestFlags flags, Error **errp) >> +{ >> + ERRP_GUARD(); >> + BDRVPreallocateState *s = bs->opaque; >> + int ret; >> + >> + if (s->data_end >= 0 && offset > s->data_end) { >> + if (s->file_end < 0) { >> + s->file_end = bdrv_getlength(bs->file->bs); >> + if (s->file_end < 0) { >> + error_setg(errp, "failed to get file length"); >> + return s->file_end; >> + } >> + } >> + >> + if (prealloc == PREALLOC_MODE_FALLOC) { >> + /* >> + * If offset <= s->file_end, the task is already done, just >> + * update s->file_end, to move part of "filter preallocation" > > s/file_end/data_end/ > >> + * to "preallocation requested by user". >> + * Otherwise just proceed to preallocate missing part. >> + */ >> + if (offset <= s->file_end) { >> + s->data_end = offset; >> + return 0; >> + } > > [...] > >> +static int preallocate_check_perm(BlockDriverState *bs, >> + uint64_t perm, uint64_t shared, Error **errp) >> +{ >> + BDRVPreallocateState *s = bs->opaque; >> + >> + if (s->data_end >= 0 && !can_write_resize(perm)) { >> + /* >> + * Loose permissions. > > *Lose > > (I assume) Oops) > >> + * We should truncate in check_perm, as in set_perm bs->file->perm will >> + * be already changed, and we should not violate it. >> + */ >> + if (s->file_end < 0) { >> + s->file_end = bdrv_getlength(bs->file->bs); >> + if (s->file_end < 0) { >> + error_setg(errp, "Failed to get file length"); >> + return s->file_end; >> + } >> + } >> + >> + if (s->data_end < s->file_end) { >> + int ret = bdrv_truncate(bs->file, s->data_end, true, >> + PREALLOC_MODE_OFF, 0, NULL); >> + if (ret < 0) { >> + error_setg(errp, "Failed to drop preallocation"); >> + s->file_end = ret; >> + return ret; >> + } >> + } >> + /* >> + * We will drop our permissions, as well as allow shared >> + * permissions, anyone will be able to change the child, so mark >> + * all states invalid. We'll regain control if get good permissions >> + * back. >> + */ >> + s->data_end = s->file_end = s->zero_start = -EINVAL; > > Shouldn’t we clear these variables whenever !can_write_resize(perm), not > just if also s->data_end >= 0? Yes, will fix. > >> + } >> + >> + return 0; >> +} > > Max > Thanks! -- Best regards, Vladimir
diff --git a/docs/system/qemu-block-drivers.rst.inc b/docs/system/qemu-block-drivers.rst.inc index b052a6d14e..60a064b232 100644 --- a/docs/system/qemu-block-drivers.rst.inc +++ b/docs/system/qemu-block-drivers.rst.inc @@ -952,3 +952,29 @@ on host and see if there are locks held by the QEMU process on the image file. More than one byte could be locked by the QEMU instance, each byte of which reflects a particular permission that is acquired or protected by the running block driver. + +Filter drivers +~~~~~~~~~~~~~~ + +QEMU supports several filter drivers, which don't store any data, but perform +some additional tasks, hooking io requests. + +.. program:: filter-drivers +.. option:: preallocate + + The preallocate filter driver is intended to be inserted between format + and protocol nodes and preallocates some additional space + (expanding the protocol file) when writing past the file’s end. This can be + useful for file-systems with slow allocation. + + Supported options: + + .. program:: preallocate + .. option:: prealloc-align + + On preallocation, align the file length to this value (in bytes), default 1M. + + .. program:: preallocate + .. option:: prealloc-size + + How much to preallocate (in bytes), default 128M. diff --git a/qapi/block-core.json b/qapi/block-core.json index 2d94873ca0..c8030e19b4 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2819,7 +2819,7 @@ 'cloop', 'compress', 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels', - 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd', + 'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd', { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' }, 'sheepdog', 'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] } @@ -3088,6 +3088,23 @@ 'data': { 'aes': 'QCryptoBlockOptionsQCow', 'luks': 'QCryptoBlockOptionsLUKS'} } +## +# @BlockdevOptionsPreallocate: +# +# Filter driver intended to be inserted between format and protocol node +# and do preallocation in protocol node on write. +# +# @prealloc-align: on preallocation, align file length to this number, +# default 1048576 (1M) +# +# @prealloc-size: how much to preallocate, default 134217728 (128M) +# +# Since: 5.2 +## +{ 'struct': 'BlockdevOptionsPreallocate', + 'base': 'BlockdevOptionsGenericFormat', + 'data': { '*prealloc-align': 'int', '*prealloc-size': 'int' } } + ## # @BlockdevOptionsQcow2: # @@ -3993,6 +4010,7 @@ 'null-co': 'BlockdevOptionsNull', 'nvme': 'BlockdevOptionsNVMe', 'parallels': 'BlockdevOptionsGenericFormat', + 'preallocate':'BlockdevOptionsPreallocate', 'qcow2': 'BlockdevOptionsQcow2', 'qcow': 'BlockdevOptionsQcow', 'qed': 'BlockdevOptionsGenericCOWFormat', diff --git a/block/preallocate.c b/block/preallocate.c new file mode 100644 index 0000000000..6510ad0149 --- /dev/null +++ b/block/preallocate.c @@ -0,0 +1,556 @@ +/* + * preallocate filter driver + * + * The driver performs preallocate operation: it is injected above + * some node, and before each write over EOF it does additional preallocating + * write-zeroes request. + * + * Copyright (c) 2020 Virtuozzo International GmbH. + * + * Author: + * Sementsov-Ogievskiy Vladimir <vsementsov@virtuozzo.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" + +#include "qapi/error.h" +#include "qemu/module.h" +#include "qemu/option.h" +#include "qemu/units.h" +#include "block/block_int.h" + + +typedef struct PreallocateOpts { + int64_t prealloc_size; + int64_t prealloc_align; +} PreallocateOpts; + +typedef struct BDRVPreallocateState { + PreallocateOpts opts; + + /* + * Track real data end, to crop preallocation on close. If < 0 the status is + * unknown. + * + * @data_end is a maximum of file size on open (or when we get write/resize + * permissions) and all write request ends after it. So it's safe to + * truncate to data_end if it is valid. + */ + int64_t data_end; + + /* + * Start of trailing preallocated area which reads as zero. May be smaller + * than data_end, if user does over-EOF write zero operation. If < 0 the + * status is unknown. + * + * If both @zero_start and @file_end are valid, the region + * [@zero_start, @file_end) is known to be preallocated zeroes. If @file_end + * is not valid, @zero_start doesn't make much sense. + */ + int64_t zero_start; + + /* + * Real end of file. Actually the cache for bdrv_getlength(bs->file->bs), + * to avoid extra lseek() calls on each write operation. If < 0 the status + * is unknown. + */ + int64_t file_end; + + /* + * All three states @data_end, @zero_start and @file_end are guaranteed to + * be invalid (< 0) when we don't have both exclusive BLK_PERM_RESIZE and + * BLK_PERM_WRITE permissions on file child. + */ +} BDRVPreallocateState; + +#define PREALLOCATE_OPT_PREALLOC_ALIGN "prealloc-align" +#define PREALLOCATE_OPT_PREALLOC_SIZE "prealloc-size" +static QemuOptsList runtime_opts = { + .name = "preallocate", + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), + .desc = { + { + .name = PREALLOCATE_OPT_PREALLOC_ALIGN, + .type = QEMU_OPT_SIZE, + .help = "on preallocation, align file length to this number, " + "default 1M", + }, + { + .name = PREALLOCATE_OPT_PREALLOC_SIZE, + .type = QEMU_OPT_SIZE, + .help = "how much to preallocate, default 128M", + }, + { /* end of list */ } + }, +}; + +static bool preallocate_absorb_opts(PreallocateOpts *dest, QDict *options, + BlockDriverState *child_bs, Error **errp) +{ + QemuOpts *opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); + + if (!qemu_opts_absorb_qdict(opts, options, errp)) { + return false; + } + + dest->prealloc_align = + qemu_opt_get_size(opts, PREALLOCATE_OPT_PREALLOC_ALIGN, 1 * MiB); + dest->prealloc_size = + qemu_opt_get_size(opts, PREALLOCATE_OPT_PREALLOC_SIZE, 128 * MiB); + + qemu_opts_del(opts); + + if (!QEMU_IS_ALIGNED(dest->prealloc_align, BDRV_SECTOR_SIZE)) { + error_setg(errp, "prealloc-align parameter of preallocate filter " + "is not aligned to %llu", BDRV_SECTOR_SIZE); + return false; + } + + if (!QEMU_IS_ALIGNED(dest->prealloc_align, + child_bs->bl.request_alignment)) { + error_setg(errp, "prealloc-align parameter of preallocate filter " + "is not aligned to underlying node request alignment " + "(%" PRIi32 ")", child_bs->bl.request_alignment); + return false; + } + + return true; +} + +static int preallocate_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) +{ + BDRVPreallocateState *s = bs->opaque; + + /* + * s->data_end and friends should be initialized on permission update. + * For this to work, mark them invalid. + */ + s->file_end = s->zero_start = s->data_end = -EINVAL; + + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds, + BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, + false, errp); + if (!bs->file) { + return -EINVAL; + } + + if (!preallocate_absorb_opts(&s->opts, options, bs->file->bs, errp)) { + return -EINVAL; + } + + bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED | + (BDRV_REQ_FUA & bs->file->bs->supported_write_flags); + + bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED | + ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) & + bs->file->bs->supported_zero_flags); + + return 0; +} + +static void preallocate_close(BlockDriverState *bs) +{ + int ret; + BDRVPreallocateState *s = bs->opaque; + + if (s->data_end < 0) { + return; + } + + if (s->file_end < 0) { + s->file_end = bdrv_getlength(bs->file->bs); + if (s->file_end < 0) { + return; + } + } + + if (s->data_end < s->file_end) { + ret = bdrv_truncate(bs->file, s->data_end, true, PREALLOC_MODE_OFF, 0, + NULL); + s->file_end = ret < 0 ? ret : s->data_end; + } +} + + +/* + * Handle reopen. + * + * We must implement reopen handlers, otherwise reopen just don't work. Handle + * new options and don't care about preallocation state, as it is handled in + * set/check permission handlers. + */ + +static int preallocate_reopen_prepare(BDRVReopenState *reopen_state, + BlockReopenQueue *queue, Error **errp) +{ + PreallocateOpts *opts = g_new0(PreallocateOpts, 1); + + if (!preallocate_absorb_opts(opts, reopen_state->options, + reopen_state->bs->file->bs, errp)) { + g_free(opts); + return -EINVAL; + } + + reopen_state->opaque = opts; + + return 0; +} + +static void preallocate_reopen_commit(BDRVReopenState *state) +{ + BDRVPreallocateState *s = state->bs->opaque; + + s->opts = *(PreallocateOpts *)state->opaque; + + g_free(state->opaque); + state->opaque = NULL; +} + +static void preallocate_reopen_abort(BDRVReopenState *state) +{ + g_free(state->opaque); + state->opaque = NULL; +} + +static coroutine_fn int preallocate_co_preadv_part( + BlockDriverState *bs, uint64_t offset, uint64_t bytes, + QEMUIOVector *qiov, size_t qiov_offset, int flags) +{ + return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset, + flags); +} + +static int coroutine_fn preallocate_co_pdiscard(BlockDriverState *bs, + int64_t offset, int bytes) +{ + return bdrv_co_pdiscard(bs->file, offset, bytes); +} + +static bool can_write_resize(uint64_t perm) +{ + return (perm & BLK_PERM_WRITE) && (perm & BLK_PERM_RESIZE); +} + +static bool has_prealloc_perms(BlockDriverState *bs) +{ + BDRVPreallocateState *s = bs->opaque; + + if (can_write_resize(bs->file->perm)) { + assert(!(bs->file->shared_perm & BLK_PERM_WRITE)); + assert(!(bs->file->shared_perm & BLK_PERM_RESIZE)); + return true; + } + + assert(s->data_end < 0); + assert(s->zero_start < 0); + assert(s->file_end < 0); + return false; +} + +/* + * Call on each write. Returns true if @want_merge_zero is true and the region + * [offset, offset + bytes) is zeroed (as a result of this call or earlier + * preallocation). + * + * want_merge_zero is used to merge write-zero request with preallocation in + * one bdrv_co_pwrite_zeroes() call. + */ +static bool coroutine_fn handle_write(BlockDriverState *bs, int64_t offset, + int64_t bytes, bool want_merge_zero) +{ + BDRVPreallocateState *s = bs->opaque; + int64_t end = offset + bytes; + int64_t prealloc_start, prealloc_end; + int ret; + + if (!has_prealloc_perms(bs)) { + /* We don't have state neither should try to recover it */ + return false; + } + + if (s->data_end < 0) { + s->data_end = bdrv_getlength(bs->file->bs); + if (s->data_end < 0) { + return false; + } + + if (s->file_end < 0) { + s->file_end = s->data_end; + } + } + + if (end <= s->data_end) { + return false; + } + + /* We have valid s->data_end, and request writes beyond it. */ + + s->data_end = end; + if (s->zero_start < 0 || !want_merge_zero) { + s->zero_start = end; + } + + if (s->file_end < 0) { + s->file_end = bdrv_getlength(bs->file->bs); + if (s->file_end < 0) { + return false; + } + } + + /* Now s->data_end, s->zero_start and s->file_end are valid. */ + + if (end <= s->file_end) { + /* No preallocation needed. */ + return want_merge_zero && offset >= s->zero_start; + } + + /* Now we want new preallocation, as request writes beyond s->data_end. */ + + prealloc_start = want_merge_zero ? MIN(offset, s->file_end) : s->file_end; + prealloc_end = QEMU_ALIGN_UP(offset + bytes + s->opts.prealloc_size, + s->opts.prealloc_align); + s->file_end = end; + + ret = bdrv_co_pwrite_zeroes( + bs->file, prealloc_start, prealloc_end - prealloc_start, + BDRV_REQ_NO_FALLBACK | BDRV_REQ_SERIALISING | BDRV_REQ_NO_WAIT); + if (ret < 0) { + s->file_end = ret; + return false; + } + + s->file_end = prealloc_end; + return want_merge_zero; +} + +static int coroutine_fn preallocate_co_pwrite_zeroes(BlockDriverState *bs, + int64_t offset, int bytes, BdrvRequestFlags flags) +{ + bool want_merge_zero = + (flags & (BDRV_REQ_ZERO_WRITE | BDRV_REQ_NO_FALLBACK)) == flags; + if (handle_write(bs, offset, bytes, want_merge_zero)) { + return 0; + } + + return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags); +} + +static coroutine_fn int preallocate_co_pwritev_part(BlockDriverState *bs, + uint64_t offset, + uint64_t bytes, + QEMUIOVector *qiov, + size_t qiov_offset, + int flags) +{ + handle_write(bs, offset, bytes, false); + + return bdrv_co_pwritev_part(bs->file, offset, bytes, qiov, qiov_offset, + flags); +} + +static int coroutine_fn +preallocate_co_truncate(BlockDriverState *bs, int64_t offset, + bool exact, PreallocMode prealloc, + BdrvRequestFlags flags, Error **errp) +{ + ERRP_GUARD(); + BDRVPreallocateState *s = bs->opaque; + int ret; + + if (s->data_end >= 0 && offset > s->data_end) { + if (s->file_end < 0) { + s->file_end = bdrv_getlength(bs->file->bs); + if (s->file_end < 0) { + error_setg(errp, "failed to get file length"); + return s->file_end; + } + } + + if (prealloc == PREALLOC_MODE_FALLOC) { + /* + * If offset <= s->file_end, the task is already done, just + * update s->file_end, to move part of "filter preallocation" + * to "preallocation requested by user". + * Otherwise just proceed to preallocate missing part. + */ + if (offset <= s->file_end) { + s->data_end = offset; + return 0; + } + } else { + /* + * We have to drop our preallocation, to + * - avoid "Cannot use preallocation for shrinking files" in + * case of offset < file_end + * - give PREALLOC_MODE_OFF a chance to keep small disk + * usage + * - give PREALLOC_MODE_FULL a chance to actually write the + * whole region as user expects + */ + if (s->file_end > s->data_end) { + ret = bdrv_co_truncate(bs->file, s->data_end, true, + PREALLOC_MODE_OFF, 0, errp); + if (ret < 0) { + s->file_end = ret; + error_prepend(errp, "preallocate-filter: failed to drop " + "write-zero preallocation: "); + return ret; + } + s->file_end = s->data_end; + } + } + + s->data_end = offset; + } + + ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, flags, errp); + if (ret < 0) { + s->file_end = s->zero_start = s->data_end = ret; + return ret; + } + + if (has_prealloc_perms(bs)) { + s->file_end = s->zero_start = s->data_end = offset; + } + return 0; +} + +static int coroutine_fn preallocate_co_flush(BlockDriverState *bs) +{ + return bdrv_co_flush(bs->file->bs); +} + +static int64_t preallocate_getlength(BlockDriverState *bs) +{ + int64_t ret; + BDRVPreallocateState *s = bs->opaque; + + if (s->data_end >= 0) { + return s->data_end; + } + + ret = bdrv_getlength(bs->file->bs); + + if (has_prealloc_perms(bs)) { + s->file_end = s->zero_start = s->data_end = ret; + } + + return ret; +} + +static int preallocate_check_perm(BlockDriverState *bs, + uint64_t perm, uint64_t shared, Error **errp) +{ + BDRVPreallocateState *s = bs->opaque; + + if (s->data_end >= 0 && !can_write_resize(perm)) { + /* + * Loose permissions. + * We should truncate in check_perm, as in set_perm bs->file->perm will + * be already changed, and we should not violate it. + */ + if (s->file_end < 0) { + s->file_end = bdrv_getlength(bs->file->bs); + if (s->file_end < 0) { + error_setg(errp, "Failed to get file length"); + return s->file_end; + } + } + + if (s->data_end < s->file_end) { + int ret = bdrv_truncate(bs->file, s->data_end, true, + PREALLOC_MODE_OFF, 0, NULL); + if (ret < 0) { + error_setg(errp, "Failed to drop preallocation"); + s->file_end = ret; + return ret; + } + } + /* + * We will drop our permissions, as well as allow shared + * permissions, anyone will be able to change the child, so mark + * all states invalid. We'll regain control if get good permissions + * back. + */ + s->data_end = s->file_end = s->zero_start = -EINVAL; + } + + return 0; +} + +static void preallocate_set_perm(BlockDriverState *bs, + uint64_t perm, uint64_t shared) +{ + BDRVPreallocateState *s = bs->opaque; + + if (can_write_resize(perm) && s->data_end < 0) { + s->data_end = s->file_end = s->zero_start = + bdrv_getlength(bs->file->bs); + } +} + +static void preallocate_child_perm(BlockDriverState *bs, BdrvChild *c, + BdrvChildRole role, BlockReopenQueue *reopen_queue, + uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) +{ + bdrv_default_perms(bs, c, role, reopen_queue, perm, shared, nperm, nshared); + + if (can_write_resize(perm)) { + /* This should come by default, but let's enforce: */ + *nperm |= BLK_PERM_WRITE | BLK_PERM_RESIZE; + + /* + * Don't share, to keep our states s->file_end, s->data_end and + * s->zero_start valid. + */ + *nshared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE); + } +} + +BlockDriver bdrv_preallocate_filter = { + .format_name = "preallocate", + .instance_size = sizeof(BDRVPreallocateState), + + .bdrv_getlength = preallocate_getlength, + .bdrv_open = preallocate_open, + .bdrv_close = preallocate_close, + + .bdrv_reopen_prepare = preallocate_reopen_prepare, + .bdrv_reopen_commit = preallocate_reopen_commit, + .bdrv_reopen_abort = preallocate_reopen_abort, + + .bdrv_co_preadv_part = preallocate_co_preadv_part, + .bdrv_co_pwritev_part = preallocate_co_pwritev_part, + .bdrv_co_pwrite_zeroes = preallocate_co_pwrite_zeroes, + .bdrv_co_pdiscard = preallocate_co_pdiscard, + .bdrv_co_flush = preallocate_co_flush, + .bdrv_co_truncate = preallocate_co_truncate, + + .bdrv_check_perm = preallocate_check_perm, + .bdrv_set_perm = preallocate_set_perm, + .bdrv_child_perm = preallocate_child_perm, + + .has_variable_length = true, + .is_filter = true, +}; + +static void bdrv_preallocate_init(void) +{ + bdrv_register(&bdrv_preallocate_filter); +} + +block_init(bdrv_preallocate_init); diff --git a/block/meson.build b/block/meson.build index a3e56b7cd1..367ec7a525 100644 --- a/block/meson.build +++ b/block/meson.build @@ -13,6 +13,7 @@ block_ss.add(files( 'block-copy.c', 'commit.c', 'copy-on-read.c', + 'preallocate.c', 'create.c', 'crypto.c', 'dirty-bitmap.c',
It's intended to be inserted between format and protocol nodes to preallocate additional space (expanding protocol file) on writes crossing EOF. It improves performance for file-systems with slow allocation. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- docs/system/qemu-block-drivers.rst.inc | 26 ++ qapi/block-core.json | 20 +- block/preallocate.c | 556 +++++++++++++++++++++++++ block/meson.build | 1 + 4 files changed, 602 insertions(+), 1 deletion(-) create mode 100644 block/preallocate.c