The following changes since commit 6d8e75d41c58892ccc5d4ad61c4da476684c1c83:

Merge remote-tracking branch 'remotes/rth/tags/pull-axp-20190519' into staging (2019-05-20 11:38:36 +0100)

for you to fetch changes up to c423a6af592cf36b4f149c54e2966dd0016b7e96:

iotests: Make 245 faster and more reliable (2019-05-20 17:08:57 +0200)

- block: AioContext management, part 1

- qmp: forbid qmp_cont in RUN_STATE_FINISH_MIGRATE

- nvme: fix copy direction in DMA reads going to CMB

- file-posix: Fix block status for unaligned raw images with O_DIRECT

- file-posix: Fix xfs_write_zeroes() after EOF

- Documentation and iotests improvements

Alberto Garcia (2):

qcow2: Define and use QCOW2_COMPRESSED_SECTOR_SIZE

block: Use BDRV_REQUEST_MAX_BYTES instead of BDRV_REQUEST_MAX_SECTORS

Kevin Wolf (10):

block: Add bdrv_try_set_aio_context()

block: Make bdrv_attach/detach_aio_context() static

block: Move recursion to bdrv_set_aio_context()

block: Propagate AioContext change to parents

test-block-iothread: Test AioContext propagation through the tree

block: Implement .(can_)set_aio_ctx for BlockBackend

block: Add blk_set_allow_aio_context_change()

blockjob: Propagate AioContext change to all job nodes

blockjob: Remove AioContext notifiers

test-block-iothread: Test AioContext propagation for block jobs

Klaus Birkelund Jensen (1):

nvme: fix copy direction in DMA reads going to CMB

Max Reitz (9):

block/file-posix: Truncate in xfs_write_zeroes()

block/file-posix: Unaligned O_DIRECT block-status

iotests: Test unaligned raw images with O_DIRECT

qemu-img.texi: Be specific about JSON object types

qemu-img.texi: Describe human-readable info output

block: Improve "Block node is read-only" message

iotests.py: Let assert_qmp() accept an array

iotests.py: Fix VM.run_job

iotests: Make 245 faster and more reliable

Vladimir Sementsov-Ogievskiy (2):

qmp: forbid qmp_cont in RUN_STATE_FINISH_MIGRATE

iotest: fix 169: do not run qmp_cont in RUN_STATE_FINISH_MIGRATE

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

qmp_cont in RUN_STATE_FINISH_MIGRATE may lead to moving vm to

RUN_STATE_RUNNING, before actual migration finish. So, when migration

thread will try to go to RUN_STATE_POSTMIGRATE, assuming transition

RUN_STATE_FINISH_MIGRATE->RUN_STATE_POSTMIGRATE, it will crash, as

current state is RUN_STATE_RUNNING, and transition

RUN_STATE_RUNNING->RUN_STATE_POSTMIGRATE is forbidden.

Reported-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

qmp.c | 3 +++

diff --git a/qmp.c b/qmp.c

--- a/qmp.c

+++ b/qmp.c

@@ -XXX,XX +XXX,XX @@ void qmp_cont(Error **errp)

return;

} else if (runstate_check(RUN_STATE_SUSPENDED)) {

return;

+ } else if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {

+ error_setg(errp, "Migration is not finalized yet");

+ return;

}

for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {

2.20.1

From: Max Reitz <mreitz@redhat.com>

Sometimes, 245 fails for me because some stream job has already finished

while the test expects it to still be active. (With -c none, it fails

basically every time.) The most reliable way to fix this is to simply

set auto_finalize=false so the job will remain in the block graph as

long as we need it. This allows us to drop the rate limiting, too,

which makes the test faster.

The only problem with this is that there is a single place that yields a

different error message depending on whether the stream job is still

copying data (so COR is enabled) or not (COR has been disabled, but the

job still has the WRITE_UNCHANGED permission on the target node). We

can easily address that by expecting either error message.

Note that we do not need auto_finalize=false (or rate limiting) for the

active commit job, because It never completes without an explicit

block-job-complete anyway.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

tests/qemu-iotests/245 | 22 ++++++++++++++--------

tests/qemu-iotests/245.out | 12 ++++++++++++

2 files changed, 26 insertions(+), 8 deletions(-)

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

--- a/tests/qemu-iotests/245

+++ b/tests/qemu-iotests/245

@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):

# hd2 <- hd0

result = self.vm.qmp('block-stream', conv_keys = True, job_id = 'stream0',

- device = 'hd0', base_node = 'hd2', speed = 512 * 1024)

+ device = 'hd0', base_node = 'hd2',

+ auto_finalize = False)

self.assert_qmp(result, 'return', {})

# We can't remove hd2 while the stream job is ongoing

@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):

opts['backing'] = None

self.reopen(opts, {}, "Cannot change 'backing' link from 'hd0' to 'hd1'")

- self.wait_until_completed(drive = 'stream0')

+ self.vm.run_job('stream0', auto_finalize = False, auto_dismiss = True)

# Reopen the chain during a block-stream job (from hd2 to hd1)

