[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 08/33] block/backup: drop support for copy_range
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v2 08/33] block/backup: drop support for copy_range |
Date: |
Fri, 28 May 2021 18:29:29 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.2 |
20.05.2021 17:21, Vladimir Sementsov-Ogievskiy wrote:
copy_range is not a default behavior since 6a30f663d4c0b3c, and it's
now available only though x-perf experimantal argument, so it's OK to
drop it.
Even when backup is used to copy disk to same filesystem, and
filesystem supports zero-copy copy_range, copy_range is probably not
what we want for backup: backup has good property of making a copy of
active disk, with no impact to active disk itself (unlike creating a
snapshot). And if copy_range instead of copying data adds fs-level
references, and on next guest write COW operation occurs, it's seems
most possible, that new block will be allocated for original vm disk,
not for backup disk. Thus, fragmentation of original disk will
increase.
We can simply add support back on demand. Now we want to publish
copy-before-write filter, and instead of thinking how to pass
use-copy-range argument to block-copy (create x-block-copy parameter
for new public filter driver, or may be set it by hand after filter
node creation?), instead of this let's just drop copy-range support in
backup for now.
After this patch copy-range support in block-copy becomes unused. Let's
keep it for a while, it won't hurt:
1. If there would be request for supporting copy_range in backup
(and/or in a new public copy-before-write filter), it will be easy
to satisfy it.
2. Probably, qemu-img convert will reuse block-copy, and qemu-img has
option to enable copy-range. qemu-img convert is not a backup, and
copy_range may be more reasonable for some cases in context of
qemu-img convert.
Actually, I know one case, where copy_range for backup job may be reasonable:
Using backup in push-backup with fleecing scheme in
[PATCH 0/6] push backup with fleecing
Of-course, no real sense in using push-backup-with-fleecing scheme with both
temp image and final backup target being on the same file system (no benefit of
fleecing, we can use simple backup without temporary image).
But we absolutely don't care about fragmentation of temp disk.
Still, it doesn't make sense, as temp-image and real-backup-target should not
be on same file-system..
Could it be some distributed filesystem, where it still make sense to call
copy_range? Theoretically could.
Another thought: I'm going also to implement RAM-cache driver, to optimize
push-backup-with-fleecing scheme. I'll need a way to copy data from RAM-cache
node to final-target. I can implement copy_range for RAM-cache, and this will
allow to not create extra buffer, but use the buffer that is already allocated
and own by RAM-cache.. Still, this behavior is obviously good, it should work
automatically, no reason to make it optional..
Hmm, so what should be summarized:
- Actually, block-copy does copy_range. So, probably it's good to change the
copy_range() function in qemu to fallback to read+write..
And about copy_range itself, what we want:
1. We want to control does it influence fragmentation of source disk. When
copying from temporary image we don't care. But when source of block-copy is
active disk in we do care to not influence how original disk lay in filesystem.
Probably, we even want an option for copy_range() syscall to control this thing.
2. We want to be efficient with copy_size, ie size of chunks to copy. We even
have existing issue in block-copy: write-zero is limited to
BLOCK_COPY_MAX_BUFFER which is obviously inefficient.
For copy-size we should have some good defaults or automatic detection logic..
For copy_range fragmentation..
If we have some internal copy_range-like optimizations like zero-copy from
RAM-cache node, or maybe copy compressed data from one qcow2 node to another
without decompression, it should be done anyway, it shouldn't be set by user
option. Still, for file-posix, we don't know, does underlying filesystem
copy_range() implementation lead to fragmentation or not. And we don't know is
user OK with it or not. So we need an option.. So, it's probably better to keep
x-perm.copy-range for now, until we don't have a good idea on interface.
--
Best regards,
Vladimir
- [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field, (continued)
- [PATCH v2 05/33] qdev-properties: PropertyInfo: add realized_set_allowed field, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 04/33] block: introduce blk_replace_bs, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 06/33] qdev: allow setting drive property for realized device, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 10/33] block/backup: move cluster size calculation to block-copy, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 07/33] block: rename backup-top to copy-before-write, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 09/33] block-copy: always set BDRV_REQ_SERIALISING flag, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 08/33] block/backup: drop support for copy_range, Vladimir Sementsov-Ogievskiy, 2021/05/20
- Re: [PATCH v2 08/33] block/backup: drop support for copy_range,
Vladimir Sementsov-Ogievskiy <=
- [PATCH v2 11/33] block/copy-before-write: relax permission requirements when no parents, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 12/33] block/copy-before-write: drop extra bdrv_unref on failure path, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 13/33] block/copy-before-write: use file child instead of backing, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args(), Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 14/33] block/copy-before-write: bdrv_cbw_append(): replace child at last, Vladimir Sementsov-Ogievskiy, 2021/05/20
- [PATCH v2 15/33] block/copy-before-write: introduce cbw_init(), Vladimir Sementsov-Ogievskiy, 2021/05/20