The following changes since commit d01beac177d44491d7db8747b79d94e1b53d173b:

Merge remote-tracking branch 'remotes/kraxel/tags/vga-20180507-pull-request' into staging (2018-05-08 14:23:02 +0100)

are available in the git repository at:

git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to a2cb9239b7610ffb00f9ced5cd7640d40b0e1ccf:

sheepdog: Fix sd_co_create_opts() memory leaks (2018-05-08 10:47:27 -0400)

Block patches

Kevin Wolf (1):

sheepdog: Fix sd_co_create_opts() memory leaks

Max Reitz (2):

block/mirror: Make cancel always cancel pre-READY

iotests: Add test for cancelling a mirror job

Stefan Hajnoczi (1):

block/mirror: honor ratelimit again

block/mirror.c | 14 +++--

block/sheepdog.c | 4 +-

tests/qemu-iotests/185.out | 4 +-

tests/qemu-iotests/218 | 138 +++++++++++++++++++++++++++++++++++++++++++++

tests/qemu-iotests/218.out | 30 ++++++++++

tests/qemu-iotests/group | 1 +

6 files changed, 183 insertions(+), 8 deletions(-)

create mode 100644 tests/qemu-iotests/218

create mode 100644 tests/qemu-iotests/218.out

2.13.6

From: Max Reitz <mreitz@redhat.com>

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>

Message-id: 20180501220509.14152-3-mreitz@redhat.com

Signed-off-by: Jeff Cody <jcody@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 100644 tests/qemu-iotests/218

create mode 100644 tests/qemu-iotests/218.out

diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218

+++ b/tests/qemu-iotests/218

@@ -XXX,XX +XXX,XX @@

+#!/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

+++ b/tests/qemu-iotests/218.out

@@ -XXX,XX +XXX,XX @@

+

+=== 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

--- a/tests/qemu-iotests/group

+++ b/tests/qemu-iotests/group

@@ -XXX,XX +XXX,XX @@

211 rw auto quick

212 rw auto quick

213 rw auto quick

+218 rw auto quick

2.13.6

From: Kevin Wolf <kwolf@redhat.com>

Both the option string for the 'redundancy' option and the

SheepdogRedundancy object that is created accordingly could be leaked in

error paths. This fixes the memory leaks.

Reported by Coverity (CID 1390614 and 1390641).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-id: 20180503153509.22223-1-kwolf@redhat.com

Reviewed-by: Jeff Cody <jcody@redhat.com>

Signed-off-by: Jeff Cody <jcody@redhat.com>

block/sheepdog.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c

index XXXXXXX..XXXXXXX 100644

--- a/block/sheepdog.c

+++ b/block/sheepdog.c

@@ -XXX,XX +XXX,XX @@ static SheepdogRedundancy *parse_redundancy_str(const char *opt)

} else {

ret = qemu_strtol(n2, NULL, 10, &parity);

if (ret < 0) {

+ g_free(redundancy);

return NULL;

}

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_create_opts(const char *filename, QemuOpts *opts,

QDict *qdict, *location_qdict;

QObject *crumpled;

Visitor *v;

- const char *redundancy;

+ char *redundancy;

Error *local_err = NULL;

int ret;

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_create_opts(const char *filename, QemuOpts *opts,

fail:

qapi_free_BlockdevCreateOptions(create_options);

qobject_unref(qdict);

+ g_free(redundancy);

return ret;

2.13.6

From: Max Reitz <mreitz@redhat.com>

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>

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-id: 20180501220509.14152-2-mreitz@redhat.com

Reviewed-by: Jeff Cody <jcody@redhat.com>

Signed-off-by: Jeff Cody <jcody@redhat.com>

block/mirror.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ 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))

+ {

s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);