aboutsummaryrefslogtreecommitdiff
path: root/hw
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2018-02-02 16:26:41 +0000
committerPeter Maydell <peter.maydell@linaro.org>2018-02-02 16:26:41 +0000
commitf74425e267f81f0f94adf47ecbd66224e0461936 (patch)
treeb73f3aed99966e35fac6a38126bc79882c69b396 /hw
parentfabbd691fd7db2e71c6702089c9656e7047eac24 (diff)
parent9ea776ee7d4061c043d0fbf89aa85f86ec0cf8a2 (diff)
downloadqemu-f74425e267f81f0f94adf47ecbd66224e0461936.zip
qemu-f74425e267f81f0f94adf47ecbd66224e0461936.tar.gz
qemu-f74425e267f81f0f94adf47ecbd66224e0461936.tar.bz2
Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging
This series is mostly about 9p request cancellation. It fixes a long standing bug (read "specification violation") where the server would send an invalid response when the client has cancelled an in-flight request. This was causing annoying spurious EINTR returns in linux. The fix comes with some related testing in QTEST. Other patches are code cleanup and improvements. # gpg: Signature made Fri 02 Feb 2018 10:16:03 GMT # gpg: using RSA key 71D4D5E5822F73D6 # gpg: Good signature from "Greg Kurz <groug@kaod.org>" # gpg: aka "Gregory Kurz <gregory.kurz@free.fr>" # gpg: aka "[jpeg image of size 3330]" # Primary key fingerprint: B482 8BAF 9431 40CE F2A3 4910 71D4 D5E5 822F 73D6 * remotes/gkurz/tags/for-upstream: tests/virtio-9p: explicitly handle potential integer overflows tests: virtio-9p: add FLUSH operation test libqos/virtio: return length written into used descriptor tests: virtio-9p: add WRITE operation test tests: virtio-9p: add LOPEN operation test tests: virtio-9p: use the synth backend tests: virtio-9p: wait for completion in the test code tests: virtio-9p: move request tag to the test functions 9pfs: Correctly handle cancelled requests 9pfs: drop v9fs_register_transport() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Diffstat (limited to 'hw')
-rw-r--r--hw/9pfs/9p-synth.c52
-rw-r--r--hw/9pfs/9p-synth.h13
-rw-r--r--hw/9pfs/9p.c25
-rw-r--r--hw/9pfs/9p.h10
-rw-r--r--hw/9pfs/trace-events1
-rw-r--r--hw/9pfs/virtio-9p-device.c8
-rw-r--r--hw/9pfs/xen-9p-backend.c3
7 files changed, 95 insertions, 17 deletions
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 8f255e9..18082df 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -19,6 +19,7 @@
#include "qemu/rcu.h"
#include "qemu/rcu_queue.h"
#include "qemu/cutils.h"
+#include "sysemu/qtest.h"
/* Root node for synth file system */
static V9fsSynthNode synth_root = {
@@ -514,6 +515,26 @@ static int synth_unlinkat(FsContext *ctx, V9fsPath *dir,
return -1;
}
+static ssize_t v9fs_synth_qtest_write(void *buf, int len, off_t offset,
+ void *arg)
+{
+ return 1;
+}
+
+static ssize_t v9fs_synth_qtest_flush_write(void *buf, int len, off_t offset,
+ void *arg)
+{
+ bool should_block = !!*(uint8_t *)buf;
+
+ if (should_block) {
+ /* This will cause the server to call us again until we're cancelled */
+ errno = EINTR;
+ return -1;
+ }
+
+ return 1;
+}
+
static int synth_init(FsContext *ctx, Error **errp)
{
QLIST_INIT(&synth_root.child);
@@ -527,6 +548,37 @@ static int synth_init(FsContext *ctx, Error **errp)
/* Mark the subsystem is ready for use */
synth_fs = 1;
+
+ if (qtest_enabled()) {
+ V9fsSynthNode *node = NULL;
+ int i, ret;
+
+ /* Directory hierarchy for WALK test */
+ for (i = 0; i < P9_MAXWELEM; i++) {
+ char *name = g_strdup_printf(QTEST_V9FS_SYNTH_WALK_FILE, i);
+
+ ret = qemu_v9fs_synth_mkdir(node, 0700, name, &node);
+ assert(!ret);
+ g_free(name);
+ }
+
+ /* File for LOPEN test */
+ ret = qemu_v9fs_synth_add_file(NULL, 0, QTEST_V9FS_SYNTH_LOPEN_FILE,
+ NULL, NULL, ctx);
+ assert(!ret);
+
+ /* File for WRITE test */
+ ret = qemu_v9fs_synth_add_file(NULL, 0, QTEST_V9FS_SYNTH_WRITE_FILE,
+ NULL, v9fs_synth_qtest_write, ctx);
+ assert(!ret);
+
+ /* File for FLUSH test */
+ ret = qemu_v9fs_synth_add_file(NULL, 0, QTEST_V9FS_SYNTH_FLUSH_FILE,
+ NULL, v9fs_synth_qtest_flush_write,
+ ctx);
+ assert(!ret);
+ }
+
return 0;
}
diff --git a/hw/9pfs/9p-synth.h b/hw/9pfs/9p-synth.h
index 49c2fc7..af7a993 100644
--- a/hw/9pfs/9p-synth.h
+++ b/hw/9pfs/9p-synth.h
@@ -49,4 +49,17 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
const char *name, v9fs_synth_read read,
v9fs_synth_write write, void *arg);
+/* qtest stuff */
+
+#define QTEST_V9FS_SYNTH_WALK_FILE "WALK%d"
+#define QTEST_V9FS_SYNTH_LOPEN_FILE "LOPEN"
+#define QTEST_V9FS_SYNTH_WRITE_FILE "WRITE"
+
+/* Any write to the "FLUSH" file is handled one byte at a time by the
+ * backend. If the byte is zero, the backend returns success (ie, 1),
+ * otherwise it forces the server to try again forever. Thus allowing
+ * the client to cancel the request.
+ */
+#define QTEST_V9FS_SYNTH_FLUSH_FILE "FLUSH"
+
#endif
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 909a611..85a1ed8 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -24,6 +24,7 @@
#include "coth.h"
#include "trace.h"
#include "migration/blocker.h"
+#include "sysemu/qtest.h"
int open_fd_hw;
int total_open_fd;
@@ -630,6 +631,24 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu, ssize_t len)
V9fsState *s = pdu->s;
int ret;
+ /*
+ * The 9p spec requires that successfully cancelled pdus receive no reply.
+ * Sending a reply would confuse clients because they would
+ * assume that any EINTR is the actual result of the operation,
+ * rather than a consequence of the cancellation. However, if
+ * the operation completed (succesfully or with an error other
+ * than caused be cancellation), we do send out that reply, both
+ * for efficiency and to avoid confusing the rest of the state machine
+ * that assumes passing a non-error here will mean a successful
+ * transmission of the reply.
+ */
+ bool discard = pdu->cancelled && len == -EINTR;
+ if (discard) {
+ trace_v9fs_rcancel(pdu->tag, pdu->id);
+ pdu->size = 0;
+ goto out_notify;
+ }
+
if (len < 0) {
int err = -len;
len = 7;
@@ -3485,7 +3504,8 @@ void pdu_submit(V9fsPDU *pdu, P9MsgHeader *hdr)
}
/* Returns 0 on success, 1 on failure. */
-int v9fs_device_realize_common(V9fsState *s, Error **errp)
+int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
+ Error **errp)
{
int i, len;
struct stat stat;
@@ -3493,6 +3513,9 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
V9fsPath path;
int rc = 1;
+ assert(!s->transport);
+ s->transport = t;
+
/* initialize pdu allocator */
QLIST_INIT(&s->free_list);
QLIST_INIT(&s->active_list);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index ffe658a..5ced427 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -346,7 +346,8 @@ void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...);
void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs);
int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
const char *name, V9fsPath *path);
-int v9fs_device_realize_common(V9fsState *s, Error **errp);
+int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
+ Error **errp);
void v9fs_device_unrealize_common(V9fsState *s, Error **errp);
V9fsPDU *pdu_alloc(V9fsState *s);
@@ -366,11 +367,4 @@ struct V9fsTransport {
void (*push_and_notify)(V9fsPDU *pdu);
};
-static inline int v9fs_register_transport(V9fsState *s, const V9fsTransport *t)
-{
- assert(!s->transport);
- s->transport = t;
- return 0;
-}
-
#endif
diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
index 08a4abf..1aee350 100644
--- a/hw/9pfs/trace-events
+++ b/hw/9pfs/trace-events
@@ -1,6 +1,7 @@
# See docs/devel/tracing.txt for syntax documentation.
# hw/9pfs/virtio-9p.c
+v9fs_rcancel(uint16_t tag, uint8_t id) "tag %d id %d"
v9fs_rerror(uint16_t tag, uint8_t id, int err) "tag %d id %d err %d"
v9fs_version(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag %d id %d msize %d version %s"
v9fs_version_return(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag %d id %d msize %d version %s"
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 43f4e53..775e8ff 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -198,17 +198,13 @@ static void virtio_9p_device_realize(DeviceState *dev, Error **errp)
V9fsVirtioState *v = VIRTIO_9P(dev);
V9fsState *s = &v->state;
- if (v9fs_device_realize_common(s, errp)) {
- goto out;
+ if (v9fs_device_realize_common(s, &virtio_9p_transport, errp)) {
+ return;
}
v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag);
virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size);
v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output);
- v9fs_register_transport(s, &virtio_9p_transport);
-
-out:
- return;
}
static void virtio_9p_device_unrealize(DeviceState *dev, Error **errp)
diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index df2a410..14f0d6a 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -446,7 +446,6 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
xen_9pdev->id = s->fsconf.fsdev_id =
g_strdup_printf("xen9p%d", xendev->dev);
xen_9pdev->tag = s->fsconf.tag = xenstore_read_fe_str(xendev, "tag");
- v9fs_register_transport(s, &xen_9p_transport);
fsdev = qemu_opts_create(qemu_find_opts("fsdev"),
s->fsconf.tag,
1, NULL);
@@ -455,7 +454,7 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
qemu_opt_set(fsdev, "security_model", xen_9pdev->security_model, NULL);
qemu_opts_set_id(fsdev, s->fsconf.fsdev_id);
qemu_fsdev_add(fsdev);
- v9fs_device_realize_common(s, NULL);
+ v9fs_device_realize_common(s, &xen_9p_transport, NULL);
return 0;