The following changes since commit 22c5f446514a2a4bb0dbe1fea26713da92fc85fa:

Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190211' into staging (2019-02-11 17:04:57 +0000)

git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 9a6719d572e99a4e79f589d0b73f7475b86f982d:

virtio-blk: cleanup using VirtIOBlock *s and VirtIODevice *vdev (2019-02-12 11:49:17 +0800)

Peter Xu (1):

iothread: fix iothread hang when stop too soon

Stefano Garzarella (1):

virtio-blk: cleanup using VirtIOBlock *s and VirtIODevice *vdev

Vladimir Sementsov-Ogievskiy (1):

qemugdb/coroutine: fix arch_prctl has unknown return type

hw/block/virtio-blk.c | 22 +++++++++-------------

iothread.c | 6 +++++-

scripts/qemugdb/coroutine.py | 2 +-

3 files changed, 15 insertions(+), 15 deletions(-)

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

In several part we still using req->dev or VIRTIO_DEVICE(req->dev)

when we have already defined s and vdev pointers:

VirtIOBlock *s = req->dev;

VirtIODevice *vdev = VIRTIO_DEVICE(s);

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Reviewed-by: Liam Merwick <liam.merwick@oracle.com>

Message-id: 20190208142347.214815-1-sgarzare@redhat.com

hw/block/virtio-blk.c | 22 +++++++++-------------

1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c

--- a/hw/block/virtio-blk.c

+++ b/hw/block/virtio-blk.c

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status)

static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,

bool is_read)

- BlockErrorAction action = blk_get_error_action(req->dev->blk,

- is_read, error);

VirtIOBlock *s = req->dev;

+ BlockErrorAction action = blk_get_error_action(s->blk, is_read, error);

if (action == BLOCK_ERROR_ACTION_STOP) {

/* Break the link as the next request is going to be parsed from the

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_flush_complete(void *opaque, int ret)

virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);

- block_acct_done(blk_get_stats(req->dev->blk), &req->acct);

+ block_acct_done(blk_get_stats(s->blk), &req->acct);

virtio_blk_free_request(req);

out:

@@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)

- sizeof(struct virtio_blk_inhdr);

iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr));

- type = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type);

+ type = virtio_ldl_p(vdev, &req->out.type);

/* VIRTIO_BLK_T_OUT defines the command direction. VIRTIO_BLK_T_BARRIER

* is an optional flag. Although a guest should not send this flag if

@@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)

case VIRTIO_BLK_T_IN:

{

bool is_write = type & VIRTIO_BLK_T_OUT;

- req->sector_num = virtio_ldq_p(VIRTIO_DEVICE(req->dev),

- &req->out.sector);

+ req->sector_num = virtio_ldq_p(vdev, &req->out.sector);

if (is_write) {

qemu_iovec_init_external(&req->qiov, out_iov, out_num);

@@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)

req->qiov.size / BDRV_SECTOR_SIZE);

}

- if (!virtio_blk_sect_range_ok(req->dev, req->sector_num,

- req->qiov.size)) {

+ if (!virtio_blk_sect_range_ok(s, req->sector_num, req->qiov.size)) {

virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);

- block_acct_invalid(blk_get_stats(req->dev->blk),

+ block_acct_invalid(blk_get_stats(s->blk),

is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);

virtio_blk_free_request(req);

return 0;

}

- block_acct_start(blk_get_stats(req->dev->blk),

- &req->acct, req->qiov.size,

+ block_acct_start(blk_get_stats(s->blk), &req->acct, req->qiov.size,

is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);

/* merge would exceed maximum number of requests or IO direction

* changes */

if (mrb->num_reqs > 0 && (mrb->num_reqs == VIRTIO_BLK_MAX_MERGE_REQS ||

is_write != mrb->is_write ||

- !req->dev->conf.request_merging)) {

- virtio_blk_submit_multireq(req->dev->blk, mrb);

+ !s->conf.request_merging)) {

+ virtio_blk_submit_multireq(s->blk, mrb);

}

assert(mrb->num_reqs < VIRTIO_BLK_MAX_MERGE_REQS);

2.20.1

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

qemu coroutine command results in following error output:

Python Exception <class 'gdb.error'> 'arch_prctl' has unknown return

type; cast the call to its declared return type: Error occurred in

Python command: 'arch_prctl' has unknown return type; cast the call to

its declared return type

Fix it by giving it what it wants: arch_prctl return type.

Information on the topic:

https://sourceware.org/gdb/onlinedocs/gdb/Calling.html

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

Message-id: 20190206151425.105871-1-vsementsov@virtuozzo.com

scripts/qemugdb/coroutine.py | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qemugdb/coroutine.py b/scripts/qemugdb/coroutine.py

--- a/scripts/qemugdb/coroutine.py

+++ b/scripts/qemugdb/coroutine.py

@@ -XXX,XX +XXX,XX @@ def get_fs_base():

pthread_self().'''

# %rsp - 120 is scratch space according to the SystemV ABI

old = gdb.parse_and_eval('*(uint64_t*)($rsp - 120)')

- gdb.execute('call arch_prctl(0x1003, $rsp - 120)', False, True)

+ gdb.execute('call (int)arch_prctl(0x1003, $rsp - 120)', False, True)

fs_base = gdb.parse_and_eval('*(uint64_t*)($rsp - 120)')

gdb.execute('set *(uint64_t*)($rsp - 120) = %s' % old, False, True)

return fs_base

2.20.1

From: Peter Xu <peterx@redhat.com>

Lukas reported an hard to reproduce QMP iothread hang on s390 that

QEMU might hang at pthread_join() of the QMP monitor iothread before

quitting:

Thread 1

#0 0x000003ffad10932c in pthread_join

#1 0x0000000109e95750 in qemu_thread_join

at /home/thuth/devel/qemu/util/qemu-thread-posix.c:570

#2 0x0000000109c95a1c in iothread_stop

#3 0x0000000109bb0874 in monitor_cleanup

#4 0x0000000109b55042 in main

While the iothread is still in the main loop:

Thread 4

#0 0x000003ffad0010e4 in ??

#1 0x000003ffad553958 in g_main_context_iterate.isra.19

#2 0x000003ffad553d90 in g_main_loop_run

#3 0x0000000109c9585a in iothread_run

at /home/thuth/devel/qemu/iothread.c:74

#4 0x0000000109e94752 in qemu_thread_start

at /home/thuth/devel/qemu/util/qemu-thread-posix.c:502

#5 0x000003ffad10825a in start_thread

#6 0x000003ffad00dcf2 in thread_start

IMHO it's because there's a race between the main thread and iothread

when stopping the thread in following sequence:

main thread iothread

=========== ==============

aio_poll()

iothread_get_g_main_context

set iothread->worker_context

iothread_stop

schedule iothread_stop_bh

execute iothread_stop_bh [1]

set iothread->running=false

(since main_loop==NULL so

skip to quit main loop.

Note: although main_loop is

NULL but worker_context is

not!)

atomic_read(&iothread->worker_context) [2]

create main_loop object

g_main_loop_run() [3]

pthread_join() [4]

We can see that when execute iothread_stop_bh() at [1] it's possible

that main_loop is still NULL because it's only created until the first

check of the worker_context later at [2]. Then the iothread will hang

in the main loop [3] and it'll starve the main thread too [4].

Here the simple solution should be that we check again the "running"

variable before check against worker_context.

CC: Thomas Huth <thuth@redhat.com>

CC: Dr. David Alan Gilbert <dgilbert@redhat.com>

CC: Stefan Hajnoczi <stefanha@redhat.com>

CC: Lukáš Doktor <ldoktor@redhat.com>

CC: Markus Armbruster <armbru@redhat.com>

CC: Eric Blake <eblake@redhat.com>

CC: Paolo Bonzini <pbonzini@redhat.com>

Reported-by: Lukáš Doktor <ldoktor@redhat.com>

Signed-off-by: Peter Xu <peterx@redhat.com>

Tested-by: Thomas Huth <thuth@redhat.com>

Message-id: 20190129051432.22023-1-peterx@redhat.com

iothread.c | 6 +++++-

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

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

--- a/iothread.c

+++ b/iothread.c

@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)

while (iothread->running) {

aio_poll(iothread->ctx, true);

- if (atomic_read(&iothread->worker_context)) {

+ /*

+ * We must check the running state again in case it was

+ * changed in previous aio_poll()

+ */

+ if (iothread->running && atomic_read(&iothread->worker_context)) {

GMainLoop *loop;

g_main_context_push_thread_default(iothread->worker_context);

2.20.1