def test_block_stream_4(self):

@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):

# hd1 <- hd0

result = self.vm.qmp('block-stream', conv_keys = True, job_id = 'stream0',

- device = 'hd1', speed = 512 * 1024)

+ device = 'hd1', auto_finalize = False)

self.assert_qmp(result, 'return', {})

# We can't reopen with the original options because that would

# make hd1 read-only and block-stream requires it to be read-write

- self.reopen(opts, {}, "Can't set node 'hd1' to r/o with copy-on-read enabled")

+ # (Which error message appears depends on whether the stream job is

+ # already done with copying at this point.)

+ self.reopen(opts, {},

+ ["Can't set node 'hd1' to r/o with copy-on-read enabled",

+ "Cannot make block node read-only, there is a writer on it"])

# We can't remove hd2 while the stream job is ongoing

opts['backing']['backing'] = None

@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):

opts['backing'] = None

self.reopen(opts)

- self.wait_until_completed(drive = 'stream0')

+ self.vm.run_job('stream0', auto_finalize = False, auto_dismiss = True)

# Reopen the chain during a block-commit job (from hd0 to hd2)

def test_block_commit_1(self):

@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):

self.assert_qmp(result, 'return', {})

result = self.vm.qmp('block-commit', conv_keys = True, job_id = 'commit0',

- device = 'hd0', speed = 1024 * 1024)

+ device = 'hd0')

self.assert_qmp(result, 'return', {})

# We can't remove hd2 while the commit job is ongoing

@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):

self.assert_qmp(result, 'return', {})

result = self.vm.qmp('block-commit', conv_keys = True, job_id = 'commit0',

- device = 'hd0', top_node = 'hd1', speed = 1024 * 1024)

+ device = 'hd0', top_node = 'hd1',

+ auto_finalize = False)

self.assert_qmp(result, 'return', {})

# We can't remove hd2 while the commit job is ongoing

@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):

self.reopen(opts, {}, "Cannot change backing link if 'hd0' has an implicit backing file")

# hd2 <- hd0

- self.wait_until_completed(drive = 'commit0')

+ self.vm.run_job('commit0', auto_finalize = False, auto_dismiss = True)

self.assert_qmp(self.get_node('hd0'), 'ro', False)

self.assertEqual(self.get_node('hd1'), None)

diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/245.out

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

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

Ran 18 tests

OK

+{"execute": "job-finalize", "arguments": {"id": "commit0"}}

+{"return": {}}

+{"data": {"id": "commit0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+{"data": {"device": "commit0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+{"execute": "job-finalize", "arguments": {"id": "stream0"}}

+{"return": {}}

+{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+{"execute": "job-finalize", "arguments": {"id": "stream0"}}

+{"return": {}}

+{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

+{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

2.20.1

From: Max Reitz <mreitz@redhat.com>

Currently, qemu crashes whenever someone queries the block status of an

unaligned image tail of an O_DIRECT image:

$ echo > foo

$ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on

Offset Length Mapped to File

qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&

QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset'

failed.

This is because bdrv_co_block_status() checks that the result returned

by the driver's implementation is aligned to the request_alignment, but

file-posix can fail to do so, which is actually mentioned in a comment

there: "[...] possibly including a partial sector at EOF".

Fix this by rounding up those partial sectors.

There are two possible alternative fixes:

(1) We could refuse to open unaligned image files with O_DIRECT

altogether. That sounds reasonable until you realize that qcow2

does necessarily not fill up its metadata clusters, and that nobody

runs qemu-img create with O_DIRECT. Therefore, unpreallocated qcow2

files usually have an unaligned image tail.

(2) bdrv_co_block_status() could ignore unaligned tails. It actually

throws away everything past the EOF already, so that sounds

reasonable.

Unfortunately, the block layer knows file lengths only with a

granularity of BDRV_SECTOR_SIZE, so bdrv_co_block_status() usually

would have to guess whether its file length information is inexact

or whether the driver is broken.

Fixing what raw_co_block_status() returns is the safest thing to do.

There seems to be no other block driver that sets request_alignment and

does not make sure that it always returns aligned values.

Cc: qemu-stable@nongnu.org

Signed-off-by: Max Reitz <mreitz@redhat.com>

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

block/file-posix.c | 16 ++++++++++++++++

1 file changed, 16 insertions(+)

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_block_status(BlockDriverState *bs,

off_t data = 0, hole = 0;

+ assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));

+

ret = fd_open(bs);

if (ret < 0) {

return ret;

@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_block_status(BlockDriverState *bs,

/* On a data extent, compute bytes to the end of the extent,

* possibly including a partial sector at EOF. */

*pnum = MIN(bytes, hole - offset);

+

+ /*

+ * We are not allowed to return partial sectors, though, so

+ * round up if necessary.

+ */

+ if (!QEMU_IS_ALIGNED(*pnum, bs->bl.request_alignment)) {

+ int64_t file_length = raw_getlength(bs);

+ if (file_length > 0) {

+ /* Ignore errors, this is just a safeguard */

+ assert(hole == file_length);

+ }

+ *pnum = ROUND_UP(*pnum, bs->bl.request_alignment);

+

ret = BDRV_BLOCK_DATA;

/* On a hole, compute bytes to the beginning of the next extent. */

2.20.1

bdrv_try_set_aio_context() currently fails if a BlockBackend is attached

to a node because it doesn't implement the BdrvChildRole callbacks for

AioContext management.

We can allow changing the AioContext of monitor-owned BlockBackends as

long as no device is attached to them.

When setting the AioContext of the root node of a BlockBackend, we now

need to pass blk->root as an ignored child because we don't want the

root node to recursively call back into BlockBackend and execute

blk_do_set_aio_context() a second time.

block/block-backend.c | 45 +++++++++++++++++++++++++++++++++++++++++--

1 file changed, 43 insertions(+), 2 deletions(-)

@@ -XXX,XX +XXX,XX @@ static void blk_root_drained_end(BdrvChild *child);

static void blk_root_change_media(BdrvChild *child, bool load);

static void blk_root_resize(BdrvChild *child);

+static bool blk_root_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,

+ GSList **ignore, Error **errp);

+static void blk_root_set_aio_ctx(BdrvChild *child, AioContext *ctx,

+ GSList **ignore);

+

static char *blk_root_get_parent_desc(BdrvChild *child)

BlockBackend *blk = child->opaque;

@@ -XXX,XX +XXX,XX @@ static const BdrvChildRole child_root = {

.attach = blk_root_attach,

.detach = blk_root_detach,

+

+ .can_set_aio_ctx = blk_root_can_set_aio_ctx,

+ .set_aio_ctx = blk_root_set_aio_ctx,

};

@@ -XXX,XX +XXX,XX @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)

return blk_get_aio_context(blk_acb->blk);

}

-void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)

+static void blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,

+ bool update_root_node)

{

BlockDriverState *bs = blk_bs(blk);

ThrottleGroupMember *tgm = &blk->public.throttle_group_member;

@@ -XXX,XX +XXX,XX @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)

throttle_group_attach_aio_context(tgm, new_context);

bdrv_drained_end(bs);

}

- bdrv_set_aio_context(bs, new_context);

+ if (update_root_node) {

+ GSList *ignore = g_slist_prepend(NULL, blk->root);

+ bdrv_set_aio_context_ignore(bs, new_context, &ignore);

+ g_slist_free(ignore);

+ }

}

}

