|
From: | Paolo Bonzini |
Subject: | Re: [PATCH v2 for-7.1 3/9] nbd: remove peppering of nbd_client_connected |
Date: | Sat, 23 Apr 2022 10:30:49 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 |
Hi, thanks for the careful review and sorry I'm only replying now. On 4/15/22 19:01, Vladimir Sementsov-Ogievskiy wrote:
@@ -982,11 +978,6 @@ static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,NBDReply local_reply; NBDStructuredReplyChunk *chunk; Error *local_err = NULL; - if (!nbd_client_connected(s)) { - error_setg(&local_err, "Connection closed"); - nbd_iter_channel_error(iter, -EIO, &local_err); - goto break_loop; - }Probably we should still check iter->ret here. It's strange to start new iteration, when user set iter->ret in previous iteration of NBD_FOREACH_REPLY_CHUNK()Or, maybe we should set iter->done in nbd_iter_channel_error ?
Yes, this second one is a possibility. I chose to check iter->ret below because it was a bit more self-contained ("before reading again check that the error code is not overwritten"), but it is also less obvious.
Paolo
if (iter->done) { /* Previous iteration was last. */@@ -1007,7 +998,7 @@ static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,}/* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */- if (nbd_reply_is_simple(reply) || !nbd_client_connected(s)) { + if (nbd_reply_is_simple(reply) || iter->ret < 0) {And then here, may be we can just goto break_loop from previous "if (ret < 0)". Then we'll not have to check iter->ret.goto break_loop; }anyway: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>(a bit weak, as it really hard to imagine all these paths and possible consequences :/
[Prev in Thread] | Current Thread | [Next in Thread] |