On 7/31/18 12:30 PM, Vladimir Sementsov-Ogievskiy wrote: > Implement reconnect. To achieve this: > > 1. add new modes: > connecting-wait: means, that reconnecting is in progress, and there > were small number of reconnect attempts, so all requests are > waiting for the connection. > connecting-nowait: reconnecting is in progress, there were a lot of > attempts of reconnect, all requests will return errors. > > two old modes are used too: > connected: normal state > quit: exiting after fatal error or on close
What makes an error fatal? Without reconnect, life is simple - if the server sends something we can't parse, we permanently turn the device into an error condition - because we have no way to get back in sync with the server for further commands. Your patch allows reconnect attempts where the connection is down (we failed to send to the server or failed to receive the server's reply), but why can we not ALSO attempt to reconnect after a parse error? A reconnect would let us get back in sync for attempting further commands. You're right that the current command should probably fail in that case (if the server sent us garbage for a specific request, it will probably do so again on a repeat of that request; which is different than when we don't even know what the server would have sent because of a disconnect). Or, put another way, we KNOW we have (corner) cases where a mis-aligned image can currently cause the server to return BLOCK_STATUS replies that aren't aligned to the advertised minimumm block size. Attempting to read the last sector of an image then causes the client to see the misaligned reply and complain, which we are treating as fatal. But why not instead just fail that particular read, but still attempt a reconnect, in order to attempt further reads elsewhere in the image that do not trip up the server's misaligned reply? > > Possible transitions are: > > * -> quit > connecting-* -> connected > connecting-wait -> connecting-nowait (transition is done after > reconnect-delay seconds in connecting-wait mode) > connected -> connecting-wait > > 2. Implement reconnect in connection_co. So, in connecting-* mode, > connection_co, tries to reconnect unlimited times. > > 3. Retry nbd queries on channel error, if we are in connecting-wait > state. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > block/nbd-client.h | 4 + > block/nbd-client.c | 304 > +++++++++++++++++++++++++++++++++++++++++++---------- > 2 files changed, 255 insertions(+), 53 deletions(-) > > @@ -781,16 +936,21 @@ static int nbd_co_request(BlockDriverState *bs, > NBDRequest *request, > } else { > assert(request->type != NBD_CMD_WRITE); > } > - ret = nbd_co_send_request(bs, request, write_qiov); > - if (ret < 0) { > - return ret; > - } > > - ret = nbd_co_receive_return_code(client, request->handle, > - &request_ret, &local_err); > - if (local_err) { > - error_report_err(local_err); > - } > + do { > + ret = nbd_co_send_request(bs, request, write_qiov); > + if (ret < 0) { > + continue; > + } > + > + ret = nbd_co_receive_return_code(client, request->handle, > + &request_ret, &local_err); > + if (local_err) { > + error_report_err(local_err); > + local_err = NULL; Conflicts with the conversion to use trace points. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature