From a1bf8b7414a336c9d26bf62efecdbe64b3617d76 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 25 Nov 2016 12:54:21 +0100 Subject: [PATCH 1/5] 9pfs: add missing coroutine_fn annotations Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index fa58877570..58310ca8d5 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1571,7 +1571,7 @@ out_nofid: v9fs_string_free(&name); } -static void v9fs_fsync(void *opaque) +static void coroutine_fn v9fs_fsync(void *opaque) { int err; int32_t fid; @@ -2337,7 +2337,7 @@ out_nofid: v9fs_string_free(&symname); } -static void v9fs_flush(void *opaque) +static void coroutine_fn v9fs_flush(void *opaque) { ssize_t err; int16_t tag; From 6e37f458d2381d3572f94fa0ce5487a3e17209fa Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Tue, 10 Jan 2017 15:32:21 +0100 Subject: [PATCH 2/5] tests: virtio-9p: improve error reporting Signed-off-by: Greg Kurz --- tests/virtio-9p-test.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c index 060407b20e..9556291567 100644 --- a/tests/virtio-9p-test.c +++ b/tests/virtio-9p-test.c @@ -236,6 +236,16 @@ static void v9fs_req_send(P9Req *req) req->t_off = 0; } +static const char *rmessage_name(uint8_t id) +{ + return + id == P9_RLERROR ? "RLERROR" : + id == P9_RVERSION ? "RVERSION" : + id == P9_RATTACH ? "RATTACH" : + id == P9_RWALK ? "RWALK" : + ""; +} + static void v9fs_req_recv(P9Req *req, uint8_t id) { QVirtIO9P *v9p = req->v9p; @@ -258,11 +268,15 @@ static void v9fs_req_recv(P9Req *req, uint8_t id) g_assert_cmpint(hdr.size, <=, P9_MAX_SIZE); g_assert_cmpint(hdr.tag, ==, req->tag); - if (hdr.id != id && hdr.id == P9_RLERROR) { - uint32_t err; - v9fs_uint32_read(req, &err); - g_printerr("Received Rlerror (%d) instead of Response %d\n", err, id); - g_assert_not_reached(); + if (hdr.id != id) { + g_printerr("Received response %d (%s) instead of %d (%s)\n", + hdr.id, rmessage_name(hdr.id), id, rmessage_name(id)); + + if (hdr.id == P9_RLERROR) { + uint32_t err; + v9fs_uint32_read(req, &err); + g_printerr("Rlerror has errno %d (%s)\n", err, strerror(err)); + } } g_assert_cmpint(hdr.id, ==, id); } From 0d78289c3dca3de8e614a551a3d4a9415168ace0 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 13 Jan 2017 18:18:20 +0100 Subject: [PATCH 3/5] 9pfs: fix off-by-one error in PDU free list The server can handle MAX_REQ - 1 PDUs at a time and the virtio-9p device has a MAX_REQ sized virtqueue. If the client manages to fill up the virtqueue, pdu_alloc() will fail and the request won't be processed without any notice to the client (it actually causes the linux 9p client to hang). This has been there since the beginning (commit 9f10751365b2 "virtio-9p: Add a virtio 9p device to qemu"), but it needs an agressive workload to run in the guest to show up. We actually allocate MAX_REQ PDUs and I see no reason not to link them all into the free list, so let's fix the init loop. Reported-by: Tuomas Tynkkynen Suggested-by: Al Viro Signed-off-by: Greg Kurz --- hw/9pfs/9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 58310ca8d5..d2d0288282 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -3454,7 +3454,7 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp) /* initialize pdu allocator */ QLIST_INIT(&s->free_list); QLIST_INIT(&s->active_list); - for (i = 0; i < (MAX_REQ - 1); i++) { + for (i = 0; i < MAX_REQ; i++) { QLIST_INSERT_HEAD(&s->free_list, &s->pdus[i], next); s->pdus[i].s = s; s->pdus[i].idx = i; From 6fe76acc2d0fcadb1d827cffffab81d6c8d66704 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 23 Jan 2017 09:46:13 +0100 Subject: [PATCH 4/5] 9pfs: local: trivial cosmetic fix in pwritev op Signed-off-by: Greg Kurz --- hw/9pfs/9p-local.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index 845675e7a1..7de07e1ba6 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -436,8 +436,7 @@ static ssize_t local_pwritev(FsContext *ctx, V9fsFidOpenState *fs, const struct iovec *iov, int iovcnt, off_t offset) { - ssize_t ret -; + ssize_t ret; #ifdef CONFIG_PREADV ret = pwritev(fs->fd, iov, iovcnt, offset); #else From fa0eb5c512d17a223d9f9bac45da48d78d12f584 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Wed, 25 Jan 2017 00:23:49 +0100 Subject: [PATCH 5/5] 9pfs: fix offset error in v9fs_xattr_read() The current code tries to copy `read_count' bytes starting at offset `offset' from a `read_count`-sized iovec. This causes v9fs_pack() to fail with ENOBUFS. Since the PDU iovec is already partially filled with `offset' bytes, let's skip them when creating `qiov_full' and have v9fs_pack() to copy the whole of it. Moreover, this is consistent with the other places where v9fs_init_qiov_from_pdu() is called. This fixes commit "bcb8998fac16 9pfs: call v9fs_init_qiov_from_pdu before v9fs_pack". Signed-off-by: Greg Kurz Reviewed-by: Stefano Stabellini --- hw/9pfs/9p.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index d2d0288282..138d8e825d 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1655,7 +1655,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu, if (is_write) { pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov); } else { - pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size); + pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size + skip); } qemu_iovec_init_external(&elem, iov, niov); @@ -1685,8 +1685,8 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, } offset += err; - v9fs_init_qiov_from_pdu(&qiov_full, pdu, 0, read_count, false); - err = v9fs_pack(qiov_full.iov, qiov_full.niov, offset, + v9fs_init_qiov_from_pdu(&qiov_full, pdu, offset, read_count, false); + err = v9fs_pack(qiov_full.iov, qiov_full.niov, 0, ((char *)fidp->fs.xattr.value) + off, read_count); qemu_iovec_destroy(&qiov_full);