]>
Commit | Line | Data |
---|---|---|
b45e13fe AD |
1 | From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
2 | From: Eric Blake <eblake@redhat.com> | |
3 | Date: Wed, 27 Sep 2017 17:57:22 +0200 | |
4 | Subject: [PATCH] nbd-client: avoid read_reply_co entry if send failed | |
5 | ||
6 | RH-Author: Eric Blake <eblake@redhat.com> | |
7 | Message-id: <20170927175725.20023-5-eblake@redhat.com> | |
8 | Patchwork-id: 76674 | |
9 | O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 4/7] nbd-client: avoid read_reply_co entry if send failed | |
10 | Bugzilla: 1495474 | |
11 | RH-Acked-by: Max Reitz <mreitz@redhat.com> | |
12 | RH-Acked-by: Jeffrey Cody <jcody@redhat.com> | |
13 | RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com> | |
14 | ||
15 | From: Stefan Hajnoczi <stefanha@redhat.com> | |
16 | ||
17 | The following segfault is encountered if the NBD server closes the UNIX | |
18 | domain socket immediately after negotiation: | |
19 | ||
20 | Program terminated with signal SIGSEGV, Segmentation fault. | |
21 | #0 aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441 | |
22 | 441 QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines, | |
23 | (gdb) bt | |
24 | #0 0x000000d3c01a50f8 in aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441 | |
25 | #1 0x000000d3c012fa90 in nbd_coroutine_end (bs=bs@entry=0xd3c0fec650, request=<optimized out>) at block/nbd-client.c:207 | |
26 | #2 0x000000d3c012fb58 in nbd_client_co_preadv (bs=0xd3c0fec650, offset=0, bytes=<optimized out>, qiov=0x7ffc10a91b20, flags=0) at block/nbd-client.c:237 | |
27 | #3 0x000000d3c0128e63 in bdrv_driver_preadv (bs=bs@entry=0xd3c0fec650, offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, flags=0) at block/io.c:836 | |
28 | #4 0x000000d3c012c3e0 in bdrv_aligned_preadv (child=child@entry=0xd3c0ff51d0, req=req@entry=0x7f31885d6e90, offset=offset@entry=0, bytes=bytes@entry=512, align=align@entry=1, qiov=qiov@entry=0x7ffc10a91b20, f | |
29 | +lags=0) at block/io.c:1086 | |
30 | #5 0x000000d3c012c6b8 in bdrv_co_preadv (child=0xd3c0ff51d0, offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, flags=flags@entry=0) at block/io.c:1182 | |
31 | #6 0x000000d3c011cc17 in blk_co_preadv (blk=0xd3c0ff4f80, offset=0, bytes=512, qiov=0x7ffc10a91b20, flags=0) at block/block-backend.c:1032 | |
32 | #7 0x000000d3c011ccec in blk_read_entry (opaque=0x7ffc10a91b40) at block/block-backend.c:1079 | |
33 | #8 0x000000d3c01bbb96 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at util/coroutine-ucontext.c:79 | |
34 | #9 0x00007f3196cb8600 in __start_context () at /lib64/libc.so.6 | |
35 | ||
36 | The problem is that nbd_client_init() uses | |
37 | nbd_client_attach_aio_context() -> aio_co_schedule(new_context, | |
38 | client->read_reply_co). Execution of read_reply_co is deferred to a BH | |
39 | which doesn't run until later. | |
40 | ||
41 | In the mean time blk_co_preadv() can be called and nbd_coroutine_end() | |
42 | calls aio_wake() on read_reply_co. At this point in time | |
43 | read_reply_co's ctx isn't set because it has never been entered yet. | |
44 | ||
45 | This patch simplifies the nbd_co_send_request() -> | |
46 | nbd_co_receive_reply() -> nbd_coroutine_end() lifecycle to just | |
47 | nbd_co_send_request() -> nbd_co_receive_reply(). The request is "ended" | |
48 | if an error occurs at any point. Callers no longer have to invoke | |
49 | nbd_coroutine_end(). | |
50 | ||
51 | This cleanup also eliminates the segfault because we don't call | |
52 | aio_co_schedule() to wake up s->read_reply_co if sending the request | |
53 | failed. It is only necessary to wake up s->read_reply_co if a reply was | |
54 | received. | |
55 | ||
56 | Note this only happens with UNIX domain sockets on Linux. It doesn't | |
57 | seem possible to reproduce this with TCP sockets. | |
58 | ||
59 | Suggested-by: Paolo Bonzini <pbonzini@redhat.com> | |
60 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | |
61 | Message-Id: <20170829122745.14309-2-stefanha@redhat.com> | |
62 | Signed-off-by: Eric Blake <eblake@redhat.com> | |
63 | (cherry picked from commit 3c2d5183f9fa4eac3d17d841e26da65a0181ae7b) | |
64 | Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> | |
65 | --- | |
66 | block/nbd-client.c | 25 +++++++++---------------- | |
67 | 1 file changed, 9 insertions(+), 16 deletions(-) | |
68 | ||
69 | diff --git a/block/nbd-client.c b/block/nbd-client.c | |
507c6de3 | 70 | index f7bca3f996..434acf647f 100644 |
b45e13fe AD |
71 | --- a/block/nbd-client.c |
72 | +++ b/block/nbd-client.c | |
73 | @@ -139,12 +139,12 @@ static int nbd_co_send_request(BlockDriverState *bs, | |
74 | request->handle = INDEX_TO_HANDLE(s, i); | |
75 | ||
76 | if (s->quit) { | |
77 | - qemu_co_mutex_unlock(&s->send_mutex); | |
78 | - return -EIO; | |
79 | + rc = -EIO; | |
80 | + goto err; | |
81 | } | |
82 | if (!s->ioc) { | |
83 | - qemu_co_mutex_unlock(&s->send_mutex); | |
84 | - return -EPIPE; | |
85 | + rc = -EPIPE; | |
86 | + goto err; | |
87 | } | |
88 | ||
89 | if (qiov) { | |
90 | @@ -161,8 +161,13 @@ static int nbd_co_send_request(BlockDriverState *bs, | |
91 | } else { | |
92 | rc = nbd_send_request(s->ioc, request); | |
93 | } | |
94 | + | |
95 | +err: | |
96 | if (rc < 0) { | |
97 | s->quit = true; | |
98 | + s->requests[i].coroutine = NULL; | |
99 | + s->in_flight--; | |
100 | + qemu_co_queue_next(&s->free_sema); | |
101 | } | |
102 | qemu_co_mutex_unlock(&s->send_mutex); | |
103 | return rc; | |
104 | @@ -196,13 +201,6 @@ static void nbd_co_receive_reply(NBDClientSession *s, | |
105 | /* Tell the read handler to read another header. */ | |
106 | s->reply.handle = 0; | |
107 | } | |
108 | -} | |
109 | - | |
110 | -static void nbd_coroutine_end(BlockDriverState *bs, | |
111 | - NBDRequest *request) | |
112 | -{ | |
113 | - NBDClientSession *s = nbd_get_client_session(bs); | |
114 | - int i = HANDLE_TO_INDEX(s, request->handle); | |
115 | ||
116 | s->requests[i].coroutine = NULL; | |
117 | ||
118 | @@ -238,7 +236,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset, | |
119 | } else { | |
120 | nbd_co_receive_reply(client, &request, &reply, qiov); | |
121 | } | |
122 | - nbd_coroutine_end(bs, &request); | |
123 | return -reply.error; | |
124 | } | |
125 | ||
126 | @@ -267,7 +264,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset, | |
127 | } else { | |
128 | nbd_co_receive_reply(client, &request, &reply, NULL); | |
129 | } | |
130 | - nbd_coroutine_end(bs, &request); | |
131 | return -reply.error; | |
132 | } | |
133 | ||
134 | @@ -301,7 +297,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, | |
135 | } else { | |
136 | nbd_co_receive_reply(client, &request, &reply, NULL); | |
137 | } | |
138 | - nbd_coroutine_end(bs, &request); | |
139 | return -reply.error; | |
140 | } | |
141 | ||
142 | @@ -325,7 +320,6 @@ int nbd_client_co_flush(BlockDriverState *bs) | |
143 | } else { | |
144 | nbd_co_receive_reply(client, &request, &reply, NULL); | |
145 | } | |
146 | - nbd_coroutine_end(bs, &request); | |
147 | return -reply.error; | |
148 | } | |
149 | ||
150 | @@ -350,7 +344,6 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count) | |
151 | } else { | |
152 | nbd_co_receive_reply(client, &request, &reply, NULL); | |
153 | } | |
154 | - nbd_coroutine_end(bs, &request); | |
155 | return -reply.error; | |
156 | ||
157 | } | |
158 | -- | |
507c6de3 | 159 | 2.11.0 |
b45e13fe | 160 |