* [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout
@ 2022-04-06 18:07 Vladimir Sementsov-Ogievskiy
2022-04-06 18:07 ` [PATCH v3 1/7] block/copy-before-write: refactor option parsing Vladimir Sementsov-Ogievskiy
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:07 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
Hi all!
v3:
01: refactor options parsing
02: - wording, grammar
- early return from cbw_do_copy_before_write() when snapshot_error is set
- drop premature optimization around waiting for in-flight requests when
we set snapshot_error
03: Hanna's r-b
05: - add callback to block_copy() interface
- add forgotten g_free(call_state)
- do cancel block-copy call on timeout
06: - wording
- increase in_flight for block_copy(), to handle timed-out block_copy()
on drain
07: - less strict limit for bps-write, as now we honestly wait for cancelled
block-copy call on final drain
Here are two new options for copy-before-write filter:
on-cbw-error allows to alter the behavior on copy-before-write operation
failure: not break guest write but break the snapshot (and therefore
backup process)
cbw-timeout allows to limit cbw operation by some timeout.
So, for example, using cbw-timeout=60 and on-cbw-error=break-snapshot
you can be sure that guest write will not stuck for more than 60
seconds and will never fail due to backup problems.
Based-on: <20220406153202.332186-1-vsementsov@openvz.org>
( [PATCH for-7.1 0/2] throttle-groups: use QEMU_CLOCK_REALTIME )
Vladimir Sementsov-Ogievskiy (7):
block/copy-before-write: refactor option parsing
block/copy-before-write: add on-cbw-error open parameter
iotests: add copy-before-write: on-cbw-error tests
util: add qemu-co-timeout
block/block-copy: block_copy(): add timeout_ns parameter
block/copy-before-write: implement cbw-timeout option
iotests: copy-before-write: add cases for cbw-timeout option
qapi/block-core.json | 31 ++-
include/block/block-copy.h | 4 +-
include/qemu/coroutine.h | 13 ++
block/block-copy.c | 33 ++-
block/copy-before-write.c | 110 +++++++---
util/qemu-co-timeout.c | 89 ++++++++
tests/qemu-iotests/tests/copy-before-write | 206 ++++++++++++++++++
.../qemu-iotests/tests/copy-before-write.out | 5 +
util/meson.build | 1 +
9 files changed, 453 insertions(+), 39 deletions(-)
create mode 100644 util/qemu-co-timeout.c
create mode 100755 tests/qemu-iotests/tests/copy-before-write
create mode 100644 tests/qemu-iotests/tests/copy-before-write.out
--
2.35.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/7] block/copy-before-write: refactor option parsing
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
@ 2022-04-06 18:07 ` Vladimir Sementsov-Ogievskiy
2022-04-07 7:08 ` Hanna Reitz
2022-04-06 18:07 ` [PATCH v3 2/7] block/copy-before-write: add on-cbw-error open parameter Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:07 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
We are going to add one more option of enum type. Let's refactor option
parsing so that we can simply work with BlockdevOptionsCbw object.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
block/copy-before-write.c | 55 ++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index a8a06fdc09..6877ff893a 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -24,6 +24,7 @@
*/
#include "qemu/osdep.h"
+#include "qapi/qmp/qjson.h"
#include "sysemu/block-backend.h"
#include "qemu/cutils.h"
@@ -328,46 +329,34 @@ static void cbw_child_perm(BlockDriverState *bs, BdrvChild *c,
}
}
-static bool cbw_parse_bitmap_option(QDict *options, BdrvDirtyBitmap **bitmap,
- Error **errp)
+static BlockdevOptions *cbw_parse_options(QDict *options, Error **errp)
{
- QDict *bitmap_qdict = NULL;
- BlockDirtyBitmap *bmp_param = NULL;
+ BlockdevOptions *opts = NULL;
Visitor *v = NULL;
- bool ret = false;
- *bitmap = NULL;
+ qdict_put_str(options, "driver", "copy-before-write");
- qdict_extract_subqdict(options, &bitmap_qdict, "bitmap.");
- if (!qdict_size(bitmap_qdict)) {
- ret = true;
- goto out;
- }
-
- v = qobject_input_visitor_new_flat_confused(bitmap_qdict, errp);
+ v = qobject_input_visitor_new_flat_confused(options, errp);
if (!v) {
goto out;
}
- visit_type_BlockDirtyBitmap(v, NULL, &bmp_param, errp);
- if (!bmp_param) {
+ visit_type_BlockdevOptions(v, NULL, &opts, errp);
+ if (!opts) {
goto out;
}
- *bitmap = block_dirty_bitmap_lookup(bmp_param->node, bmp_param->name, NULL,
- errp);
- if (!*bitmap) {
- goto out;
- }
-
- ret = true;
+ /*
+ * Delete options which we are going to parse through BlockdevOptions
+ * object for original options.
+ */
+ qdict_extract_subqdict(options, NULL, "bitmap");
out:
- qapi_free_BlockDirtyBitmap(bmp_param);
visit_free(v);
- qobject_unref(bitmap_qdict);
+ qdict_del(options, "driver");
- return ret;
+ return opts;
}
static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
@@ -376,6 +365,14 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
BDRVCopyBeforeWriteState *s = bs->opaque;
BdrvDirtyBitmap *bitmap = NULL;
int64_t cluster_size;
+ g_autoptr(BlockdevOptions) full_opts = NULL;
+ BlockdevOptionsCbw *opts;
+
+ full_opts = cbw_parse_options(options, errp);
+ if (!full_opts) {
+ return -EINVAL;
+ }
+ opts = &full_opts->u.copy_before_write;
bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
@@ -390,8 +387,12 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
- if (!cbw_parse_bitmap_option(options, &bitmap, errp)) {
- return -EINVAL;
+ if (opts->has_bitmap) {
+ bitmap = block_dirty_bitmap_lookup(opts->bitmap->node,
+ opts->bitmap->name, NULL, errp);
+ if (!bitmap) {
+ return -EINVAL;
+ }
}
bs->total_sectors = bs->file->bs->total_sectors;
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/7] block/copy-before-write: add on-cbw-error open parameter
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
2022-04-06 18:07 ` [PATCH v3 1/7] block/copy-before-write: refactor option parsing Vladimir Sementsov-Ogievskiy
@ 2022-04-06 18:07 ` Vladimir Sementsov-Ogievskiy
2022-04-07 8:22 ` Hanna Reitz
2022-04-06 18:07 ` [PATCH v3 3/7] iotests: add copy-before-write: on-cbw-error tests Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:07 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
Currently, behavior on copy-before-write operation failure is simple:
report error to the guest.
Let's implement alternative behavior: break the whole copy-before-write
process (and corresponding backup job or NBD client) but keep guest
working. It's needed if we consider guest stability as more important.
The realisation is simple: on copy-before-write failure we set
s->snapshot_ret and continue guest operations. s->snapshot_ret being
set will lead to all further snapshot API requests. Note that all
in-flight snapshot-API requests may still success: we do wait for them
on BREAK_SNAPSHOT-failure path in cbw_do_copy_before_write().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
qapi/block-core.json | 25 ++++++++++++++++++++++++-
block/copy-before-write.c | 32 ++++++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index beeb91952a..085f1666af 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4163,6 +4163,25 @@
'base': 'BlockdevOptionsGenericFormat',
'data': { '*bottom': 'str' } }
+##
+# @OnCbwError:
+#
+# An enumeration of possible behaviors for copy-before-write operation
+# failures.
+#
+# @break-guest-write: report the error to the guest. This way, the guest
+# will not be able to overwrite areas that cannot be
+# backed up, so the backup process remains valid.
+#
+# @break-snapshot: continue guest write. Doing so will make the provided
+# snapshot state invalid and any backup or export
+# process based on it will finally fail.
+#
+# Since: 7.1
+##
+{ 'enum': 'OnCbwError',
+ 'data': [ 'break-guest-write', 'break-snapshot' ] }
+
##
# @BlockdevOptionsCbw:
#
@@ -4184,11 +4203,15 @@
# modifications (or removing) of specified bitmap doesn't
# influence the filter. (Since 7.0)
#
+# @on-cbw-error: Behavior on failure of copy-before-write operation.
+# Default is @break-guest-write. (Since 7.0)
+#
# Since: 6.2
##
{ 'struct': 'BlockdevOptionsCbw',
'base': 'BlockdevOptionsGenericFormat',
- 'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap' } }
+ 'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap',
+ '*on-cbw-error': 'OnCbwError' } }
##
# @BlockdevOptions:
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index 6877ff893a..ffb05d22f8 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -41,6 +41,7 @@
typedef struct BDRVCopyBeforeWriteState {
BlockCopyState *bcs;
BdrvChild *target;
+ OnCbwError on_cbw_error;
/*
* @lock: protects access to @access_bitmap, @done_bitmap and
@@ -65,6 +66,14 @@ typedef struct BDRVCopyBeforeWriteState {
* node. These areas must not be rewritten by guest.
*/
BlockReqList frozen_read_reqs;
+
+ /*
+ * @snapshot_error is normally zero. But on first copy-before-write failure
+ * when @on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT, @snapshot_error takes
+ * value of this error (<0). After that all in-flight and further
+ * snapshot-API requests will fail with that error.
+ */
+ int snapshot_error;
} BDRVCopyBeforeWriteState;
static coroutine_fn int cbw_co_preadv(
@@ -95,16 +104,27 @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
return 0;
}
+ if (s->snapshot_error) {
+ return 0;
+ }
+
off = QEMU_ALIGN_DOWN(offset, cluster_size);
end = QEMU_ALIGN_UP(offset + bytes, cluster_size);
ret = block_copy(s->bcs, off, end - off, true);
- if (ret < 0) {
+ if (ret < 0 && s->on_cbw_error == ON_CBW_ERROR_BREAK_GUEST_WRITE) {
return ret;
}
WITH_QEMU_LOCK_GUARD(&s->lock) {
- bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off);
+ if (ret < 0) {
+ assert(s->on_cbw_error == ON_CBW_ERROR_BREAK_SNAPSHOT);
+ if (!s->snapshot_error) {
+ s->snapshot_error = ret;
+ }
+ } else {
+ bdrv_set_dirty_bitmap(s->done_bitmap, off, end - off);
+ }
reqlist_wait_all(&s->frozen_read_reqs, off, end - off, &s->lock);
}
@@ -176,6 +196,11 @@ static BlockReq *cbw_snapshot_read_lock(BlockDriverState *bs,
QEMU_LOCK_GUARD(&s->lock);
+ if (s->snapshot_error) {
+ g_free(req);
+ return NULL;
+ }
+
if (bdrv_dirty_bitmap_next_zero(s->access_bitmap, offset, bytes) != -1) {
g_free(req);
return NULL;
@@ -351,6 +376,7 @@ static BlockdevOptions *cbw_parse_options(QDict *options, Error **errp)
* object for original options.
*/
qdict_extract_subqdict(options, NULL, "bitmap");
+ qdict_del(options, "on-cbw-error");
out:
visit_free(v);
@@ -394,6 +420,8 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
}
+ s->on_cbw_error = opts->has_on_cbw_error ? opts->on_cbw_error :
+ ON_CBW_ERROR_BREAK_GUEST_WRITE;
bs->total_sectors = bs->file->bs->total_sectors;
bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 3/7] iotests: add copy-before-write: on-cbw-error tests
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
2022-04-06 18:07 ` [PATCH v3 1/7] block/copy-before-write: refactor option parsing Vladimir Sementsov-Ogievskiy
2022-04-06 18:07 ` [PATCH v3 2/7] block/copy-before-write: add on-cbw-error open parameter Vladimir Sementsov-Ogievskiy
@ 2022-04-06 18:07 ` Vladimir Sementsov-Ogievskiy
2022-04-06 18:07 ` [PATCH v3 4/7] util: add qemu-co-timeout Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:07 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
Add tests for new option of copy-before-write filter: on-cbw-error.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
---
tests/qemu-iotests/tests/copy-before-write | 128 ++++++++++++++++++
.../qemu-iotests/tests/copy-before-write.out | 5 +
2 files changed, 133 insertions(+)
create mode 100755 tests/qemu-iotests/tests/copy-before-write
create mode 100644 tests/qemu-iotests/tests/copy-before-write.out
diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write
new file mode 100755
index 0000000000..a32608f597
--- /dev/null
+++ b/tests/qemu-iotests/tests/copy-before-write
@@ -0,0 +1,128 @@
+#!/usr/bin/env python3
+# group: auto backup
+#
+# Copyright (c) 2022 Virtuozzo International GmbH
+#
+# 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/>.
+#
+
+import os
+import re
+
+import iotests
+from iotests import qemu_img_create, qemu_io
+
+
+temp_img = os.path.join(iotests.test_dir, 'temp')
+source_img = os.path.join(iotests.test_dir, 'source')
+size = '1M'
+
+
+class TestCbwError(iotests.QMPTestCase):
+ def tearDown(self):
+ self.vm.shutdown()
+ os.remove(temp_img)
+ os.remove(source_img)
+
+ def setUp(self):
+ qemu_img_create('-f', iotests.imgfmt, source_img, size)
+ qemu_img_create('-f', iotests.imgfmt, temp_img, size)
+ qemu_io('-c', 'write 0 1M', source_img)
+
+ self.vm = iotests.VM()
+ self.vm.launch()
+
+ def do_cbw_error(self, on_cbw_error):
+ result = self.vm.qmp('blockdev-add', {
+ 'node-name': 'cbw',
+ 'driver': 'copy-before-write',
+ 'on-cbw-error': on_cbw_error,
+ 'file': {
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': source_img,
+ }
+ },
+ 'target': {
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'blkdebug',
+ 'image': {
+ 'driver': 'file',
+ 'filename': temp_img
+ },
+ 'inject-error': [
+ {
+ 'event': 'write_aio',
+ 'errno': 5,
+ 'immediately': False,
+ 'once': True
+ }
+ ]
+ }
+ }
+ })
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm.qmp('blockdev-add', {
+ 'node-name': 'access',
+ 'driver': 'snapshot-access',
+ 'file': 'cbw'
+ })
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm.hmp_qemu_io('cbw', 'write 0 1M')
+ self.assert_qmp(result, 'return', '')
+
+ result = self.vm.hmp_qemu_io('access', 'read 0 1M')
+ self.assert_qmp(result, 'return', '')
+
+ self.vm.shutdown()
+ log = self.vm.get_log()
+ log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log)
+ log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log)
+ log = iotests.filter_qemu_io(log)
+ return log
+
+ def test_break_snapshot_on_cbw_error(self):
+ """break-snapshot behavior:
+ Guest write succeed, but further snapshot-read fails, as snapshot is
+ broken.
+ """
+ log = self.do_cbw_error('break-snapshot')
+
+ self.assertEqual(log, """\
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read failed: Permission denied
+""")
+
+ def test_break_guest_write_on_cbw_error(self):
+ """break-guest-write behavior:
+ Guest write fails, but snapshot-access continues working and further
+ snapshot-read succeeds.
+ """
+ log = self.do_cbw_error('break-guest-write')
+
+ self.assertEqual(log, """\
+write failed: Input/output error
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+""")
+
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2'],
+ supported_protocols=['file'])
diff --git a/tests/qemu-iotests/tests/copy-before-write.out b/tests/qemu-iotests/tests/copy-before-write.out
new file mode 100644
index 0000000000..fbc63e62f8
--- /dev/null
+++ b/tests/qemu-iotests/tests/copy-before-write.out
@@ -0,0 +1,5 @@
+..
+----------------------------------------------------------------------
+Ran 2 tests
+
+OK
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 4/7] util: add qemu-co-timeout
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2022-04-06 18:07 ` [PATCH v3 3/7] iotests: add copy-before-write: on-cbw-error tests Vladimir Sementsov-Ogievskiy
@ 2022-04-06 18:07 ` Vladimir Sementsov-Ogievskiy
2022-04-07 9:20 ` Hanna Reitz
2022-04-06 18:07 ` [PATCH v3 5/7] block/block-copy: block_copy(): add timeout_ns parameter Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:07 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
Add new API, to make a time limited call of the coroutine.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
include/qemu/coroutine.h | 13 ++++++
util/qemu-co-timeout.c | 89 ++++++++++++++++++++++++++++++++++++++++
util/meson.build | 1 +
3 files changed, 103 insertions(+)
create mode 100644 util/qemu-co-timeout.c
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index c828a95ee0..8704b05da8 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -316,6 +316,19 @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
qemu_co_sleep_ns_wakeable(&w, type, ns);
}
+typedef void CleanupFunc(void *opaque);
+/**
+ * Run entry in a coroutine and start timer. Wait for entry to finish or for
+ * timer to elapse, what happen first. If entry finished, return 0, if timer
+ * elapsed earlier, return -ETIMEDOUT.
+ *
+ * Be careful, entry execution is not canceled, user should handle it somehow.
+ * If @clean is provided, it's called after coroutine finish if timeout
+ * happened.
+ */
+int coroutine_fn qemu_co_timeout(CoroutineEntry *entry, void *opaque,
+ uint64_t timeout_ns, CleanupFunc clean);
+
/**
* Wake a coroutine if it is sleeping in qemu_co_sleep_ns. The timer will be
* deleted. @sleep_state must be the variable whose address was given to
diff --git a/util/qemu-co-timeout.c b/util/qemu-co-timeout.c
new file mode 100644
index 0000000000..00cd335649
--- /dev/null
+++ b/util/qemu-co-timeout.c
@@ -0,0 +1,89 @@
+/*
+ * Helper functionality for distributing a fixed total amount of
+ * an abstract resource among multiple coroutines.
+ *
+ * Copyright (c) 2022 Virtuozzo International GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/coroutine.h"
+#include "block/aio.h"
+
+typedef struct QemuCoTimeoutState {
+ CoroutineEntry *entry;
+ void *opaque;
+ QemuCoSleep sleep_state;
+ bool marker;
+ CleanupFunc *clean;
+} QemuCoTimeoutState;
+
+static void coroutine_fn qemu_co_timeout_entry(void *opaque)
+{
+ QemuCoTimeoutState *s = opaque;
+
+ s->entry(s->opaque);
+
+ if (s->marker) {
+ assert(!s->sleep_state.to_wake);
+ /* .marker set by qemu_co_timeout, it have been failed */
+ if (s->clean) {
+ s->clean(s->opaque);
+ }
+ g_free(s);
+ } else {
+ s->marker = true;
+ qemu_co_sleep_wake(&s->sleep_state);
+ }
+}
+
+int coroutine_fn qemu_co_timeout(CoroutineEntry *entry, void *opaque,
+ uint64_t timeout_ns, CleanupFunc clean)
+{
+ QemuCoTimeoutState *s;
+ Coroutine *co;
+
+ if (timeout_ns == 0) {
+ entry(opaque);
+ return 0;
+ }
+
+ s = g_new(QemuCoTimeoutState, 1);
+ *s = (QemuCoTimeoutState) {
+ .entry = entry,
+ .opaque = opaque,
+ .clean = clean
+ };
+
+ co = qemu_coroutine_create(qemu_co_timeout_entry, s);
+
+ aio_co_enter(qemu_get_current_aio_context(), co);
+ qemu_co_sleep_ns_wakeable(&s->sleep_state, QEMU_CLOCK_REALTIME, timeout_ns);
+
+ if (s->marker) {
+ /* .marker set by qemu_co_timeout_entry, success */
+ g_free(s);
+ return 0;
+ }
+
+ /* Don't free s, as we can't cancel qemu_co_timeout_entry execution */
+ s->marker = true;
+ return -ETIMEDOUT;
+}
diff --git a/util/meson.build b/util/meson.build
index f6ee74ad0c..249891db72 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -83,6 +83,7 @@ if have_block
util_ss.add(files('block-helpers.c'))
util_ss.add(files('qemu-coroutine-sleep.c'))
util_ss.add(files('qemu-co-shared-resource.c'))
+ util_ss.add(files('qemu-co-timeout.c'))
util_ss.add(files('thread-pool.c', 'qemu-timer.c'))
util_ss.add(files('readline.c'))
util_ss.add(files('throttle.c'))
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 5/7] block/block-copy: block_copy(): add timeout_ns parameter
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2022-04-06 18:07 ` [PATCH v3 4/7] util: add qemu-co-timeout Vladimir Sementsov-Ogievskiy
@ 2022-04-06 18:07 ` Vladimir Sementsov-Ogievskiy
2022-04-07 9:20 ` Hanna Reitz
2022-04-06 18:08 ` [PATCH v3 6/7] block/copy-before-write: implement cbw-timeout option Vladimir Sementsov-Ogievskiy
2022-04-06 18:08 ` [PATCH v3 7/7] iotests: copy-before-write: add cases for " Vladimir Sementsov-Ogievskiy
6 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:07 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
Add possibility to limit block_copy() call in time. To be used in the
next commit.
As timed-out block_copy() call will continue in background anyway (we
can't immediately cancel IO operation), it's important also give user a
possibility to pass a callback, to do some additional actions on
block-copy call finish.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
include/block/block-copy.h | 4 +++-
block/block-copy.c | 33 ++++++++++++++++++++++++++-------
block/copy-before-write.c | 2 +-
3 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 68bbd344b2..ba0b425d78 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -40,7 +40,9 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
int64_t offset, int64_t *count);
int coroutine_fn block_copy(BlockCopyState *s, int64_t offset, int64_t bytes,
- bool ignore_ratelimit);
+ bool ignore_ratelimit, uint64_t timeout_ns,
+ BlockCopyAsyncCallbackFunc cb,
+ void *cb_opaque);
/*
* Run block-copy in a coroutine, create corresponding BlockCopyCallState
diff --git a/block/block-copy.c b/block/block-copy.c
index ec46775ea5..bb947afdda 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -883,23 +883,42 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state)
return ret;
}
+static void coroutine_fn block_copy_async_co_entry(void *opaque)
+{
+ block_copy_common(opaque);
+}
+
int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes,
- bool ignore_ratelimit)
+ bool ignore_ratelimit, uint64_t timeout_ns,
+ BlockCopyAsyncCallbackFunc cb,
+ void *cb_opaque)
{
- BlockCopyCallState call_state = {
+ int ret;
+ BlockCopyCallState *call_state = g_new(BlockCopyCallState, 1);
+
+ *call_state = (BlockCopyCallState) {
.s = s,
.offset = start,
.bytes = bytes,
.ignore_ratelimit = ignore_ratelimit,
.max_workers = BLOCK_COPY_MAX_WORKERS,
+ .cb = cb,
+ .cb_opaque = cb_opaque,
};
- return block_copy_common(&call_state);
-}
+ ret = qemu_co_timeout(block_copy_async_co_entry, call_state, timeout_ns,
+ g_free);
+ if (ret < 0) {
+ assert(ret == -ETIMEDOUT);
+ block_copy_call_cancel(call_state);
+ /* call_state will be freed by running coroutine. */
+ return ret;
+ }
-static void coroutine_fn block_copy_async_co_entry(void *opaque)
-{
- block_copy_common(opaque);
+ ret = call_state->ret;
+ g_free(call_state);
+
+ return ret;
}
BlockCopyCallState *block_copy_async(BlockCopyState *s,
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index ffb05d22f8..b7487e4ffe 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -111,7 +111,7 @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
off = QEMU_ALIGN_DOWN(offset, cluster_size);
end = QEMU_ALIGN_UP(offset + bytes, cluster_size);
- ret = block_copy(s->bcs, off, end - off, true);
+ ret = block_copy(s->bcs, off, end - off, true, 0, NULL, NULL);
if (ret < 0 && s->on_cbw_error == ON_CBW_ERROR_BREAK_GUEST_WRITE) {
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 6/7] block/copy-before-write: implement cbw-timeout option
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2022-04-06 18:07 ` [PATCH v3 5/7] block/block-copy: block_copy(): add timeout_ns parameter Vladimir Sementsov-Ogievskiy
@ 2022-04-06 18:08 ` Vladimir Sementsov-Ogievskiy
2022-04-07 9:20 ` Hanna Reitz
2022-04-06 18:08 ` [PATCH v3 7/7] iotests: copy-before-write: add cases for " Vladimir Sementsov-Ogievskiy
6 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:08 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
In some scenarios, when copy-before-write operations lasts too long
time, it's better to cancel it.
Most useful would be to use the new option together with
on-cbw-error=break-snapshot: this way if cbw operation takes too long
time we'll just cancel backup process but do not disturb the guest too
much.
Note the tricky point of realization: we keep additional point in
bs->in_fligth during block_copy operation even if it's timed-out.
Background "cancelled" block_copy operations will finish at some point
and will want to access state. We should care to not free the state in
.bdrv_close() earlier.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
qapi/block-core.json | 8 +++++++-
block/copy-before-write.c | 23 ++++++++++++++++++++++-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 085f1666af..e9cd7e88f6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4206,12 +4206,18 @@
# @on-cbw-error: Behavior on failure of copy-before-write operation.
# Default is @break-guest-write. (Since 7.0)
#
+# @cbw-timeout: Zero means no limit. Non-zero sets the timeout in seconds
+# for copy-before-write operation. When a timeout occurs,
+# the respective copy-before-write operation will fail, and
+# the @on-cbw-error parameter will decide how this failure
+# is handled. Default 0. (Since 7.1)
+#
# Since: 6.2
##
{ 'struct': 'BlockdevOptionsCbw',
'base': 'BlockdevOptionsGenericFormat',
'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap',
- '*on-cbw-error': 'OnCbwError' } }
+ '*on-cbw-error': 'OnCbwError', '*cbw-timeout': 'uint32' } }
##
# @BlockdevOptions:
diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index b7487e4ffe..13992d28c2 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -42,6 +42,7 @@ typedef struct BDRVCopyBeforeWriteState {
BlockCopyState *bcs;
BdrvChild *target;
OnCbwError on_cbw_error;
+ uint32_t cbw_timeout_ns;
/*
* @lock: protects access to @access_bitmap, @done_bitmap and
@@ -83,6 +84,14 @@ static coroutine_fn int cbw_co_preadv(
return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
}
+static void block_copy_cb(void *opaque)
+{
+ BlockDriverState *bs = opaque;
+
+ bs->in_flight--;
+ aio_wait_kick();
+}
+
/*
* Do copy-before-write operation.
*
@@ -111,7 +120,16 @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs,
off = QEMU_ALIGN_DOWN(offset, cluster_size);
end = QEMU_ALIGN_UP(offset + bytes, cluster_size);
- ret = block_copy(s->bcs, off, end - off, true, 0, NULL, NULL);
+ /*
+ * Increase in_flight, so that in case of timed-out block-copy, the
+ * remaining background block_copy() request (which can't be immediately
+ * cancelled by timeout) is presented in bs->in_flight. This way we are
+ * sure that on bs close() we'll previously wait for all timed-out but yet
+ * running block_copy calls.
+ */
+ bs->in_flight++;
+ ret = block_copy(s->bcs, off, end - off, true, s->cbw_timeout_ns,
+ block_copy_cb, bs);
if (ret < 0 && s->on_cbw_error == ON_CBW_ERROR_BREAK_GUEST_WRITE) {
return ret;
}
@@ -377,6 +395,7 @@ static BlockdevOptions *cbw_parse_options(QDict *options, Error **errp)
*/
qdict_extract_subqdict(options, NULL, "bitmap");
qdict_del(options, "on-cbw-error");
+ qdict_del(options, "cbw-timeout");
out:
visit_free(v);
@@ -422,6 +441,8 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
}
s->on_cbw_error = opts->has_on_cbw_error ? opts->on_cbw_error :
ON_CBW_ERROR_BREAK_GUEST_WRITE;
+ s->cbw_timeout_ns = opts->has_cbw_timeout ?
+ opts->cbw_timeout * NANOSECONDS_PER_SECOND : 0;
bs->total_sectors = bs->file->bs->total_sectors;
bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 7/7] iotests: copy-before-write: add cases for cbw-timeout option
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2022-04-06 18:08 ` [PATCH v3 6/7] block/copy-before-write: implement cbw-timeout option Vladimir Sementsov-Ogievskiy
@ 2022-04-06 18:08 ` Vladimir Sementsov-Ogievskiy
2022-04-07 9:19 ` Hanna Reitz
6 siblings, 1 reply; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-06 18:08 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, hreitz,
vsementsov, stefanha, eblake
Add two simple test-cases: timeout failure with
break-snapshot-on-cbw-error behavior and similar with
break-guest-write-on-cbw-error behavior.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
tests/qemu-iotests/tests/copy-before-write | 78 +++++++++++++++++++
.../qemu-iotests/tests/copy-before-write.out | 4 +-
2 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write
index a32608f597..5c90b8cd50 100755
--- a/tests/qemu-iotests/tests/copy-before-write
+++ b/tests/qemu-iotests/tests/copy-before-write
@@ -122,6 +122,84 @@ read 1048576/1048576 bytes at offset 0
1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
""")
+ def do_cbw_timeout(self, on_cbw_error):
+ result = self.vm.qmp('object-add', {
+ 'qom-type': 'throttle-group',
+ 'id': 'group0',
+ 'limits': {'bps-write': 300 * 1024}
+ })
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm.qmp('blockdev-add', {
+ 'node-name': 'cbw',
+ 'driver': 'copy-before-write',
+ 'on-cbw-error': on_cbw_error,
+ 'cbw-timeout': 1,
+ 'file': {
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': source_img,
+ }
+ },
+ 'target': {
+ 'driver': 'throttle',
+ 'throttle-group': 'group0',
+ 'file': {
+ 'driver': 'qcow2',
+ 'file': {
+ 'driver': 'file',
+ 'filename': temp_img
+ }
+ }
+ }
+ })
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm.qmp('blockdev-add', {
+ 'node-name': 'access',
+ 'driver': 'snapshot-access',
+ 'file': 'cbw'
+ })
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm.hmp_qemu_io('cbw', 'write 0 512K')
+ self.assert_qmp(result, 'return', '')
+
+ # We need second write to trigger throttling
+ result = self.vm.hmp_qemu_io('cbw', 'write 512K 512K')
+ self.assert_qmp(result, 'return', '')
+
+ result = self.vm.hmp_qemu_io('access', 'read 0 1M')
+ self.assert_qmp(result, 'return', '')
+
+ self.vm.shutdown()
+ log = self.vm.get_log()
+ log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log)
+ log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log)
+ log = iotests.filter_qemu_io(log)
+ return log
+
+ def test_timeout_break_guest(self):
+ log = self.do_cbw_timeout('break-guest-write')
+ self.assertEqual(log, """\
+wrote 524288/524288 bytes at offset 0
+512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+write failed: Connection timed out
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+""")
+
+ def test_timeout_break_snapshot(self):
+ log = self.do_cbw_timeout('break-snapshot')
+ self.assertEqual(log, """\
+wrote 524288/524288 bytes at offset 0
+512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 524288/524288 bytes at offset 524288
+512 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read failed: Permission denied
+""")
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2'],
diff --git a/tests/qemu-iotests/tests/copy-before-write.out b/tests/qemu-iotests/tests/copy-before-write.out
index fbc63e62f8..89968f35d7 100644
--- a/tests/qemu-iotests/tests/copy-before-write.out
+++ b/tests/qemu-iotests/tests/copy-before-write.out
@@ -1,5 +1,5 @@
-..
+....
----------------------------------------------------------------------
-Ran 2 tests
+Ran 4 tests
OK
--
2.35.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/7] block/copy-before-write: refactor option parsing
2022-04-06 18:07 ` [PATCH v3 1/7] block/copy-before-write: refactor option parsing Vladimir Sementsov-Ogievskiy
@ 2022-04-07 7:08 ` Hanna Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Hanna Reitz @ 2022-04-07 7:08 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, vsementsov,
stefanha, eblake
On 06.04.22 20:07, Vladimir Sementsov-Ogievskiy wrote:
> We are going to add one more option of enum type. Let's refactor option
> parsing so that we can simply work with BlockdevOptionsCbw object.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
> block/copy-before-write.c | 55 ++++++++++++++++++++-------------------
> 1 file changed, 28 insertions(+), 27 deletions(-)
>
> diff --git a/block/copy-before-write.c b/block/copy-before-write.c
> index a8a06fdc09..6877ff893a 100644
> --- a/block/copy-before-write.c
> +++ b/block/copy-before-write.c
[...]
> @@ -376,6 +365,14 @@ static int cbw_open(BlockDriverState *bs, QDict *options, int flags,
> BDRVCopyBeforeWriteState *s = bs->opaque;
> BdrvDirtyBitmap *bitmap = NULL;
> int64_t cluster_size;
> + g_autoptr(BlockdevOptions) full_opts = NULL;
> + BlockdevOptionsCbw *opts;
> +
> + full_opts = cbw_parse_options(options, errp);
> + if (!full_opts) {
> + return -EINVAL;
> + }
> + opts = &full_opts->u.copy_before_write;
I would prefer an `assert(full_opts->driver ==
BLOCKDEV_DRIVER_COPY_BEFORE_WRITE);` here, but, either way:
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/7] block/copy-before-write: add on-cbw-error open parameter
2022-04-06 18:07 ` [PATCH v3 2/7] block/copy-before-write: add on-cbw-error open parameter Vladimir Sementsov-Ogievskiy
@ 2022-04-07 8:22 ` Hanna Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Hanna Reitz @ 2022-04-07 8:22 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, vsementsov,
stefanha, eblake
On 06.04.22 20:07, Vladimir Sementsov-Ogievskiy wrote:
> Currently, behavior on copy-before-write operation failure is simple:
> report error to the guest.
>
> Let's implement alternative behavior: break the whole copy-before-write
> process (and corresponding backup job or NBD client) but keep guest
> working. It's needed if we consider guest stability as more important.
>
> The realisation is simple: on copy-before-write failure we set
> s->snapshot_ret and continue guest operations. s->snapshot_ret being
> set will lead to all further snapshot API requests. Note that all
> in-flight snapshot-API requests may still success: we do wait for them
> on BREAK_SNAPSHOT-failure path in cbw_do_copy_before_write().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
> qapi/block-core.json | 25 ++++++++++++++++++++++++-
> block/copy-before-write.c | 32 ++++++++++++++++++++++++++++++--
> 2 files changed, 54 insertions(+), 3 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index beeb91952a..085f1666af 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
[...]
> @@ -4184,11 +4203,15 @@
> # modifications (or removing) of specified bitmap doesn't
> # influence the filter. (Since 7.0)
> #
> +# @on-cbw-error: Behavior on failure of copy-before-write operation.
> +# Default is @break-guest-write. (Since 7.0)
With *7.1:
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
> +#
> # Since: 6.2
> ##
> { 'struct': 'BlockdevOptionsCbw',
> 'base': 'BlockdevOptionsGenericFormat',
> - 'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap' } }
> + 'data': { 'target': 'BlockdevRef', '*bitmap': 'BlockDirtyBitmap',
> + '*on-cbw-error': 'OnCbwError' } }
>
> ##
> # @BlockdevOptions:
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 7/7] iotests: copy-before-write: add cases for cbw-timeout option
2022-04-06 18:08 ` [PATCH v3 7/7] iotests: copy-before-write: add cases for " Vladimir Sementsov-Ogievskiy
@ 2022-04-07 9:19 ` Hanna Reitz
2022-04-07 10:54 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 15+ messages in thread
From: Hanna Reitz @ 2022-04-07 9:19 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, vsementsov,
stefanha, eblake
On 06.04.22 20:08, Vladimir Sementsov-Ogievskiy wrote:
> Add two simple test-cases: timeout failure with
> break-snapshot-on-cbw-error behavior and similar with
> break-guest-write-on-cbw-error behavior.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
> tests/qemu-iotests/tests/copy-before-write | 78 +++++++++++++++++++
> .../qemu-iotests/tests/copy-before-write.out | 4 +-
> 2 files changed, 80 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write
> index a32608f597..5c90b8cd50 100755
> --- a/tests/qemu-iotests/tests/copy-before-write
> +++ b/tests/qemu-iotests/tests/copy-before-write
> @@ -122,6 +122,84 @@ read 1048576/1048576 bytes at offset 0
> 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> """)
>
> + def do_cbw_timeout(self, on_cbw_error):
> + result = self.vm.qmp('object-add', {
> + 'qom-type': 'throttle-group',
> + 'id': 'group0',
> + 'limits': {'bps-write': 300 * 1024}
Hm, yes, I can’t find a way to make this work without your other
series. For some reason, not even -accel tcg helps; and using qtest to
advance the virtual clock doesn’t really help because the qemu-io
commands block while the request is throttled.
One thing that should work would be to run everything in a
qemu-storage-daemon instance, and then having qemu-io access an NBD
export...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 4/7] util: add qemu-co-timeout
2022-04-06 18:07 ` [PATCH v3 4/7] util: add qemu-co-timeout Vladimir Sementsov-Ogievskiy
@ 2022-04-07 9:20 ` Hanna Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Hanna Reitz @ 2022-04-07 9:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, vsementsov,
stefanha, eblake
On 06.04.22 20:07, Vladimir Sementsov-Ogievskiy wrote:
> Add new API, to make a time limited call of the coroutine.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
> include/qemu/coroutine.h | 13 ++++++
> util/qemu-co-timeout.c | 89 ++++++++++++++++++++++++++++++++++++++++
> util/meson.build | 1 +
> 3 files changed, 103 insertions(+)
> create mode 100644 util/qemu-co-timeout.c
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 5/7] block/block-copy: block_copy(): add timeout_ns parameter
2022-04-06 18:07 ` [PATCH v3 5/7] block/block-copy: block_copy(): add timeout_ns parameter Vladimir Sementsov-Ogievskiy
@ 2022-04-07 9:20 ` Hanna Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Hanna Reitz @ 2022-04-07 9:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, vsementsov,
stefanha, eblake
On 06.04.22 20:07, Vladimir Sementsov-Ogievskiy wrote:
> Add possibility to limit block_copy() call in time. To be used in the
> next commit.
>
> As timed-out block_copy() call will continue in background anyway (we
> can't immediately cancel IO operation), it's important also give user a
> possibility to pass a callback, to do some additional actions on
> block-copy call finish.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
> include/block/block-copy.h | 4 +++-
> block/block-copy.c | 33 ++++++++++++++++++++++++++-------
> block/copy-before-write.c | 2 +-
> 3 files changed, 30 insertions(+), 9 deletions(-)
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 6/7] block/copy-before-write: implement cbw-timeout option
2022-04-06 18:08 ` [PATCH v3 6/7] block/copy-before-write: implement cbw-timeout option Vladimir Sementsov-Ogievskiy
@ 2022-04-07 9:20 ` Hanna Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Hanna Reitz @ 2022-04-07 9:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: kwolf, v.sementsov-og, jsnow, qemu-devel, armbru, vsementsov,
stefanha, eblake
On 06.04.22 20:08, Vladimir Sementsov-Ogievskiy wrote:
> In some scenarios, when copy-before-write operations lasts too long
> time, it's better to cancel it.
>
> Most useful would be to use the new option together with
> on-cbw-error=break-snapshot: this way if cbw operation takes too long
> time we'll just cancel backup process but do not disturb the guest too
> much.
>
> Note the tricky point of realization: we keep additional point in
> bs->in_fligth during block_copy operation even if it's timed-out.
*flight
> Background "cancelled" block_copy operations will finish at some point
> and will want to access state. We should care to not free the state in
> .bdrv_close() earlier.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
> qapi/block-core.json | 8 +++++++-
> block/copy-before-write.c | 23 ++++++++++++++++++++++-
> 2 files changed, 29 insertions(+), 2 deletions(-)
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 7/7] iotests: copy-before-write: add cases for cbw-timeout option
2022-04-07 9:19 ` Hanna Reitz
@ 2022-04-07 10:54 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 15+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-04-07 10:54 UTC (permalink / raw)
To: Hanna Reitz, Vladimir Sementsov-Ogievskiy, qemu-block
Cc: qemu-devel, eblake, armbru, stefanha, kwolf, jsnow, vsementsov
07.04.2022 12:19, Hanna Reitz wrote:
> On 06.04.22 20:08, Vladimir Sementsov-Ogievskiy wrote:
>> Add two simple test-cases: timeout failure with
>> break-snapshot-on-cbw-error behavior and similar with
>> break-guest-write-on-cbw-error behavior.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
>> ---
>> tests/qemu-iotests/tests/copy-before-write | 78 +++++++++++++++++++
>> .../qemu-iotests/tests/copy-before-write.out | 4 +-
>> 2 files changed, 80 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write
>> index a32608f597..5c90b8cd50 100755
>> --- a/tests/qemu-iotests/tests/copy-before-write
>> +++ b/tests/qemu-iotests/tests/copy-before-write
>> @@ -122,6 +122,84 @@ read 1048576/1048576 bytes at offset 0
>> 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> """)
>> + def do_cbw_timeout(self, on_cbw_error):
>> + result = self.vm.qmp('object-add', {
>> + 'qom-type': 'throttle-group',
>> + 'id': 'group0',
>> + 'limits': {'bps-write': 300 * 1024}
>
> Hm, yes, I can’t find a way to make this work without your other series. For some reason, not even -accel tcg helps; and using qtest to advance the virtual clock doesn’t really help because the qemu-io commands block while the request is throttled.
>
> One thing that should work would be to run everything in a qemu-storage-daemon instance, and then having qemu-io access an NBD export...
>
Simple switch to QEMUMachine helps. I'll resend soon.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-04-07 10:57 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-06 18:07 [PATCH v3 0/7] copy-before-write: on-cbw-error and cbw-timeout Vladimir Sementsov-Ogievskiy
2022-04-06 18:07 ` [PATCH v3 1/7] block/copy-before-write: refactor option parsing Vladimir Sementsov-Ogievskiy
2022-04-07 7:08 ` Hanna Reitz
2022-04-06 18:07 ` [PATCH v3 2/7] block/copy-before-write: add on-cbw-error open parameter Vladimir Sementsov-Ogievskiy
2022-04-07 8:22 ` Hanna Reitz
2022-04-06 18:07 ` [PATCH v3 3/7] iotests: add copy-before-write: on-cbw-error tests Vladimir Sementsov-Ogievskiy
2022-04-06 18:07 ` [PATCH v3 4/7] util: add qemu-co-timeout Vladimir Sementsov-Ogievskiy
2022-04-07 9:20 ` Hanna Reitz
2022-04-06 18:07 ` [PATCH v3 5/7] block/block-copy: block_copy(): add timeout_ns parameter Vladimir Sementsov-Ogievskiy
2022-04-07 9:20 ` Hanna Reitz
2022-04-06 18:08 ` [PATCH v3 6/7] block/copy-before-write: implement cbw-timeout option Vladimir Sementsov-Ogievskiy
2022-04-07 9:20 ` Hanna Reitz
2022-04-06 18:08 ` [PATCH v3 7/7] iotests: copy-before-write: add cases for " Vladimir Sementsov-Ogievskiy
2022-04-07 9:19 ` Hanna Reitz
2022-04-07 10:54 ` Vladimir Sementsov-Ogievskiy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.