* [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY @ 2018-05-01 22:05 Max Reitz 2018-05-01 22:05 ` [Qemu-devel] [PATCH 1/2] " Max Reitz ` (4 more replies) 0 siblings, 5 replies; 9+ messages in thread From: Max Reitz @ 2018-05-01 22:05 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Max Reitz, Jeff Cody, Kevin Wolf, John Snow Currently, you cannot cancel a mirror job without the @force flag set. This is intentional once source and target are in sync, but probably not so much before that happens. The main reason for me thinking this is because it is an undocumented change in 2.12.0 in respect to 2.11. (b76e4458b1eb3c32e9824fe6aa51f67d2b251748 only notes a behavior change after READY, but not before.) This series depends on Stefan’s patch “block/mirror: honor ratelimit again” (which is in Jeff’s queue). Based-on: <20180424123527.19168-1-stefanha@redhat.com> Max Reitz (2): block/mirror: Make cancel always cancel pre-READY iotests: Add test for cancelling a mirror job block/mirror.c | 4 +- tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/218.out | 30 ++++++++++ tests/qemu-iotests/group | 1 + 4 files changed, 172 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/218 create mode 100644 tests/qemu-iotests/218.out -- 2.14.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 1/2] block/mirror: Make cancel always cancel pre-READY 2018-05-01 22:05 [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Max Reitz @ 2018-05-01 22:05 ` Max Reitz 2018-05-01 23:31 ` Eric Blake 2018-05-01 22:05 ` [Qemu-devel] [PATCH 2/2] iotests: Add test for cancelling a mirror job Max Reitz ` (3 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Max Reitz @ 2018-05-01 22:05 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Max Reitz, Jeff Cody, Kevin Wolf, John Snow Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 made the mirror block job respect block-job-cancel's @force flag: With that flag set, it would now always really cancel, even post-READY. Unfortunately, it had a side effect: Without that flag set, it would now never cancel, not even before READY. Considering that is an incompatible change and not noted anywhere in the commit or the description of block-job-cancel's @force parameter, this seems unintentional and we should revert to the previous behavior, which is to immediately cancel the job when block-job-cancel is called before source and target are in sync (i.e. before the READY event). Cc: qemu-stable@nongnu.org Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1572856 Reported-by: Yanan Fu <yfu@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/mirror.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/mirror.c b/block/mirror.c index 7bfad6e844..003f957b12 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -874,7 +874,9 @@ static void coroutine_fn mirror_run(void *opaque) } trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); block_job_sleep_ns(&s->common, delay_ns); - if (block_job_is_cancelled(&s->common) && s->common.force) { + if (block_job_is_cancelled(&s->common) && + (!s->synced || s->common.force)) + { break; } s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); -- 2.14.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] block/mirror: Make cancel always cancel pre-READY 2018-05-01 22:05 ` [Qemu-devel] [PATCH 1/2] " Max Reitz @ 2018-05-01 23:31 ` Eric Blake 2018-05-02 12:35 ` Max Reitz 0 siblings, 1 reply; 9+ messages in thread From: Eric Blake @ 2018-05-01 23:31 UTC (permalink / raw) To: Max Reitz, qemu-block Cc: Kevin Wolf, Jeff Cody, John Snow, qemu-devel, qemu-stable On 05/01/2018 05:05 PM, Max Reitz wrote: > Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 made the mirror block > job respect block-job-cancel's @force flag: With that flag set, it would > now always really cancel, even post-READY. > > Unfortunately, it had a side effect: Without that flag set, it would now > never cancel, not even before READY. Considering that is an > incompatible change and not noted anywhere in the commit or the > description of block-job-cancel's @force parameter, this seems > unintentional and we should revert to the previous behavior, which is to > immediately cancel the job when block-job-cancel is called before source > and target are in sync (i.e. before the READY event). > > Cc: qemu-stable@nongnu.org Actually adding that in cc on this reply. > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1572856 > Reported-by: Yanan Fu <yfu@redhat.com> > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/mirror.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake <eblake@redhat.com> > > diff --git a/block/mirror.c b/block/mirror.c > index 7bfad6e844..003f957b12 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -874,7 +874,9 @@ static void coroutine_fn mirror_run(void *opaque) > } > trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); > block_job_sleep_ns(&s->common, delay_ns); > - if (block_job_is_cancelled(&s->common) && s->common.force) { > + if (block_job_is_cancelled(&s->common) && > + (!s->synced || s->common.force)) > + { > break; > } > s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] block/mirror: Make cancel always cancel pre-READY 2018-05-01 23:31 ` Eric Blake @ 2018-05-02 12:35 ` Max Reitz 0 siblings, 0 replies; 9+ messages in thread From: Max Reitz @ 2018-05-02 12:35 UTC (permalink / raw) To: Eric Blake, qemu-block Cc: Kevin Wolf, Jeff Cody, John Snow, qemu-devel, qemu-stable [-- Attachment #1: Type: text/plain, Size: 1986 bytes --] On 2018-05-02 01:31, Eric Blake wrote: > On 05/01/2018 05:05 PM, Max Reitz wrote: >> Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 made the mirror block >> job respect block-job-cancel's @force flag: With that flag set, it would >> now always really cancel, even post-READY. >> >> Unfortunately, it had a side effect: Without that flag set, it would now >> never cancel, not even before READY. Considering that is an >> incompatible change and not noted anywhere in the commit or the >> description of block-job-cancel's @force parameter, this seems >> unintentional and we should revert to the previous behavior, which is to >> immediately cancel the job when block-job-cancel is called before source >> and target are in sync (i.e. before the READY event). >> >> Cc: qemu-stable@nongnu.org > > Actually adding that in cc on this reply. Thanks, I knew I was going to forget... Max >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1572856 >> Reported-by: Yanan Fu <yfu@redhat.com> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> block/mirror.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> > >> >> diff --git a/block/mirror.c b/block/mirror.c >> index 7bfad6e844..003f957b12 100644 >> --- a/block/mirror.c >> +++ b/block/mirror.c >> @@ -874,7 +874,9 @@ static void coroutine_fn mirror_run(void *opaque) >> } >> trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); >> block_job_sleep_ns(&s->common, delay_ns); >> - if (block_job_is_cancelled(&s->common) && s->common.force) { >> + if (block_job_is_cancelled(&s->common) && >> + (!s->synced || s->common.force)) >> + { >> break; >> } >> s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); >> > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 2/2] iotests: Add test for cancelling a mirror job 2018-05-01 22:05 [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Max Reitz 2018-05-01 22:05 ` [Qemu-devel] [PATCH 1/2] " Max Reitz @ 2018-05-01 22:05 ` Max Reitz 2018-05-02 14:31 ` Jeff Cody 2018-05-01 23:29 ` [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Eric Blake ` (2 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Max Reitz @ 2018-05-01 22:05 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Max Reitz, Jeff Cody, Kevin Wolf, John Snow We already have an extensive mirror test (041) which does cover cancelling a mirror job, especially after it has emitted the READY event. However, it does not check what exact events are emitted after block-job-cancel is executed. More importantly, it does not use throttling to ensure that it covers the case of block-job-cancel before READY. It would be possible to add this case to 041, but considering it is already our largest test file, it makes sense to create a new file for these cases. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/218.out | 30 ++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 169 insertions(+) create mode 100755 tests/qemu-iotests/218 create mode 100644 tests/qemu-iotests/218.out diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218 new file mode 100755 index 0000000000..92c331b6fb --- /dev/null +++ b/tests/qemu-iotests/218 @@ -0,0 +1,138 @@ +#!/usr/bin/env python +# +# This test covers what happens when a mirror block job is cancelled +# in various phases of its existence. +# +# Note that this test only checks the emitted events (i.e. +# BLOCK_JOB_COMPLETED vs. BLOCK_JOB_CANCELLED), it does not compare +# whether the target is in sync with the source when the +# BLOCK_JOB_COMPLETED event occurs. This is covered by other tests +# (such as 041). +# +# Copyright (C) 2018 Red Hat, Inc. +# +# 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/>. +# +# Creator/Owner: Max Reitz <mreitz@redhat.com> + +import iotests +from iotests import log + +iotests.verify_platform(['linux']) + + +# Launches the VM, adds two null-co nodes (source and target), and +# starts a blockdev-mirror job on them. +# +# Either both or none of speed and buf_size must be given. + +def start_mirror(vm, speed=None, buf_size=None): + vm.launch() + + ret = vm.qmp('blockdev-add', + node_name='source', + driver='null-co', + size=1048576) + assert ret['return'] == {} + + ret = vm.qmp('blockdev-add', + node_name='target', + driver='null-co', + size=1048576) + assert ret['return'] == {} + + if speed is not None: + ret = vm.qmp('blockdev-mirror', + job_id='mirror', + device='source', + target='target', + sync='full', + speed=speed, + buf_size=buf_size) + else: + ret = vm.qmp('blockdev-mirror', + job_id='mirror', + device='source', + target='target', + sync='full') + + assert ret['return'] == {} + + +log('') +log('=== Cancel mirror job before convergence ===') +log('') + +log('--- force=false ---') +log('') + +with iotests.VM() as vm: + # Low speed so it does not converge + start_mirror(vm, 65536, 65536) + + log('Cancelling job') + log(vm.qmp('block-job-cancel', device='mirror', force=False)) + + log(vm.event_wait('BLOCK_JOB_CANCELLED'), + filters=[iotests.filter_qmp_event]) + +log('') +log('--- force=true ---') +log('') + +with iotests.VM() as vm: + # Low speed so it does not converge + start_mirror(vm, 65536, 65536) + + log('Cancelling job') + log(vm.qmp('block-job-cancel', device='mirror', force=True)) + + log(vm.event_wait('BLOCK_JOB_CANCELLED'), + filters=[iotests.filter_qmp_event]) + + +log('') +log('=== Cancel mirror job after convergence ===') +log('') + +log('--- force=false ---') +log('') + +with iotests.VM() as vm: + start_mirror(vm) + + log(vm.event_wait('BLOCK_JOB_READY'), + filters=[iotests.filter_qmp_event]) + + log('Cancelling job') + log(vm.qmp('block-job-cancel', device='mirror', force=False)) + + log(vm.event_wait('BLOCK_JOB_COMPLETED'), + filters=[iotests.filter_qmp_event]) + +log('') +log('--- force=true ---') +log('') + +with iotests.VM() as vm: + start_mirror(vm) + + log(vm.event_wait('BLOCK_JOB_READY'), + filters=[iotests.filter_qmp_event]) + + log('Cancelling job') + log(vm.qmp('block-job-cancel', device='mirror', force=True)) + + log(vm.event_wait('BLOCK_JOB_CANCELLED'), + filters=[iotests.filter_qmp_event]) diff --git a/tests/qemu-iotests/218.out b/tests/qemu-iotests/218.out new file mode 100644 index 0000000000..7dbf78e682 --- /dev/null +++ b/tests/qemu-iotests/218.out @@ -0,0 +1,30 @@ + +=== Cancel mirror job before convergence === + +--- force=false --- + +Cancelling job +{u'return': {}} +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'} + +--- force=true --- + +Cancelling job +{u'return': {}} +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'} + +=== Cancel mirror job after convergence === + +--- force=false --- + +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'} +Cancelling job +{u'return': {}} +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_COMPLETED'} + +--- force=true --- + +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'} +Cancelling job +{u'return': {}} +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_CANCELLED'} diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 2edc377370..cc8cd8cc8e 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -215,3 +215,4 @@ 214 rw auto 215 rw auto quick 216 rw auto quick +218 rw auto quick -- 2.14.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] iotests: Add test for cancelling a mirror job 2018-05-01 22:05 ` [Qemu-devel] [PATCH 2/2] iotests: Add test for cancelling a mirror job Max Reitz @ 2018-05-02 14:31 ` Jeff Cody 0 siblings, 0 replies; 9+ messages in thread From: Jeff Cody @ 2018-05-02 14:31 UTC (permalink / raw) To: Max Reitz; +Cc: qemu-block, qemu-devel, Kevin Wolf, John Snow On Wed, May 02, 2018 at 12:05:09AM +0200, Max Reitz wrote: > We already have an extensive mirror test (041) which does cover > cancelling a mirror job, especially after it has emitted the READY > event. However, it does not check what exact events are emitted after > block-job-cancel is executed. More importantly, it does not use > throttling to ensure that it covers the case of block-job-cancel before > READY. > > It would be possible to add this case to 041, but considering it is > already our largest test file, it makes sense to create a new file for > these cases. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/218.out | 30 ++++++++++ > tests/qemu-iotests/group | 1 + Minor conflict in group, but nothing that I can't sort out when applying. > 3 files changed, 169 insertions(+) > create mode 100755 tests/qemu-iotests/218 > create mode 100644 tests/qemu-iotests/218.out > > diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218 > new file mode 100755 > index 0000000000..92c331b6fb > --- /dev/null > +++ b/tests/qemu-iotests/218 > @@ -0,0 +1,138 @@ > +#!/usr/bin/env python > +# > +# This test covers what happens when a mirror block job is cancelled > +# in various phases of its existence. > +# > +# Note that this test only checks the emitted events (i.e. > +# BLOCK_JOB_COMPLETED vs. BLOCK_JOB_CANCELLED), it does not compare > +# whether the target is in sync with the source when the > +# BLOCK_JOB_COMPLETED event occurs. This is covered by other tests > +# (such as 041). > +# > +# Copyright (C) 2018 Red Hat, Inc. > +# > +# 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/>. > +# > +# Creator/Owner: Max Reitz <mreitz@redhat.com> > + > +import iotests > +from iotests import log > + > +iotests.verify_platform(['linux']) > + > + > +# Launches the VM, adds two null-co nodes (source and target), and > +# starts a blockdev-mirror job on them. > +# > +# Either both or none of speed and buf_size must be given. > + > +def start_mirror(vm, speed=None, buf_size=None): > + vm.launch() > + > + ret = vm.qmp('blockdev-add', > + node_name='source', > + driver='null-co', > + size=1048576) > + assert ret['return'] == {} > + > + ret = vm.qmp('blockdev-add', > + node_name='target', > + driver='null-co', > + size=1048576) > + assert ret['return'] == {} > + > + if speed is not None: > + ret = vm.qmp('blockdev-mirror', > + job_id='mirror', > + device='source', > + target='target', > + sync='full', > + speed=speed, > + buf_size=buf_size) > + else: > + ret = vm.qmp('blockdev-mirror', > + job_id='mirror', > + device='source', > + target='target', > + sync='full') > + > + assert ret['return'] == {} > + > + > +log('') > +log('=== Cancel mirror job before convergence ===') > +log('') > + > +log('--- force=false ---') > +log('') > + > +with iotests.VM() as vm: > + # Low speed so it does not converge > + start_mirror(vm, 65536, 65536) > + > + log('Cancelling job') > + log(vm.qmp('block-job-cancel', device='mirror', force=False)) > + > + log(vm.event_wait('BLOCK_JOB_CANCELLED'), > + filters=[iotests.filter_qmp_event]) > + > +log('') > +log('--- force=true ---') > +log('') > + > +with iotests.VM() as vm: > + # Low speed so it does not converge > + start_mirror(vm, 65536, 65536) > + > + log('Cancelling job') > + log(vm.qmp('block-job-cancel', device='mirror', force=True)) > + > + log(vm.event_wait('BLOCK_JOB_CANCELLED'), > + filters=[iotests.filter_qmp_event]) > + > + > +log('') > +log('=== Cancel mirror job after convergence ===') > +log('') > + > +log('--- force=false ---') > +log('') > + > +with iotests.VM() as vm: > + start_mirror(vm) > + > + log(vm.event_wait('BLOCK_JOB_READY'), > + filters=[iotests.filter_qmp_event]) > + > + log('Cancelling job') > + log(vm.qmp('block-job-cancel', device='mirror', force=False)) > + > + log(vm.event_wait('BLOCK_JOB_COMPLETED'), > + filters=[iotests.filter_qmp_event]) > + > +log('') > +log('--- force=true ---') > +log('') > + > +with iotests.VM() as vm: > + start_mirror(vm) > + > + log(vm.event_wait('BLOCK_JOB_READY'), > + filters=[iotests.filter_qmp_event]) > + > + log('Cancelling job') > + log(vm.qmp('block-job-cancel', device='mirror', force=True)) > + > + log(vm.event_wait('BLOCK_JOB_CANCELLED'), > + filters=[iotests.filter_qmp_event]) > diff --git a/tests/qemu-iotests/218.out b/tests/qemu-iotests/218.out > new file mode 100644 > index 0000000000..7dbf78e682 > --- /dev/null > +++ b/tests/qemu-iotests/218.out > @@ -0,0 +1,30 @@ > + > +=== Cancel mirror job before convergence === > + > +--- force=false --- > + > +Cancelling job > +{u'return': {}} > +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'} > + > +--- force=true --- > + > +Cancelling job > +{u'return': {}} > +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 65536, u'len': 1048576, u'offset': 65536}, u'event': u'BLOCK_JOB_CANCELLED'} > + > +=== Cancel mirror job after convergence === > + > +--- force=false --- > + > +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'} > +Cancelling job > +{u'return': {}} > +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_COMPLETED'} > + > +--- force=true --- > + > +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_READY'} > +Cancelling job > +{u'return': {}} > +{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'device': u'mirror', u'type': u'mirror', u'speed': 0, u'len': 1048576, u'offset': 1048576}, u'event': u'BLOCK_JOB_CANCELLED'} > diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group > index 2edc377370..cc8cd8cc8e 100644 > --- a/tests/qemu-iotests/group > +++ b/tests/qemu-iotests/group > @@ -215,3 +215,4 @@ > 214 rw auto > 215 rw auto quick > 216 rw auto quick > +218 rw auto quick > -- > 2.14.3 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY 2018-05-01 22:05 [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Max Reitz 2018-05-01 22:05 ` [Qemu-devel] [PATCH 1/2] " Max Reitz 2018-05-01 22:05 ` [Qemu-devel] [PATCH 2/2] iotests: Add test for cancelling a mirror job Max Reitz @ 2018-05-01 23:29 ` Eric Blake 2018-05-02 14:32 ` Jeff Cody 2018-05-02 17:36 ` John Snow 4 siblings, 0 replies; 9+ messages in thread From: Eric Blake @ 2018-05-01 23:29 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Jeff Cody, John Snow, qemu-devel On 05/01/2018 05:05 PM, Max Reitz wrote: > Currently, you cannot cancel a mirror job without the @force flag set. > This is intentional once source and target are in sync, but probably not > so much before that happens. The main reason for me thinking this is > because it is an undocumented change in 2.12.0 in respect to 2.11. Yeah, that definitely feels like a regression worth fixing. > > (b76e4458b1eb3c32e9824fe6aa51f67d2b251748 only notes a behavior change > after READY, but not before.) > > > This series depends on Stefan’s patch “block/mirror: honor ratelimit > again” (which is in Jeff’s queue). > > Based-on: <20180424123527.19168-1-stefanha@redhat.com> > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY 2018-05-01 22:05 [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Max Reitz ` (2 preceding siblings ...) 2018-05-01 23:29 ` [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Eric Blake @ 2018-05-02 14:32 ` Jeff Cody 2018-05-02 17:36 ` John Snow 4 siblings, 0 replies; 9+ messages in thread From: Jeff Cody @ 2018-05-02 14:32 UTC (permalink / raw) To: Max Reitz; +Cc: qemu-block, qemu-devel, Kevin Wolf, John Snow On Wed, May 02, 2018 at 12:05:07AM +0200, Max Reitz wrote: > Currently, you cannot cancel a mirror job without the @force flag set. > This is intentional once source and target are in sync, but probably not > so much before that happens. The main reason for me thinking this is > because it is an undocumented change in 2.12.0 in respect to 2.11. > > (b76e4458b1eb3c32e9824fe6aa51f67d2b251748 only notes a behavior change > after READY, but not before.) > > > This series depends on Stefan’s patch “block/mirror: honor ratelimit > again” (which is in Jeff’s queue). > > Based-on: <20180424123527.19168-1-stefanha@redhat.com> > > > Max Reitz (2): > block/mirror: Make cancel always cancel pre-READY > iotests: Add test for cancelling a mirror job > > block/mirror.c | 4 +- > tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/218.out | 30 ++++++++++ > tests/qemu-iotests/group | 1 + > 4 files changed, 172 insertions(+), 1 deletion(-) > create mode 100755 tests/qemu-iotests/218 > create mode 100644 tests/qemu-iotests/218.out > > -- > 2.14.3 > Thanks, Applied to my block branch: git://github.com/codyprime/qemu-kvm-jtc block -Jeff ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY 2018-05-01 22:05 [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Max Reitz ` (3 preceding siblings ...) 2018-05-02 14:32 ` Jeff Cody @ 2018-05-02 17:36 ` John Snow 4 siblings, 0 replies; 9+ messages in thread From: John Snow @ 2018-05-02 17:36 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Jeff Cody, qemu-devel On 05/01/2018 06:05 PM, Max Reitz wrote: > Currently, you cannot cancel a mirror job without the @force flag set. > This is intentional once source and target are in sync, but probably not > so much before that happens. The main reason for me thinking this is > because it is an undocumented change in 2.12.0 in respect to 2.11. > > (b76e4458b1eb3c32e9824fe6aa51f67d2b251748 only notes a behavior change > after READY, but not before.) > > > This series depends on Stefan’s patch “block/mirror: honor ratelimit > again” (which is in Jeff’s queue). > > Based-on: <20180424123527.19168-1-stefanha@redhat.com> > > > Max Reitz (2): > block/mirror: Make cancel always cancel pre-READY > iotests: Add test for cancelling a mirror job > > block/mirror.c | 4 +- > tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/218.out | 30 ++++++++++ > tests/qemu-iotests/group | 1 + > 4 files changed, 172 insertions(+), 1 deletion(-) > create mode 100755 tests/qemu-iotests/218 > create mode 100644 tests/qemu-iotests/218.out > Way too late to the party, but all the same, since I was CC'd and I already looked into it: Reviewed-by: John Snow <jsnow@redhat.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-05-02 17:36 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-01 22:05 [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Max Reitz 2018-05-01 22:05 ` [Qemu-devel] [PATCH 1/2] " Max Reitz 2018-05-01 23:31 ` Eric Blake 2018-05-02 12:35 ` Max Reitz 2018-05-01 22:05 ` [Qemu-devel] [PATCH 2/2] iotests: Add test for cancelling a mirror job Max Reitz 2018-05-02 14:31 ` Jeff Cody 2018-05-01 23:29 ` [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY Eric Blake 2018-05-02 14:32 ` Jeff Cody 2018-05-02 17:36 ` John Snow
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.