+void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)

+{

+ blk_do_set_aio_context(blk, new_context, true);

+}

+

+static bool blk_root_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,

+ GSList **ignore, Error **errp)

+{

+ BlockBackend *blk = child->opaque;

+

+ /* Only manually created BlockBackends that are not attached to anything

+ * can change their AioContext without updating their user. */

+ if (!blk->name || blk->dev) {

+ /* TODO Add BB name/QOM path */

+ error_setg(errp, "Cannot change iothread of active block backend");

+ return false;

+ }

+

+ return true;

+}

+

+static void blk_root_set_aio_ctx(BdrvChild *child, AioContext *ctx,

+ GSList **ignore)

+{

+ BlockBackend *blk = child->opaque;

+ blk_do_set_aio_context(blk, ctx, false);

+}

+

void blk_add_aio_context_notifier(BlockBackend *blk,

void (*attached_aio_context)(AioContext *new_context, void *opaque),

void (*detach_aio_context)(void *opaque), void *opaque)

2.20.1

Some users (like block jobs) can tolerate an AioContext change for their

BlockBackend. Add a function that tells the BlockBackend that it can

allow changes.

include/sysemu/block-backend.h | 1 +

block/block-backend.c | 10 ++++++++++

2 files changed, 11 insertions(+)

@@ -XXX,XX +XXX,XX @@ int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,

void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm);

void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);

+void blk_set_allow_aio_context_change(BlockBackend *blk, bool allow);

void blk_iostatus_enable(BlockBackend *blk);

bool blk_iostatus_is_enabled(const BlockBackend *blk);

BlockDeviceIoStatus blk_iostatus(const BlockBackend *blk);

@@ -XXX,XX +XXX,XX @@ struct BlockBackend {

uint64_t shared_perm;

bool disable_perm;

+ bool allow_aio_context_change;

bool allow_write_beyond_eof;

NotifierList remove_bs_notifiers, insert_bs_notifiers;

@@ -XXX,XX +XXX,XX @@ void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow)

blk->allow_write_beyond_eof = allow;

+void blk_set_allow_aio_context_change(BlockBackend *blk, bool allow)

+{

+ blk->allow_aio_context_change = allow;

+}

+

static int blk_check_byte_request(BlockBackend *blk, int64_t offset,

size_t size)

@@ -XXX,XX +XXX,XX @@ static bool blk_root_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,

{

BlockBackend *blk = child->opaque;

+ if (blk->allow_aio_context_change) {

+ return true;

+ }

+

/* Only manually created BlockBackends that are not attached to anything

* can change their AioContext without updating their user. */

if (!blk->name || blk->dev) {